[PATCH] use round_jiffies() for slow work thread pool's 5 second cull timer

[PATCH] use round_jiffies() for slow work thread pool's 5 second cull timer

[Chris Peterson]

The slow work thread pool culls its idle threads after 5 seconds without any work requests. Also, the slow work thread pool waits 5 seconds before starting new threads after OOM.

This patch uses round_jiffies() to round these 5 second timers to whole seconds. In this case, the actual timer wait would be between 4.75 and 5.75 seconds (because round_jiffies() rounds < 0.25 seconds down and > 0.25 seconds up). This patch also refactors the mod_timer() logic into a separate helper function.

Signed-off-by: Chris Peterson <cpeterso@cpeterso.com>
---
diff --git a/kernel/slow-work.c b/kernel/slow-work.c
index b28d191..9bfcb53 100644
--- a/kernel/slow-work.c
+++ b/kernel/slow-work.c
@@ -318,6 +318,12 @@ cant_get_ref:
 }
 EXPORT_SYMBOL(slow_work_enqueue);
 
+static void slow_work_defer_cull_time(void)
+{
+	mod_timer(&slow_work_cull_timer,
+			  round_jiffies_relative(SLOW_WORK_CULL_TIMEOUT));
+}
+
 /*
  * Worker thread culling algorithm
  */
@@ -335,8 +341,7 @@ static bool slow_work_cull_thread(void)
 		    list_empty(&vslow_work_queue) &&
 		    atomic_read(&slow_work_thread_count) >
 		    slow_work_min_threads) {
-			mod_timer(&slow_work_cull_timer,
-				  jiffies + SLOW_WORK_CULL_TIMEOUT);
+			slow_work_defer_cull_time();
 			do_cull = true;
 		}
 	}
@@ -393,8 +398,7 @@ static int slow_work_thread(void *_data)
 			    list_empty(&vslow_work_queue) &&
 			    atomic_read(&slow_work_thread_count) >
 			    slow_work_min_threads)
-				mod_timer(&slow_work_cull_timer,
-					  jiffies + SLOW_WORK_CULL_TIMEOUT);
+				slow_work_defer_cull_time();
 			continue;
 		}
 
@@ -458,7 +462,7 @@ static void slow_work_new_thread_execute(struct slow_work *work)
 		if (atomic_dec_and_test(&slow_work_thread_count))
 			BUG(); /* we're running on a slow work thread... */
 		mod_timer(&slow_work_oom_timer,
-			  jiffies + SLOW_WORK_OOM_TIMEOUT);
+				round_jiffies_relative(SLOW_WORK_OOM_TIMEOUT));
 	} else {
 		/* ratelimit the starting of new threads */
 		mod_timer(&slow_work_oom_timer, jiffies + 1);
@@ -502,8 +506,7 @@ static int slow_work_min_threads_sysctl(struct ctl_table *table, int write,
 			if (n < 0 && !slow_work_may_not_start_new_thread)
 				slow_work_enqueue(&slow_work_new_thread);
 			else if (n > 0)
-				mod_timer(&slow_work_cull_timer,
-					  jiffies + SLOW_WORK_CULL_TIMEOUT);
+				slow_work_defer_cull_time();
 		}
 		mutex_unlock(&slow_work_user_lock);
 	}
@@ -529,8 +532,7 @@ static int slow_work_max_threads_sysctl(struct ctl_table *table, int write,
 				atomic_read(&slow_work_thread_count);
 
 			if (n < 0)
-				mod_timer(&slow_work_cull_timer,
-					  jiffies + SLOW_WORK_CULL_TIMEOUT);
+				slow_work_defer_cull_time();
 		}
 		mutex_unlock(&slow_work_user_lock);
 	}

Re: [PATCH] use round_jiffies() for slow work thread pool's 5 second cull timer

[Richard Kennedy]

