Re: [PATCH 1/7] posix-cpu-timers: don't account cpu timer afterstopped thread runtime accounting

[Olivier Langlois]

On Tue, 2013-05-07 at 01:47 +0200, Frederic Weisbecker wrote:
> On Fri, May 03, 2013 at 12:47:42AM -0400, kosaki.motohiro@xxxxxxxxx wrote:
> > From: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx>
> > 
> > When tsk->signal->cputimer->running is 1, signal->cputimer and
> > tsk->sum_sched_runtime increase at the same pace because update_curr()
> > increases both accounting.
> > 
> > However, there is one exception. When thread exiting, __exit_signal() turns
> > over task's sum_shced_runtime to sig->sum_sched_runtime, but it doesn't stop
> > signal->cputimer accounting.
> > 
> > This inconsistency makes POSIX timer wake up too early. This patch fixes it.
> > 
> > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> > Cc: Frederic Weisbecker <fweisbec@xxxxxxxxx>
> > Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> > Acked-by: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> > Signed-off-by: Olivier Langlois <olivier@xxxxxxxxxxxxxx>
> > Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx>
> > ---
> >  kernel/sched/stats.h |    7 +++++++
> >  1 files changed, 7 insertions(+), 0 deletions(-)
> > 
> > diff --git a/kernel/sched/stats.h b/kernel/sched/stats.h
> > index 2ef90a5..5a0cfc4 100644
> > --- a/kernel/sched/stats.h
> > +++ b/kernel/sched/stats.h
> > @@ -225,6 +225,13 @@ static inline void account_group_exec_runtime(struct task_struct *tsk,
> >       if (!cputimer->running)
> >               return;
> >  
> > +     /*
> > +      * After turning over se.sum_exec_runtime to sig->sum_sched_runtime
> > +      * in __exit_signal(), we must not account exec_runtime for consistency.
> > +      */
> > +     if (unlikely(!tsk->sighand))
> > +             return;
> 
> Ok, if we want the clock and timer to be consistent, do we also want the same check in
> account_group_user_time() and account_group_system_time()? The task can still account
> a tick after autoreaping itself between release_task() and the final schedule().
> 
If I can reply, I believe that Kosaki refactoring is superior to my
initial proposal because:

1. Condition will only be evaluated when cputimer is on (and not slow
down scheduler when it is off)
2. It handles all the cases when this function is called. There are few
other places outside update_curr() that this function is called.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/