linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Eric W. Biederman" <ebiederm@xmission.com>
To: Mateusz Guzik <mjguzik@gmail.com>
Cc: brauner@kernel.org,  oleg@redhat.com,  akpm@linux-foundation.org,
	linux-mm@kvack.org,  linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] exit: perform randomness and pid work without tasklist_lock
Date: Fri, 31 Jan 2025 14:55:47 -0600	[thread overview]
Message-ID: <87ikpubt4c.fsf@email.froward.int.ebiederm.org> (raw)
In-Reply-To: <20250128160743.3142544-1-mjguzik@gmail.com> (Mateusz Guzik's message of "Tue, 28 Jan 2025 17:07:43 +0100")


Oleg asked that I take a look, and I took one.

I very much agree with Oleg that this should be one patch per thing
you want to effect as the issues can be intricate in this part of
the code.

Moving proc_flush_pid inside of tasklist_lock is a bad idea.  The code
has previously been several kinds of a sore spot.  If you look at
proc_invalidate_siblings_dcache you can see calls to d_invalidate,
deactivate_super, and a few other vfs calls that could potentially do
quite a lot of work and potentially take a number of locks.  It has been
a long time but I remember when we used to flush the proc entries under
the tasklist_lock that there were actual deadlocks caused by some rare
code paths that were trying to free memory to allocate memory to make
progress.

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.

There are two more layers to pids.  The pid number allocation of
alloc_pid/free_pid, and the struct pid layer maintained by get_pid,
put_pid.  Those two layers don't need the tasklist_lock.


It is safe to move free_pid out of tasklist_lock.  I am not certain
how sane it is.

Eric


  parent reply	other threads:[~2025-01-31 20:57 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-28 16:07 Mateusz Guzik
2025-01-28 18:29 ` Oleg Nesterov
2025-01-28 18:38   ` Mateusz Guzik
2025-01-28 19:22     ` Oleg Nesterov
2025-01-30 11:01       ` Mateusz Guzik
2025-01-31 20:55 ` Eric W. Biederman [this message]
2025-01-31 22:31   ` Mateusz Guzik
2025-01-31 23:09     ` Eric W. Biederman
2025-02-01 14:03       ` Mateusz Guzik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ikpubt4c.fsf@email.froward.int.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=akpm@linux-foundation.org \
    --cc=brauner@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mjguzik@gmail.com \
    --cc=oleg@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox