Re: [patch 3/4] taskstats: Introduce cdata_acct for complete cumulative accounting

[Oleg Nesterov <oleg@redhat.com>]

On 11/25, Michael Holzheu wrote:
>
> Hello Oleg,
>
> On Tue, 2010-11-23 at 17:59 +0100, Oleg Nesterov wrote:
> > On 11/19, Michael Holzheu wrote:
> > > TODO:
> > > -----
> > > With this patch we take the siglock twice. First for the dead task
> > > and second for the parent of the dead task. This give the following
> > > lockdep warning (probably a lockdep annotation is needed here):
> >
> > And we already discussed this ;) We do not need 2 siglock's, only
> > parent's. Just move the callsite in __exit_signal() down, under
> > another (lockless) group_dead check.
> >
> > Or I missed something?
>
> The problem with moving this down to the second group_dead check is that
> after __unhash_process() is called, pid_alive(tsk) which is checked in
> thread_group_cputime() returns false. Therefore we always get zero CPU
> times.

I see, thanks.

> So I probably have to introduce a second group_dead check at the
> beginning of __exit_signal():

Probably...

But in fact this reminds we should cleanup this code somehow.
By the time we call thread_group_times() there are no other
threads.

> My personal feeling is that probably the only acceptable thing would be
> to make the new behavior configurable with a sysctl and define the
> default as it currently is (POSIX compliant).
>
> This would only introduce two additional checks in __exit_signal() and
> wait_task_zombie() and would not add any new fields to the
> signal_struct.

Yeah, it would be nice to avoid new fields.


Hmm. Somehow I forgot about 4/4, please see another email...

Oleg.