Re: [RFC][PATCH 09/10] taskstats: Fix exit CPU time accounting

[Martin Schwidefsky <schwidefsky@de.ibm.com>]

On Tue, 28 Sep 2010 14:53:55 +0530
Balbir Singh <balbir@linux.vnet.ibm.com> wrote:

> * Martin Schwidefsky <schwidefsky@de.ibm.com> [2010-09-28 11:08:28]:
> 
> > On Tue, 28 Sep 2010 14:06:02 +0530
> > Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> > 
> > > * Martin Schwidefsky <schwidefsky@de.ibm.com> [2010-09-27 15:42:57]:
> > > > Got the part about self-reaping processes. But there is another issue:
> > > > consider an exiting thread where the group leader is still active.
> > > > The time for the thread will be added to the utime/stime fields in
> > > > the signal structure. Taskstats will happily ignore that time while
> > > > the group leader is still running.
> > > >
> > > 
> > > Why do you say that? Not sure your comment is very clean, in
> > > fill_tgid, we do
> > > 
> > > 1. Accumulate signal stats (contains stats for dead threads)
> > > 2. Accumulate stats for current threads
> > > 
> > > fill_tgid_exit does something similar
> > 
> > Hmm, I can't find anything in the code where the tsk->signal->{utime,stime}
> 
> That is left to the actual subsystem (I should have been clearer in
> stating that the limitation is not with the taskstats infrastructure
> itself). Yes, your observation is indeed correct.
> 
> taskstats code is expected to contain the callback for the subsystems
> it supports. delayacct() already does the right thing today, AFAICS
> 
> > gets transferred to the taskstats record. There is a loop in fill_tgid over
> > the threads of the process but all it does is to call delayacct_add_tsk.
> > And that function does nothing with the cpu time of dead threads which is
> > stored in the signal structure. In addition which taskstats field is
> > supposed to contain the cpu time of the dead thread, ac_utime/ac_stime?
> >
> 
> I've not focused much on ac_*, The changes that need to occur are we
> need to get the tsacct.c callbacks into taskstats.

Ok, so tsacct.c is the right place to implement it. We will need new fields
in struct taskstats to contain the tsk->signal->{utime,stime} and some code
in bacct_add_tsk to do the copy. Agreed ?

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.