On Thu, Jan 30, 2025 at 02:37:54PM -0800, Andrew Morton wrote:
> On Thu, 30 Jan 2025 20:40:25 +0000 Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> wrote:
>
> > If you wish to utilise a pidfd interface to refer to the current process or
> > thread it is rather cumbersome, requiring something like:
> >
> > int pidfd = pidfd_open(getpid(), 0 or PIDFD_THREAD);
> >
> > ...
> >
> > close(pidfd);
> >
> > Or the equivalent call opening /proc/self. It is more convenient to use a
> > sentinel value to indicate to an interface that accepts a pidfd that we
> > simply wish to refer to the current process thread.
> >
>
> The above code sequence doesn't seem at all onerous. I'm not
> understanding why it's worth altering the kernel to permit this little
> shortcut?
In practice it adds quite a bit of overhead for something that whatever
mechanism is using the pidfd can avoid.
It was specifically intended for a real case of utilising
process_madvise(), using the newly extended ability to batch _any_
madvise() operations for the current process, like:
if (process_madvise(PIDFD_SELF, iovec, 10, MADV_GUARD_INSTALL, 0)) {
... error handling ...
}
vs.
pid_t pid = getpid();
int pidfd = pidfd_open(pid, PIDFD_THREAD);
if (pidfd < 0) {
... error handling ...
}
if (process_madvise(PIDFD_SELF, iovec, 10, MADV_GUARD_INSTALL, 0)) {
... cleanup pidfd ...
... error handling ...
}
...
... cleanup pidfd ...
So in practice, it's actually a lot more ceremony and noise. Suren has been
working with this code in practice and found this to be useful.
The suggestion to embed it as PIDFD_SELF rather than to pass it as a
process_madvise() flag was made on the original series where I extended its
functionality.
So in practice I think it's onerous enough to justify this, plus it allows
for a more fluent use of pidfd's in other cases where one is referring to
the same process/thread, to the extent that I've seen people commenting on
supporting it while sending series relating to pidfd.
Also Christian and others appear to support this idea.