Chris Peterson wrote:
> The slow work thread pool culls its idle threads after 5 seconds without any work requests. Also, the slow work thread pool waits 5 seconds before starting new threads after OOM.
> 
> This patch uses round_jiffies() to round these 5 second timers to whole seconds. In this case, the actual timer wait would be between 4.75 and 5.75 seconds (because round_jiffies() rounds < 0.25 seconds down and > 0.25 seconds up). This patch also refactors the mod_timer() logic into a separate helper function.
> 
> Signed-off-by: Chris Peterson <cpeterso@cpeterso.com>
> ---
> diff --git a/kernel/slow-work.c b/kernel/slow-work.c
> index b28d191..9bfcb53 100644
> --- a/kernel/slow-work.c
> +++ b/kernel/slow-work.c
> @@ -318,6 +318,12 @@ cant_get_ref:
>  }
>  EXPORT_SYMBOL(slow_work_enqueue);
>  
> +static void slow_work_defer_cull_time(void)
> +{
> +	mod_timer(&slow_work_cull_timer,
> +			  round_jiffies_relative(SLOW_WORK_CULL_TIMEOUT));
> +}
> +
Hi Chris,

Doesn't mod_timer take an absolute time not a relative one?

So I think this should be

	mod_timer(&timer,round_jiffies(jiffies + TIMEOUT) );

regards

Richard

Re: [PATCH] use round_jiffies() for slow work thread pool's 5 second cull timer

[Chris Peterson]

Yes, you are correct. I misread how round_jiffies_relative() worked.

chris


On Sun, May 10, 2009 at 7:23 AM, Richard Kennedy
<richard@rsk.demon.co.uk> wrote:
> Chris Peterson wrote:
>> The slow work thread pool culls its idle threads after 5 seconds without any work requests. Also, the slow work thread pool waits 5 seconds before starting new threads after OOM.
>>
>> This patch uses round_jiffies() to round these 5 second timers to whole seconds. In this case, the actual timer wait would be between 4.75 and 5.75 seconds (because round_jiffies() rounds < 0.25 seconds down and > 0.25 seconds up). This patch also refactors the mod_timer() logic into a separate helper function.
>>
>> Signed-off-by: Chris Peterson <cpeterso@cpeterso.com>
>> ---
>> diff --git a/kernel/slow-work.c b/kernel/slow-work.c
>> index b28d191..9bfcb53 100644
>> --- a/kernel/slow-work.c
>> +++ b/kernel/slow-work.c
>> @@ -318,6 +318,12 @@ cant_get_ref:
>>  }
>>  EXPORT_SYMBOL(slow_work_enqueue);
>>
>> +static void slow_work_defer_cull_time(void)
>> +{
>> +     mod_timer(&slow_work_cull_timer,
>> +                       round_jiffies_relative(SLOW_WORK_CULL_TIMEOUT));
>> +}
>> +
> Hi Chris,
>
> Doesn't mod_timer take an absolute time not a relative one?
>
> So I think this should be
>
>        mod_timer(&timer,round_jiffies(jiffies + TIMEOUT) );
>
> regards
>
> Richard
>
>

[PATCH v2] use round_jiffies() for slow work thread pool's 5 second cull timer

[Chris Peterson]

This is a revised patch to round the slow work queue's 5 second timers to 
whole seconds with round_jiffies(). The slow work queue uses 5 second 
timers to cull idle threads and, after OOM, to delay new thread 
creation.This patch also refactors the mod_timer() logic into a separate 
helper function.

Signed-off-by: Chris Peterson <cpeterso@cpeterso.com>
---
diff --git a/kernel/slow-work.c b/kernel/slow-work.c
index b28d191..0ef9bf9 100644
--- a/kernel/slow-work.c
+++ b/kernel/slow-work.c
@@ -318,6 +318,12 @@ cant_get_ref:
 }
 EXPORT_SYMBOL(slow_work_enqueue);
 
+static void slow_work_defer_cull_time(void)
+{
+	mod_timer(&slow_work_cull_timer,
+			  round_jiffies(jiffies + SLOW_WORK_CULL_TIMEOUT));
+}
+
 /*
  * Worker thread culling algorithm
  */
@@ -335,8 +341,7 @@ static bool slow_work_cull_thread(void)
 		    list_empty(&vslow_work_queue) &&
 		    atomic_read(&slow_work_thread_count) >
 		    slow_work_min_threads) {
-			mod_timer(&slow_work_cull_timer,
-				  jiffies + SLOW_WORK_CULL_TIMEOUT);
+			slow_work_defer_cull_time();
 			do_cull = true;
 		}
 	}
@@ -393,8 +398,7 @@ static int slow_work_thread(void *_data)
 			    list_empty(&vslow_work_queue) &&
 			    atomic_read(&slow_work_thread_count) >
 			    slow_work_min_threads)
-				mod_timer(&slow_work_cull_timer,
-					  jiffies + SLOW_WORK_CULL_TIMEOUT);
+				slow_work_defer_cull_time();
 			continue;
 		}
 
@@ -458,7 +462,7 @@ static void slow_work_new_thread_execute(struct slow_work *work)
 		if (atomic_dec_and_test(&slow_work_thread_count))
 			BUG(); /* we're running on a slow work thread... */
 		mod_timer(&slow_work_oom_timer,
-			  jiffies + SLOW_WORK_OOM_TIMEOUT);
+				round_jiffies(jiffies + SLOW_WORK_OOM_TIMEOUT));
 	} else {
 		/* ratelimit the starting of new threads */
 		mod_timer(&slow_work_oom_timer, jiffies + 1);
@@ -502,8 +506,7 @@ static int slow_work_min_threads_sysctl(struct ctl_table *table, int write,
 			if (n < 0 && !slow_work_may_not_start_new_thread)
 				slow_work_enqueue(&slow_work_new_thread);
 			else if (n > 0)
-				mod_timer(&slow_work_cull_timer,
-					  jiffies + SLOW_WORK_CULL_TIMEOUT);
+				slow_work_defer_cull_time();
 		}
 		mutex_unlock(&slow_work_user_lock);
 	}
@@ -529,8 +532,7 @@ static int slow_work_max_threads_sysctl(struct ctl_table *table, int write,
 				atomic_read(&slow_work_thread_count);
 
 			if (n < 0)
-				mod_timer(&slow_work_cull_timer,
-					  jiffies + SLOW_WORK_CULL_TIMEOUT);
+				slow_work_defer_cull_time();
 		}
 		mutex_unlock(&slow_work_user_lock);
 	}

Re: [PATCH v2] use round_jiffies() for slow work thread pool's 5 second cull timer

[David Howells]

Chris Peterson <cpeterso@cpeterso.com> wrote:

> This is a revised patch to round the slow work queue's 5 second timers to 
> whole seconds with round_jiffies().

Why?  Your patch description doesn't say.

David

Re: [PATCH v2] use round_jiffies() for slow work thread pool's 5 second cull timer

[Chris Peterson]

>> This is a revised patch to round the slow work queue's 5 second timers to
>> whole seconds with round_jiffies().
>
> Why?  Your patch description doesn't say.
>

oops, good question. round_jiffies is useful for timers for which the
exact time they fire does not matter too much, as long as they fire
approximately every X seconds.

By rounding non-time-critical timers to whole seconds, all such timers
will be batched up to fire at the same time, rather than at various
times spread out. The goal of this is to have the CPU wake up less,
which saves power.

chris

Re: [PATCH v2] use round_jiffies() for slow work thread pool's 5 second cull timer

[David Howells]

How about this modified version of your patch?

David
---
From: Chris Peterson <cpeterso@cpeterso.com>
Subject: [PATCH] Use round_jiffies() for slow work thread pool's cull and OOM timers

Round the slow work queue's cull and OOM timeouts to whole second boundary with
round_jiffies().  The slow work queue uses a pair of timers to cull idle
threads and, after OOM, to delay new thread creation.

