Re: [PATCH] fs, proc: Introduce the /proc/<pid>/children entry v2

[Cyrill Gorcunov]

On Fri, Dec 09, 2011 at 04:30:09PM +0100, Oleg Nesterov wrote:
...
> >
> > It is a potential problem, from the lock-hold point of view and
> > also it can cause large scheduling latencies.  What's involved in
> > making ->children an rcu-protected list?
> 
> At first glance, this doesn't look trivial... forget_original_parent()
> abuses ->sibling.
> 

But wait, forget_original_parent may move task out of the original
list and put it in dead_children list, right? Then release_task
may call delayed_put_task_struct under as call_rcu which should
wait until we finish reading in our rcu section, isn't it?

Even if we get inconsistent picture of children (ie we get pid
of shildren which just getting dead) I think it's fine.

> But yes, it is not really nice to hold tasklist_lock here. May be
> we can change this code so that every iteration records the reported
> task_struct and then tries to continue. This means we should verify
> that ->real_parent is still the same under tasklist, but at least
> this way we do not hold it throughout.
> 

And if real_parent is changed, I should simply skip such task and
continue, right?

> > If so, I guess the feature could/should be configurable.  Probably with
> > a CONFIG_PROC_CHILDREN which is selected by CONFIG_CHECKPOINT_RESTORE.
> > Which is all getting a bit over the top, but I suppose we must do it.
> 
> Heh. This is the rare case when I personally like the new feature ;)
> I mean, it looks "obviously useful" to me. If nothing else, it can
> help to debug the problems. Probably the tools like pstree can use it.
> 

Well, I'm personally fine with either approach. We could make it
CONFIG_CHECKPOINT_RESTORE dependant and make CONFIG_CHECKPOINT_RESTORE
being Y on default, then if someone not needed this facility he simply
turn it off, no?

> Personally I'd even prefer /proc/pid/children/ directory (like
> /proc/pid/task), but I guess this needs much more complications.
> 

I fear so. Oleg, if it possible I would like to bring in as minimum
code as I can ;)

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