Re: [PATCH 2/2] taskstats: add_del_listener() should ignore !valid listener's

[Oleg Nesterov <oleg@redhat.com>]

On 07/20, Vasiliy Kulikov wrote:
>
> On Wed, Jul 20, 2011 at 21:00 +0200, Oleg Nesterov wrote:
> > When send_cpu_listeners() finds the orphaned listener it marks
> > it as !valid and drops listeners->sem. Before it takes this sem
> > for wrinting, s->pid can be reused and add_del_listener() can
> > wrongly try to re-use this entry.
> >
> > Change add_del_listener() to check ->valid = T.
>
> I think the current logic is wrong.

me too ;)

still I think this patch makes sense anyway.

> If s was claimed is invalid, it
> then points to a dead task,

I forgot everything I knew about netlink. But, it seems, netlink_bind()
can use nl_pid != current->pid. But even if this is not true,

> it is unknown when it died.  As the same pid
> can be reused by a new process BEFORE add_del_listener(), it is unknown
> whether ->pid points to the actual owner task.

And? Why should we try to reuse the !valid entries? struct listener
is just the "addr" we should send the message to. If someone calls
add_listener() it should works regardless of dead entries with the
same ->pid.

> Rather than optimizing the wrong algorithm it's better to change the
> processes keeping way.

Not sure I understand... This is not optimizing, the patch tries to
fix the unlikely (in fact mostly theoretical) and minor problem. Still
this is the problem, no?

Thanks,

Oleg.