This patch also extracts the mod_timer() logic for the cull timer into a
separate helper function.

By rounding non-time-critical timers such as these to whole seconds, they will
be batched up to fire at the same time rather than being spread out.  This
allows the CPU wake up less, which saves power.

Signed-off-by: Chris Peterson <cpeterso@cpeterso.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 kernel/slow-work.c |   23 ++++++++++++++---------
 1 files changed, 14 insertions(+), 9 deletions(-)


diff --git a/kernel/slow-work.c b/kernel/slow-work.c
index 96e418d..a3ce6cf 100644
--- a/kernel/slow-work.c
+++ b/kernel/slow-work.c
@@ -319,6 +319,15 @@ cant_get_ref:
 EXPORT_SYMBOL(slow_work_enqueue);
 
 /*
+ * Schedule a cull of the thread pool at some time in the near future
+ */
+static void slow_work_schedule_cull(void)
+{
+	mod_timer(&slow_work_cull_timer,
+		  round_jiffies(jiffies + SLOW_WORK_CULL_TIMEOUT));
+}
+
+/*
  * Worker thread culling algorithm
  */
 static bool slow_work_cull_thread(void)
@@ -335,8 +344,7 @@ static bool slow_work_cull_thread(void)
 		    list_empty(&vslow_work_queue) &&
 		    atomic_read(&slow_work_thread_count) >
 		    slow_work_min_threads) {
-			mod_timer(&slow_work_cull_timer,
-				  jiffies + SLOW_WORK_CULL_TIMEOUT);
+			slow_work_schedule_cull();
 			do_cull = true;
 		}
 	}
@@ -394,8 +402,7 @@ static int slow_work_thread(void *_data)
 			    list_empty(&vslow_work_queue) &&
 			    atomic_read(&slow_work_thread_count) >
 			    slow_work_min_threads)
-				mod_timer(&slow_work_cull_timer,
-					  jiffies + SLOW_WORK_CULL_TIMEOUT);
+				slow_work_schedule_cull();
 			continue;
 		}
 
@@ -460,7 +467,7 @@ static void slow_work_new_thread_execute(struct slow_work *work)
 		if (atomic_dec_and_test(&slow_work_thread_count))
 			BUG(); /* we're running on a slow work thread... */
 		mod_timer(&slow_work_oom_timer,
-			  jiffies + SLOW_WORK_OOM_TIMEOUT);
+			  round_jiffies(jiffies + SLOW_WORK_OOM_TIMEOUT));
 	} else {
 		/* ratelimit the starting of new threads */
 		mod_timer(&slow_work_oom_timer, jiffies + 1);
@@ -504,8 +511,7 @@ static int slow_work_min_threads_sysctl(struct ctl_table *table, int write,
 			if (n < 0 && !slow_work_may_not_start_new_thread)
 				slow_work_enqueue(&slow_work_new_thread);
 			else if (n > 0)
-				mod_timer(&slow_work_cull_timer,
-					  jiffies + SLOW_WORK_CULL_TIMEOUT);
+				slow_work_schedule_cull();
 		}
 		mutex_unlock(&slow_work_user_lock);
 	}
@@ -531,8 +537,7 @@ static int slow_work_max_threads_sysctl(struct ctl_table *table, int write,
 				atomic_read(&slow_work_thread_count);
 
 			if (n < 0)
-				mod_timer(&slow_work_cull_timer,
-					  jiffies + SLOW_WORK_CULL_TIMEOUT);
+				slow_work_schedule_cull();
 		}
 		mutex_unlock(&slow_work_user_lock);
 	}

[PATCH] Use round_jiffies() for slow work thread pool's cull and OOM timers

[David Howells]

From: Chris Peterson <cpeterso@cpeterso.com>

Round the slow work queue's cull and OOM timeouts to whole second boundary with
round_jiffies().  The slow work queue uses a pair of timers to cull idle
threads and, after OOM, to delay new thread creation.

