Re: [PATCH 2.6.27-rc5] Fix itimer/many thread hang.

[Frank Mayhar]

On Wed, 2008-09-10 at 16:12 +0400, Oleg Nesterov wrote:
> On 09/09, Frank Mayhar wrote:
> >
> > On Tue, 2008-09-09 at 20:01 +0400, Oleg Nesterov wrote:
> >
> > > As for this particular function, it seems to me that ->signal == NULL
> > > is not possible, no?
> >
> > That's not completely clear to me.  I'm allowing for the possibility
> > that it might be called during, say, process teardown.  It's used in so
> > many places that I'm uncomfortable leaving the == NULL check out.
> 
> Please see my reply to Roland.

I saw it.  I dunno, I'm more of the "belt-and-suspenders" mindset.  I'll
add a comment, thought, that this is probably a "can't happen" but we
check it anyway.

> > > Btw, this function has a lot of callers, perhaps it is better to
> > > uninline it.
> >
> > If that's the consensus I'll do so.  I assumed that speed was more
> > important than space in this case.  Am I mistaken?
> 
> Are you sure inline will be faster? It has a lot of calllers, think
> about i-cache. And the function call is not that expensive.

Hmm.  It just seems to me that making it inline enables the optimizer to
do smarter things with the flow of control.  The routine isn't all that
long, disassembling the posix_cpu_timers_exit_group() routine gives a
good view of it and it appears to be around 120 bytes.  Certainly less
than 200 bytes (all of posix_cpu_timers_exit_group() is only 201 bytes).
That doesn't seem like a cache buster but I defer to those who are more
familiar with this stuff.

> > > So, the first CLONE_THREAD creates ->cputime.totals. After that
> > > thread_group_cputime_account_xxx() start to use it even if the task
> > > doesn't have the attached cpu timers.
> > >
> > > Stupid question: can't we allocate .totals in posix_cpu_timer_create() /
> > > set_process_cpu_timer() ?
> >
> > That was the original plan but we (that is, Roland and I) decided to
> > eliminate the separate storage for the dead-threads totals.  It's now
> > all kept in the totals field, for the whole thread group.
> 
> I see, thanks.

You're welcome.  I actually like the new method of keeping track of this
stuff; it seems cleaner and certainly removes the requirement of walking
the thread group to add it up.  Should help some microbenchmarks, at
least. :-)
-- 
Frank Mayhar <fmayhar@xxxxxxxxxx>
Google, Inc.

--
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/