Re: [PATCH v2] exit: perform randomness and pid work without tasklist_lock

[Mateusz Guzik]

On Sat, Feb 1, 2025 at 12:09 AM Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote:
>
> Mateusz Guzik <mjguzik@xxxxxxxxx> writes:
>
> > On Fri, Jan 31, 2025 at 9:56 PM Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote:
> >> Moving proc_flush_pid inside of tasklist_lock is a bad idea.
> >
> > The patch does not make such a change though.
> >
> > The call is still performed without the lock, but it also dodges the
> > additional refcount dance (and notably eliminates an atomic from an
> > area protected by tasklist_lock).
>
> My mistake I saw you had moved it up, but I had missed just how
> far.
>
> It is still a bad idea to move it early, as that has caused problems
> with lingering proc entries for reaped task clogging up the dcache.
>

I would argue the time window to find the about-to-be-whacked task is
not big, but this part is not important enough for me to push for it.
So I'm going to drop this bit for now.

> >> It is wrong that attach_pid/detach_pid can be performed without the
> >> tasklist_lock.  There are reasonable guarantees provided by the posix
> >> standard that the set of processes sent a signal is the set of
> >> processes at a point in time.  The tasklist_lock is how we provide
> >> those guarantees currently.
> >>
> >
> > I don't see anything calling these without the lock and neither my
> > patch nor a follow up about pids suggest anything of the sort.
>
> No.  You simply said fork called attach_pid without the lock and
> your description implied it was safe to call attach_pid/detach_pid
> without the lock.
>

Huh, indeed that's how it reads like. That's very poorly stated at best, my bad.

The key was *allocating* a pid happens without the tasklist_lock (but
with pidmap_lock) and the part which gets rid of it (detach_pid ->
free_pid) operates under both.

As you can see the patch keeps detach_pid inside the
tasklist_lock-protected area.

> >> It is safe to move free_pid out of tasklist_lock.  I am not certain
> >> how sane it is.
> >>
> >
> > Where is the sanity problem here? AFAICS this just delays some wakeups
> > in the worst case.
>
> At the end of the day it might be a sensible way to go.  I just haven't
> found the arguments to convince my gut of that yet.  It is important for
> me at least to convince my gut because it usually notices problems
> before the rest of me does.
>

There is definitely no rush.

I'm going to cook v3 if only just for fun.
-- 
Mateusz Guzik <mjguzik gmail.com>