This patch also extracts the mod_timer() logic for the cull timer into a
separate helper function.

By rounding non-time-critical timers such as these to whole seconds, they will
be batched up to fire at the same time rather than being spread out.  This
allows the CPU wake up less, which saves power.

Signed-off-by: Chris Peterson <cpeterso@cpeterso.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 kernel/slow-work.c |   23 ++++++++++++++---------
 1 files changed, 14 insertions(+), 9 deletions(-)


diff --git a/kernel/slow-work.c b/kernel/slow-work.c
index 96e418d..a3ce6cf 100644
--- a/kernel/slow-work.c
+++ b/kernel/slow-work.c
@@ -319,6 +319,15 @@ cant_get_ref:
 EXPORT_SYMBOL(slow_work_enqueue);
 
 /*
+ * Schedule a cull of the thread pool at some time in the near future
+ */
+static void slow_work_schedule_cull(void)
+{
+	mod_timer(&slow_work_cull_timer,
+		  round_jiffies(jiffies + SLOW_WORK_CULL_TIMEOUT));
+}
+
+/*
  * Worker thread culling algorithm
  */
 static bool slow_work_cull_thread(void)
@@ -335,8 +344,7 @@ static bool slow_work_cull_thread(void)
 		    list_empty(&vslow_work_queue) &&
 		    atomic_read(&slow_work_thread_count) >
 		    slow_work_min_threads) {
-			mod_timer(&slow_work_cull_timer,
-				  jiffies + SLOW_WORK_CULL_TIMEOUT);
+			slow_work_schedule_cull();
 			do_cull = true;
 		}
 	}
@@ -394,8 +402,7 @@ static int slow_work_thread(void *_data)
 			    list_empty(&vslow_work_queue) &&
 			    atomic_read(&slow_work_thread_count) >
 			    slow_work_min_threads)
-				mod_timer(&slow_work_cull_timer,
-					  jiffies + SLOW_WORK_CULL_TIMEOUT);
+				slow_work_schedule_cull();
 			continue;
 		}
 
@@ -460,7 +467,7 @@ static void slow_work_new_thread_execute(struct slow_work *work)
 		if (atomic_dec_and_test(&slow_work_thread_count))
 			BUG(); /* we're running on a slow work thread... */
 		mod_timer(&slow_work_oom_timer,
-			  jiffies + SLOW_WORK_OOM_TIMEOUT);
+			  round_jiffies(jiffies + SLOW_WORK_OOM_TIMEOUT));
 	} else {
 		/* ratelimit the starting of new threads */
 		mod_timer(&slow_work_oom_timer, jiffies + 1);
@@ -504,8 +511,7 @@ static int slow_work_min_threads_sysctl(struct ctl_table *table, int write,
 			if (n < 0 && !slow_work_may_not_start_new_thread)
 				slow_work_enqueue(&slow_work_new_thread);
 			else if (n > 0)
-				mod_timer(&slow_work_cull_timer,
-					  jiffies + SLOW_WORK_CULL_TIMEOUT);
+				slow_work_schedule_cull();
 		}
 		mutex_unlock(&slow_work_user_lock);
 	}
@@ -531,8 +537,7 @@ static int slow_work_max_threads_sysctl(struct ctl_table *table, int write,
 				atomic_read(&slow_work_thread_count);
 
 			if (n < 0)
-				mod_timer(&slow_work_cull_timer,
-					  jiffies + SLOW_WORK_CULL_TIMEOUT);
+				slow_work_schedule_cull();
 		}
 		mutex_unlock(&slow_work_user_lock);
 	}

+ slow-work-use-round_jiffies-for-the-thread-pools-5-second-cull-timer.patch added to -mm tree

[akpm]

The patch titled
     slow-work: use round_jiffies() for the thread pool's 5 second cull timer
has been added to the -mm tree.  Its filename is
     slow-work-use-round_jiffies-for-the-thread-pools-5-second-cull-timer.patch

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

*** Remember to use Documentation/SubmitChecklist when testing your code ***

