Message-ID: <26c376ac-7239-66fe-9c7e-ec99dfb880cd@kernel.dk>
Date:   Sat, 10 Dec 2022 18:58:38 -0700
From:   Jens Axboe <axboe@...nel.dk>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     netdev <netdev@...r.kernel.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>
Subject: Re: [GIT PULL] Add support for epoll min wait time

On 12/10/22 11:51?AM, Linus Torvalds wrote:
> On Sat, Dec 10, 2022 at 7:36 AM Jens Axboe <axboe@...nel.dk> wrote:
>>
>> This adds an epoll_ctl method for setting the minimum wait time for
>> retrieving events.
> 
> So this is something very close to what the TTY layer has had forever,
> and is useful (well... *was* useful) for pretty much the same reason.
> 
> However, let's learn from successful past interfaces: the tty layer
> doesn't have just VTIME, it has VMIN too.
> 
> And I think they very much go hand in hand: you want for at least VMIN
> events or for at most VTIME after the last event.

It has been suggested before too. A more modern example is how IRQ
coalescing works on eg nvme or nics. Those generally are of the nature
of "wait for X time, or until Y events are available". We can certainly
do something like that here too, it's just adding a minevents and
passing them in together.

I'll add that, really should be trivial, and resend later in the merge
window once we're happy with that.

> Yes, yes, you have that 'maxevents' thing, but that's not at all the
> same as VMIN. That's just the buffer size.

Right, the fact that maxevents is all we have means it's not very useful
for anything but "don't write beyond this size of array I have for
events". io_uring has minevents as the general interface, and doesn't
really care about maxevents as it obviously doesn't need to copy it
anywhere.

> Also note that the tty layer VTIME is *different* from what I think
> your "minimum wait time" is. VTIME is a "inter event timer", not a
> "minimum total time". If new events keep on coming, the timer resets -
> until either things time out, or you hit VMIN events.

Right, and I don't think that's what we want here. Some of the hw
coalescing works the same time, basically triggering the timeout once we
have received one event. But that makes it hard to manage the latency,
if your budget is XX usec. Now it becomes YY usec + XX usec instead, if
an event isn't immediatly available.

> I get the feeling that the tty layer did this right, and this epoll
> series did not. The tty model certainly feels more flexible, and does
> have decades of experience. tty traffic *used* to be just about the
> lowest-latency traffic machines handled back when, so I think it might
> be worth looking at as a model.
> 
> So I get the feeling that if you are adding some new "timeout for
> multiple events" model to epoll, you should look at previous users.
> 
> And btw, the tty layer most definitely doesn't handle every possible case.
> 
> There are at least three different valid timeouts:
> 
>  (a) the "final timeout" that epoll already has (ie "in no case wait
> more than this, even if there are no events")
> 
>  (b) the "max time we wait if we have at least one event" (your new "min_wait")
> 
>  (c) the "inter-event timeout" (tty layer VTIME)
> 
> and in addition to the timers, there's that whole "if I have gotten X
> events, I have enough, so stop timing out" (tty layer VMIN).

I do like the VMIN and I think it makes sense. Using VMIN == 0 and VTIME
!= 0 would give you the same behavior it has now, having both be
non-zero would be an OR condition for when to exit.

> And again, that "at least X events" should not be "this is my buffer
> size". You may well want to have a *big* buffer for when there are
> events queued up or the machine is just under very heavy load, but may
> well feel like "if I got N events, I have enough to deal with, and
> don't want to time out for any more".

For epoll, the maxevents already exists for this. Any call should reap
anything up to maxevents, not stop if minevents is met but more events
are available. Only maxevents should terminate the reaping for available
events.

> Now, maybe there is some reason why the tty like VMIN/VTIME just isn't
> relevant, but I do think that people have successfully used VMIN/VTIME
> for long enough that it should be at least given some thought.

Ah it's close enough to thing that are available now. As you mentioned,
there are only really that many different ways to do this if you factor
in a VMIN events as well.

-- 
Jens Axboe

Powered by blists - more mailing lists