Message-ID: <e3c55fda-043e-9cc7-3248-63574b374c13@kernel.dk>
Date:   Sat, 5 Nov 2022 12:46:03 -0600
From:   Jens Axboe <axboe@...nel.dk>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc:     linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCHSET v3 0/5] Add support for epoll min_wait

On 11/5/22 12:05 PM, Willem de Bruijn wrote:
> 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.

It's not just based on your feedback, if you read the original cover
letter, then that is the question that is posed in terms of API - ctl to
modify it, new syscall, or both? So figured I should at least try and
see what the syscall would look like.

> 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?

No, it's not really related to saving a single syscall, and you'd
potentially save more than just one as well. If we look at the two
extremes of applications, one will be low load and you're handling
probably just 1 event per loop. Not really interesting. At the other
end, you're fully loaded, and by the time you check for events, you have
'maxevents' (or close to) available. That obviously reduces system
calls, but more importantly, it also allows the application to get some
batching effects from processing these events.

In the medium range, there's enough processing to react pretty quickly
to events coming in, and you then end up doing just 1 event (or close to
that). To overcome that, we have some applications that detect this
medium range and do an artificial sleep before calling epoll_wait().
That was a nice effiency win for them. But we can do this a lot more
efficiently in the kernel. That was the idea behind this, and the
initial results from TAO (which does that sleep hack) proved it to be
more than worthwhile. Syscall reduction is one thing, improved batching
another, and just as importanly is sleep+wakeup reductions.

-- 
Jens Axboe

Powered by blists - more mailing lists