Re: [RFC net-next 0/5] Suspend IRQs during preferred busy poll

[Joe Damato <jdamato@fastly.com>]

On Fri, Aug 16, 2024 at 01:01:42PM -0400, Willem de Bruijn wrote:
> Joe Damato wrote:
> > On Fri, Aug 16, 2024 at 10:59:51AM -0400, Willem de Bruijn wrote:
[...]
> > > Willem de Bruijn wrote:
> > > If the only goal is to safely reenable interrupts when the application
> > > stops calling epoll_wait, does this have to be user tunable?
> > > 
> > > Can it be either a single good enough constant, or derived from
> > > another tunable, like busypoll_read.
> > 
> > I believe you meant busy_read here, is that right?
> > 
> > At any rate:
> > 
> >   - I don't think a single constant is appropriate, just as it
> >     wasn't appropriate for the existing mechanism
> >     (napi_defer_hard_irqs/gro_flush_timeout), and
> > 
> >   - Deriving the value from a pre-existing parameter to preserve the
> >     ABI, like busy_read, makes using this more confusing for users
> >     and complicates the API significantly.
> > 
> > I agree we should get the API right from the start; that's why we've
> > submit this as an RFC ;)
> > 
> > We are happy to take suggestions from the community, but, IMHO,
> > re-using an existing parameter for a different purpose only in
> > certain circumstances (if I understand your suggestions) is a much
> > worse choice than adding a new tunable that clearly states its
> > intended singular purpose.
> 
> Ack. I was thinking whether an epoll flag through your new epoll
> ioctl interface to toggle the IRQ suspension (and timer start)
> would be preferable. Because more fine grained.

I understand why you are asking about this and I think it would be
great if this were possible, but unfortunately it isn't.

epoll contexts can be associated with any NAPI ID, but the IRQ
suspension is NAPI specific.

As an example: imagine a user program creates an epoll context and
adds fds with NAPI ID 1111 to the context. It then issues the ioctl
to set the suspend timeout for that context. Then, for whatever
reason, the user app decides to remove all the fds and add new ones,
this time from NAPI ID 2222, which happens to be a different
net_device.

What does that mean for the suspend timeout? It's not clear to me
what the right behavior would be in this situation (does it persist?
does it get cleared when a new NAPI ID is added? etc) and it makes
the user API much more complicated, with many more edge cases and
possible bugs.

> Also, the value is likely dependent more on the expected duration
> of userspace processing? If so, it would be the same for all
> devices, so does a per-netdev value make sense?

There is presently no way to set values like gro_timeout,
defer_hard_irqs, or this new proposed value on a per-NAPI basis.
IMHO, that is really where all of these values should live.

I mentioned on the list previously (and also in the cover letter),
that time permitting, I think the correct evolution of this would be
to support per-NAPI settings (via netdev-genl, I assume) so that
user programs can set all 3 values on only the NAPIs they care
about.

Until that functionality is available, it would seem per-netdev is
the only way for this feature to be added at the present time. I
simply haven't had the time to add the above interface. This
feature we're proposing has demonstrable performance value, but it
doesn't seem sensible to block it until time permits me to add a
per-NAPI interface for all of these values given that we already
globally expose 2 of them.

That said, I appreciate the thoughtfulness of your replies and I am
open to other suggestions.

- Joe