See http://userweb.kernel.org/~akpm/stuff/added-to-mm.txt to find
out what to do about this

The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/

------------------------------------------------------
Subject: slow-work: use round_jiffies() for the thread pool's 5 second cull timer
From: Chris Peterson <cpeterso@cpeterso.com>

Round the slow work queue's 5 second timers to whole seconds with
round_jiffies().  The slow work queue uses 5 second timers to cull idle
threads and, after OOM, to delay new thread creation. This patch also
refactors the mod_timer() logic into a separate helper function.

Signed-off-by: Chris Peterson <cpeterso@cpeterso.com>
Cc: Richard Kennedy <richard@rsk.demon.co.uk>
Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 kernel/slow-work.c |   20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff -puN kernel/slow-work.c~slow-work-use-round_jiffies-for-the-thread-pools-5-second-cull-timer kernel/slow-work.c
--- a/kernel/slow-work.c~slow-work-use-round_jiffies-for-the-thread-pools-5-second-cull-timer
+++ a/kernel/slow-work.c
@@ -318,6 +318,12 @@ cant_get_ref:
 }
 EXPORT_SYMBOL(slow_work_enqueue);
 
+static void slow_work_defer_cull_time(void)
+{
+	mod_timer(&slow_work_cull_timer,
+			  round_jiffies(jiffies + SLOW_WORK_CULL_TIMEOUT));
+}
+
 /*
  * Worker thread culling algorithm
  */
@@ -335,8 +341,7 @@ static bool slow_work_cull_thread(void)
 		    list_empty(&vslow_work_queue) &&
 		    atomic_read(&slow_work_thread_count) >
 		    slow_work_min_threads) {
-			mod_timer(&slow_work_cull_timer,
-				  jiffies + SLOW_WORK_CULL_TIMEOUT);
+			slow_work_defer_cull_time();
 			do_cull = true;
 		}
 	}
@@ -393,8 +398,7 @@ static int slow_work_thread(void *_data)
 			    list_empty(&vslow_work_queue) &&
 			    atomic_read(&slow_work_thread_count) >
 			    slow_work_min_threads)
-				mod_timer(&slow_work_cull_timer,
-					  jiffies + SLOW_WORK_CULL_TIMEOUT);
+				slow_work_defer_cull_time();
 			continue;
 		}
 
@@ -458,7 +462,7 @@ static void slow_work_new_thread_execute
 		if (atomic_dec_and_test(&slow_work_thread_count))
 			BUG(); /* we're running on a slow work thread... */
 		mod_timer(&slow_work_oom_timer,
-			  jiffies + SLOW_WORK_OOM_TIMEOUT);
+				round_jiffies(jiffies + SLOW_WORK_OOM_TIMEOUT));
 	} else {
 		/* ratelimit the starting of new threads */
 		mod_timer(&slow_work_oom_timer, jiffies + 1);
@@ -502,8 +506,7 @@ static int slow_work_min_threads_sysctl(
 			if (n < 0 && !slow_work_may_not_start_new_thread)
 				slow_work_enqueue(&slow_work_new_thread);
 			else if (n > 0)
-				mod_timer(&slow_work_cull_timer,
-					  jiffies + SLOW_WORK_CULL_TIMEOUT);
+				slow_work_defer_cull_time();
 		}
 		mutex_unlock(&slow_work_user_lock);
 	}
@@ -529,8 +532,7 @@ static int slow_work_max_threads_sysctl(
 				atomic_read(&slow_work_thread_count);
 
 			if (n < 0)
-				mod_timer(&slow_work_cull_timer,
-					  jiffies + SLOW_WORK_CULL_TIMEOUT);
+				slow_work_defer_cull_time();
 		}
 		mutex_unlock(&slow_work_user_lock);
 	}
_

Patches currently in -mm which might be from cpeterso@cpeterso.com are

slow-work-use-round_jiffies-for-the-thread-pools-5-second-cull-timer.patch