[PATCH 1/3] timer: Optimize fastpath_timer_check()

[PATCH 1/3] timer: Optimize fastpath_timer_check()

[Jason Low]

In fastpath_timer_check(), the task_cputime() function is always
called to compute the utime and stime values. However, this is not
necessary if there are no per-thread timers to check for. This patch
modifies the code such that we compute the task_cputime values only
when there are per-thread timers set.

Signed-off-by: Jason Low <jason.low2@hp.com>
---
 kernel/time/posix-cpu-timers.c |   15 +++++++--------
 1 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
index 892e3da..02596ff 100644
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -1117,16 +1117,15 @@ static inline int task_cputime_expired(const struct task_cputime *sample,
 static inline int fastpath_timer_check(struct task_struct *tsk)
 {
 	struct signal_struct *sig;
-	cputime_t utime, stime;
-
-	task_cputime(tsk, &utime, &stime);
 
 	if (!task_cputime_zero(&tsk->cputime_expires)) {
-		struct task_cputime task_sample = {
-			.utime = utime,
-			.stime = stime,
-			.sum_exec_runtime = tsk->se.sum_exec_runtime
-		};
+		struct task_cputime task_sample;
+		cputime_t utime, stime;
+
+		task_cputime(tsk, &utime, &stime);
+		task_sample.utime = utime;
+		task_sample.stime = stime;
+		task_sample.sum_exec_runtime = tsk->se.sum_exec_runtime;
 
 		if (task_cputime_expired(&task_sample, &tsk->cputime_expires))
 			return 1;
-- 
1.7.2.5

Re: [PATCH 1/3] timer: Optimize fastpath_timer_check()

[George Spelvin]

> 	if (!task_cputime_zero(&tsk->cputime_expires)) {
>+		struct task_cputime task_sample;
>+		cputime_t utime, stime;
>+
>+		task_cputime(tsk, &utime, &stime);
>+		task_sample.utime = utime;
>+		task_sample.stime = stime;
>+		task_sample.sum_exec_runtime = tsk->se.sum_exec_runtime;

Er, task_sample.[us]time are already the correct types.
Whay are the local variables necessary?  How about:

 	if (!task_cputime_zero(&tsk->cputime_expires)) {
+		struct task_cputime task_sample;
+
+		task_cputime(tsk, &task_simple.utime, &task_simple.stime);
+		task_sample.sum_exec_runtime = tsk->se.sum_exec_runtime;

Re: [PATCH 1/3] timer: Optimize fastpath_timer_check()

[Jason Low]

On Wed, 2015-08-26 at 12:57 -0400, George Spelvin wrote:
> > 	if (!task_cputime_zero(&tsk->cputime_expires)) {
> >+		struct task_cputime task_sample;
> >+		cputime_t utime, stime;
> >+
> >+		task_cputime(tsk, &utime, &stime);
> >+		task_sample.utime = utime;
> >+		task_sample.stime = stime;
> >+		task_sample.sum_exec_runtime = tsk->se.sum_exec_runtime;
> 
> Er, task_sample.[us]time are already the correct types.
> Whay are the local variables necessary?  How about:
> 
>  	if (!task_cputime_zero(&tsk->cputime_expires)) {
> +		struct task_cputime task_sample;
> +
> +		task_cputime(tsk, &task_simple.utime, &task_simple.stime);
> +		task_sample.sum_exec_runtime = tsk->se.sum_exec_runtime;

Yes, good point. Now that we're moving the task_cputime() call to after
the task_sample structure is declared, the utime and stime local
variables are not required anymore.

Re: [PATCH 1/3] timer: Optimize fastpath_timer_check()

[Frederic Weisbecker]

On Tue, Aug 25, 2015 at 08:17:46PM -0700, Jason Low wrote:
> In fastpath_timer_check(), the task_cputime() function is always
> called to compute the utime and stime values. However, this is not
> necessary if there are no per-thread timers to check for. This patch
> modifies the code such that we compute the task_cputime values only
> when there are per-thread timers set.
> 
> Signed-off-by: Jason Low <jason.low2@hp.com>

Reviewed-by: Frederic Weisbecker <fweisbec@gmail.com>

Thanks.

> ---
>  kernel/time/posix-cpu-timers.c |   15 +++++++--------
>  1 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
> index 892e3da..02596ff 100644
> --- a/kernel/time/posix-cpu-timers.c
> +++ b/kernel/time/posix-cpu-timers.c
> @@ -1117,16 +1117,15 @@ static inline int task_cputime_expired(const struct task_cputime *sample,
>  static inline int fastpath_timer_check(struct task_struct *tsk)
>  {
>  	struct signal_struct *sig;
> -	cputime_t utime, stime;
> -
> -	task_cputime(tsk, &utime, &stime);
>  
>  	if (!task_cputime_zero(&tsk->cputime_expires)) {
> -		struct task_cputime task_sample = {
> -			.utime = utime,
> -			.stime = stime,
> -			.sum_exec_runtime = tsk->se.sum_exec_runtime
> -		};
> +		struct task_cputime task_sample;
> +		cputime_t utime, stime;
> +
> +		task_cputime(tsk, &utime, &stime);
> +		task_sample.utime = utime;
> +		task_sample.stime = stime;
> +		task_sample.sum_exec_runtime = tsk->se.sum_exec_runtime;
>  
>  		if (task_cputime_expired(&task_sample, &tsk->cputime_expires))
>  			return 1;
> -- 
> 1.7.2.5
> 

Re: [PATCH 1/3] timer: Optimize fastpath_timer_check()

[Davidlohr Bueso]

On Tue, 2015-08-25 at 20:17 -0700, Jason Low wrote:
> In fastpath_timer_check(), the task_cputime() function is always
> called to compute the utime and stime values. However, this is not
> necessary if there are no per-thread timers to check for. This patch
> modifies the code such that we compute the task_cputime values only
> when there are per-thread timers set.
> 
> Signed-off-by: Jason Low <jason.low2@hp.com>

Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>

Re: [PATCH 1/3] timer: Optimize fastpath_timer_check()

[Jason Low]

On Mon, 2015-08-31 at 08:15 -0700, Davidlohr Bueso wrote:
> On Tue, 2015-08-25 at 20:17 -0700, Jason Low wrote:
> > In fastpath_timer_check(), the task_cputime() function is always
> > called to compute the utime and stime values. However, this is not
> > necessary if there are no per-thread timers to check for. This patch
> > modifies the code such that we compute the task_cputime values only
> > when there are per-thread timers set.
> > 
> > Signed-off-by: Jason Low <jason.low2@hp.com>
> 
> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>

Thanks David and Frederic.

I want to mention that this patch has been slightly changed. As
suggested by George, those extra utime, stime local variables are not
needed anymore, and we should be able to directly call:

task_cputime(tsk, &task_sample.utime, &task_sample.stime);

---
diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
index 892e3da..3a8c5b4 100644
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -1117,16 +1117,13 @@ static inline int task_cputime_expired(const struct task_cputime *sample,
 static inline int fastpath_timer_check(struct task_struct *tsk)
 {
        struct signal_struct *sig;
-       cputime_t utime, stime;
-
-       task_cputime(tsk, &utime, &stime);

        if (!task_cputime_zero(&tsk->cputime_expires)) {
-               struct task_cputime task_sample = {
-                       .utime = utime,
-                       .stime = stime,
-                       .sum_exec_runtime = tsk->se.sum_exec_runtime
-               };
+               struct task_cputime task_sample;
+
+               task_cputime(tsk, &task_sample.utime, &task_sample.stime);
+
+               task_sample.sum_exec_runtime = tsk->se.sum_exec_runtime;

                if (task_cputime_expired(&task_sample, &tsk->cputime_expires))
                        return 1;