netdev - Re: [PATCHSET v3 0/5] Add support for epoll min

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+FuTSfEqmx_rHPLaSp+o+tYzqCvF6oSjSOse0KoFvXj9xK9Cw@mail.gmail.com>
Date:   Sat, 5 Nov 2022 14:05:35 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Jens Axboe <axboe@...nel.dk>
Cc:     Willem de Bruijn <willemdebruijn.kernel@...il.com>,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCHSET v3 0/5] Add support for epoll min_wait

On Sat, Nov 5, 2022 at 1:39 PM Jens Axboe <axboe@...nel.dk> wrote:
>
> >> FWIW, when adding nsec resolution I initially opted for an init-based
> >> approach, passing a new flag to epoll_create1. Feedback then was that
> >> it was odd to have one syscall affect the behavior of another. The
> >> final version just added a new epoll_pwait2 with timespec.
> >
> > I'm fine with just doing a pure syscall variant too, it was my original
> > plan. Only changed it to allow for easier experimentation and adoption,
> > and based on the fact that most use cases would likely use a fixed value
> > per context anyway.
> >
> > I think it'd be a shame to drop the ctl, unless there's strong arguments
> > against it. I'm quite happy to add a syscall variant too, that's not a
> > big deal and would be a minor addition. Patch 6 should probably cut out
> > the ctl addition and leave that for a patch 7, and then a patch 8 for
> > adding a syscall.
> I split the ctl patch out from the core change, and then took a look at
> doing a syscall variant too. But there are a few complications there...
> It would seem to make the most sense to build this on top of the newest
> epoll wait syscall, epoll_pwait2(). But we're already at the max number
> of arguments there...
>
> Arguably pwait2 should've been converted to use some kind of versioned
> struct instead. I'm going to take a stab at pwait3 with that kind of
> interface.

Don't convert to a syscall approach based solely on my feedback. It
would be good to hear from others.

At a high level, I'm somewhat uncomfortable merging two syscalls for
behavior that already works, just to save half the syscall overhead.
There is no shortage of calls that may make some sense for a workload
to merge. Is the quoted 6-7% cpu cycle reduction due to saving one
SYSENTER/SYSEXIT (as the high resolution timer wake-up will be the
same), or am I missing something more fundamental?