On Thu, 30 Jan 2025 23:10:53 +0000 Pedro Falcato [off-list ref] wrote:

On Thu, Jan 30, 2025 at 10:53 PM Lorenzo Stoakes
[off-list ref] wrote:

quoted

quoted

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

Thanks.

Could we please get all the above spelled out much more thoroughly in
the [0/n] description (aka Patch Series Sales Brochure)?