On Thu, Jan 30, 2025 at 10:53 PM Lorenzo Stoakes lorenzo.stoakes@oracle.com wrote:
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.
It's also nice to add that people on the libc/allocator side should also appreciate skipping pidfd_open's reliability concerns (mostly, that RLIMIT_NOFILE Should Not(tm) ever affect thread spawning or a malloc[1]). Besides the big syscall reduction and nice speedup, that is.
[1] whether this is the already case is an exercise left to the reader, but at the very least we should not add onto existing problems