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@oracle.com 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.