sunrpc: make the cache cleaner workqueue deferrable

PATCH In review Artem Bityutskiy 2 participants 6 messages Jul 1, 2010 – Jul 17, 2010

Subsystems (3)

Networking NFS client NFS server

lore

Share & tools

lore

Artem Bityutskiy

author

2010-07-01 15:09 UTC #0 lore raw

PATCH 1/1 sunrpc: make the cache cleaner workqueue deferrable

From: Artem Bityutskiy <[email protected]>

This patch makes the cache_cleaner workqueue deferrable, to prevent unnecessary system wake-ups, which is very important for embedded battery-powered devices.

do_cache_clean() is called every 30 seconds at the moment, and often makes the system wake up from its power-save sleep state. With this change, when the workqueue uses a deferrable timer, the do_cache_clean() invocation will be delayed and combined with the closest "real" wake-up. This improves the power consumption situation.

Note, I tried to create a DECLARE_DELAYED_WORK_DEFERRABLE() helper macro, similar to DECLARE_DELAYED_WORK(), but failed because of the way the timer wheel core stores the deferrable flag (it is the LSBit in the time->base pointer). My attempt to define a static variable with this bit set ended up with the "initializer element is not constant" error.

Thus, I have to use run-time initialization, so I created a new cache_initialize() function which is called once when sunrpc is being initialized.

Signed-off-by Artem Bityutskiy <[email protected]>

---
 include/linux/sunrpc/cache.h |    1 +
 net/sunrpc/cache.c           |    7 ++++++-
 net/sunrpc/sunrpc_syms.c     |    1 +
 3 files changed, 8 insertions(+), 1 deletions(-)

include/linux/sunrpc/cache.h +1 -0

index 6f52b4d..7bf3e84 100644
@@ -192,6 +192,7 @@ extern int cache_check(struct cache_detail *detail,
 extern void cache_flush(void);
 extern void cache_purge(struct cache_detail *detail);
 #define NEVER (0x7FFFFFFF)
+extern void __init cache_initialize(void);
 extern int cache_register(struct cache_detail *cd);
 extern void cache_unregister(struct cache_detail *cd);

net/sunrpc/cache.c +6 -1

index 58de76c..939d048 100644
@@ -320,7 +320,7 @@ static struct cache_detail *current_detail;
 static int current_index;
 
 static void do_cache_clean(struct work_struct *work);
-static DECLARE_DELAYED_WORK(cache_cleaner, do_cache_clean);
+static struct delayed_work cache_cleaner;
 
 static void sunrpc_init_cache_detail(struct cache_detail *cd)
 {

@@ -1504,6 +1504,11 @@ static int create_cache_proc_entries(struct cache_detail *cd)
 }
 #endif
 
+void __init cache_initialize(void)
+{
+	INIT_DELAYED_WORK_DEFERRABLE(&cache_cleaner, do_cache_clean);
+}
+
 int cache_register(struct cache_detail *cd)
 {
 	int ret;

net/sunrpc/sunrpc_syms.c +1 -0

index f438347..c52b184 100644
@@ -43,6 +43,7 @@ init_sunrpc(void)
 #ifdef CONFIG_PROC_FS
 	rpc_proc_init();
 #endif
+	cache_initialize();
 	cache_register(&ip_map_cache);
 	cache_register(&unix_gid_cache);
 	svc_init_xprt_sock();	/* svc sock transport */

Signature · 1 line

1.7.0.1

Artem Bityutskiy

author

2010-07-05 12:35 UTC #1 lore raw

Quoted 19 lines · Artem Bityutskiy

From: Artem Bityutskiy <[email protected]>
This patch makes the cache_cleaner workqueue deferrable, to prevent unnecessary system wake-ups, which is very important for embedded battery-powered devices.
do_cache_clean() is called every 30 seconds at the moment, and often makes the system wake up from its power-save sleep state. With this change, when the workqueue uses a deferrable timer, the do_cache_clean() invocation will be delayed and combined with the closest "real" wake-up. This improves the power consumption situation.
Note, I tried to create a DECLARE_DELAYED_WORK_DEFERRABLE() helper macro, similar to DECLARE_DELAYED_WORK(), but failed because of the way the timer wheel core stores the deferrable flag (it is the LSBit in the time->base pointer). My attempt to define a static variable with this bit set ended up with the "initializer element is not constant" error.

Quoted 5 lines

Thus, I have to use run-time initialization, so I created a new cache_initialize() function which is called once when sunrpc is being initialized.
Signed-off-by Artem Bityutskiy <[email protected]>

Bruce,

I did some git-logging, and found out that sunrpc stuff goes from you, from Trond, and from David, and MAINTAINERS also mentions Neil. Whom should I ping WRT to merging this patch?

Signature · 2 lines

Best Regards,
Artem Bityutskiy (Артём Битюцкий)

J. Bruce Fields

2010-07-06 16:29 UTC #2 lore raw

Quoted 24 lines · Artem Bityutskiy

Quoted 23 lines · Artem Bityutskiy
From: Artem Bityutskiy <[email protected]>
This patch makes the cache_cleaner workqueue deferrable, to prevent unnecessary system wake-ups, which is very important for embedded battery-powered devices.
do_cache_clean() is called every 30 seconds at the moment, and often makes the system wake up from its power-save sleep state. With this change, when the workqueue uses a deferrable timer, the do_cache_clean() invocation will be delayed and combined with the closest "real" wake-up. This improves the power consumption situation.
Note, I tried to create a DECLARE_DELAYED_WORK_DEFERRABLE() helper macro, similar to DECLARE_DELAYED_WORK(), but failed because of the way the timer wheel core stores the deferrable flag (it is the LSBit in the time->base pointer). My attempt to define a static variable with this bit set ended up with the "initializer element is not constant" error.
Thus, I have to use run-time initialization, so I created a new cache_initialize() function which is called once when sunrpc is being initialized.

Quoted 7 lines

Quoted 1 line
Signed-off-by Artem Bityutskiy <[email protected]>
Bruce,
I did some git-logging, and found out that sunrpc stuff goes from you, from Trond, and from David, and MAINTAINERS also mentions Neil. Whom should I ping WRT to merging this patch?

I got it. (I'm just getting back from vacation, and very backlogged.)

--b.

J. Bruce Fields

2010-07-06 16:28 UTC #3 lore raw

In reply to Artem Bityutskiy

Quoted 19 lines · Artem Bityutskiy

From: Artem Bityutskiy <[email protected]>
This patch makes the cache_cleaner workqueue deferrable, to prevent unnecessary system wake-ups, which is very important for embedded battery-powered devices.
do_cache_clean() is called every 30 seconds at the moment, and often makes the system wake up from its power-save sleep state. With this change, when the workqueue uses a deferrable timer, the do_cache_clean() invocation will be delayed and combined with the closest "real" wake-up. This improves the power consumption situation.
Note, I tried to create a DECLARE_DELAYED_WORK_DEFERRABLE() helper macro, similar to DECLARE_DELAYED_WORK(), but failed because of the way the timer wheel core stores the deferrable flag (it is the LSBit in the time->base pointer). My attempt to define a static variable with this bit set ended up with the "initializer element is not constant" error.

Quoted 3 lines

Thus, I have to use run-time initialization, so I created a new cache_initialize() function which is called once when sunrpc is being initialized.

Thanks, applying for 2.6.36.

--b.

Quoted 60 lines

Signed-off-by Artem Bityutskiy <[email protected]>

---
 include/linux/sunrpc/cache.h |    1 +
 net/sunrpc/cache.c           |    7 ++++++-
 net/sunrpc/sunrpc_syms.c     |    1 +
 3 files changed, 8 insertions(+), 1 deletions(-)

include/linux/sunrpc/cache.h +1 -0

index 6f52b4d..7bf3e84 100644
@@ -192,6 +192,7 @@ extern int cache_check(struct cache_detail *detail,
 extern void cache_flush(void);
 extern void cache_purge(struct cache_detail *detail);
 #define NEVER (0x7FFFFFFF)
+extern void __init cache_initialize(void);
 extern int cache_register(struct cache_detail *cd);
 extern void cache_unregister(struct cache_detail *cd);

net/sunrpc/cache.c +6 -1

index 58de76c..939d048 100644
@@ -320,7 +320,7 @@ static struct cache_detail *current_detail;
 static int current_index;
 
 static void do_cache_clean(struct work_struct *work);
-static DECLARE_DELAYED_WORK(cache_cleaner, do_cache_clean);
+static struct delayed_work cache_cleaner;
 
 static void sunrpc_init_cache_detail(struct cache_detail *cd)
 {

@@ -1504,6 +1504,11 @@ static int create_cache_proc_entries(struct cache_detail *cd)
 }
 #endif
 
+void __init cache_initialize(void)
+{
+	INIT_DELAYED_WORK_DEFERRABLE(&cache_cleaner, do_cache_clean);
+}
+
 int cache_register(struct cache_detail *cd)
 {
 	int ret;

net/sunrpc/sunrpc_syms.c +1 -0

index f438347..c52b184 100644
@@ -43,6 +43,7 @@ init_sunrpc(void)
 #ifdef CONFIG_PROC_FS
 	rpc_proc_init();
 #endif
+	cache_initialize();
 	cache_register(&ip_map_cache);
 	cache_register(&unix_gid_cache);
 	svc_init_xprt_sock();	/* svc sock transport */

Signature · 1 line

1.7.0.1

J. Bruce Fields

2010-07-06 17:19 UTC #4 lore raw

PATCH 1/1 timers.c: document meaning of deferrable timer

In reply to Artem Bityutskiy

From: J. Bruce Fields <[email protected]>

Steal some text from 6e453a67510 "Add support for deferrable timers". A reader shouldn't have to dig through the git logs for the basic description of a deferrable timer.

Signed-off-by J. Bruce Fields <[email protected]>

---
 kernel/timer.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

kernel/timer.c +7 -2

index ee305c8..947bcf6 100644
@@ -90,8 +90,13 @@ static DEFINE_PER_CPU(struct tvec_base *, tvec_bases) = &boot_tvec_bases;
 
 /*
  * Note that all tvec_bases are 2 byte aligned and lower bit of
- * base in timer_list is guaranteed to be zero. Use the LSB for
- * the new flag to indicate whether the timer is deferrable
+ * base in timer_list is guaranteed to be zero. Use the LSB to
+ * indicate whether the timer is deferrable.
+ *
+ * A deferrable timer will work normally when the system is busy, but
+ * will not cause a CPU to come out of idle just to service it; instead,
+ * the timer will be serviced when the CPU eventually wakes up with a
+ * subsequent non-deferrable timer.
  */
 #define TBASE_DEFERRABLE_FLAG		(0x1)

Signature · 1 line

1.7.0.4

Artem Bityutskiy

author

2010-07-17 12:30 UTC #5 lore raw

Re: [PATCH] timers.c: document meaning of deferrable timer

Quoted 7 lines · J. Bruce Fields

From: J. Bruce Fields <[email protected]>
=20
Steal some text from 6e453a67510 "Add support for deferrable timers". A reader shouldn't have to dig through the git logs for the basic description of a deferrable timer.
=20
Signed-off-by J. Bruce Fields <[email protected]>

Yes, this is a good idea. Someone cares to pick this?

--=20
Best Regards,
Artem Bityutskiy (=D0=90=D1=80=D1=82=D1=91=D0=BC =D0=91=D0=B8=D1=82=D1=8E= =D1=86=D0=BA=D0=B8=D0=B9)