Re: [PATCH] pid: use READ_ONCE() in pid_alive()

[David Laight]

On Sun, 4 Oct 2026 15:14:26 +0200
Oleg Nesterov <oleg@xxxxxxxxxx> wrote:

> On 10/04, David Laight wrote:
> >
> > On Sun, 4 Oct 2026 12:55:34 +0200
> > Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
> >  
> > > Yep. That is why do_each_pid_task() needs tasklist_lock.
> > >
> > > This is the known fact, let me quote the part of my old email
> > > https://lore.kernel.org/all/20200512150936.GA28621@xxxxxxxxxx/
> > >  
> > > 	> Currently the tasklist_lock is shared mainly in order to observe
> > > 	> the list atomically for the PRIO_PGRP and PRIO_USER cases, as
> > > 	> the actual lookups are already rcu-safe,  
> > >
> > > 	not really...
> > >
> > > 	do_each_pid_task(PIDTYPE_PGID) can race with change_pid(PIDTYPE_PGID)
> > > 	which moves the task from one hlist to another. Yes, it is safe in
> > > 	that task_struct can't go away. But still this is not right because
> > > 	do_each_pid_task() can scan the wrong (2nd) hlist.
> > >
> > > Somehow I thought this was documented, but it isn't. And this is not obvious.
> > > I think this deserves a comment above do_each_pid_task(), will send the patch.  
> >
> > I guess the rcu protection lets the task exit without holding the lock?
> > Is that really significant given the other things that happen during task exit.  
> 
> Sorry, I don't understand your question...

I was wondering if the (partial) rcu protection of these lists was worth
the trouble.
If the 'add code' all the readers and have to hold the lock then does that
leave anything other than task exit doing an rcu-delete.
I wouldn't have though acquiring the lock in the task exit code would
be noticeable.
Is there some other path where rcu protection is 'good enough'?
 
> 
> > Could do_each_pid_task() use hlist_nulls_for_each_entry_rcu() and rescan
> > if it got the wrong terminator.
> > Or does scanning twice cause grief as well.  
> 
> I don't think it can. Say, __kill_pgrp_info() is a "typical" user of
> do_each_pid_task(). What can it do if it detects that get_nulls_value()
> doesn't match after the main loop? The signal was already sent.

It would have to check each entry to ensure it was on the correct list.
(That probably doesn't need the 'nulls' variant.)
The problem is that the rescan will do things twice.
This is ok for a search, but probably not for sending a signal.

David

> 
> Oleg.
>