| Message ID | 20241102005214.32443-6-jdamato@fastly.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Suspend IRQs during application busy periods | expand |
On 11/1/2024 7:52 PM, Joe Damato wrote: > From: Martin Karsten <mkarsten@uwaterloo.ca> > > When events are reported to userland and prefer_busy_poll is set, irqs > are temporarily suspended using napi_suspend_irqs. > > If no events are found and ep_poll would go to sleep, irq suspension is > cancelled using napi_resume_irqs. > > Signed-off-by: Martin Karsten <mkarsten@uwaterloo.ca> > Co-developed-by: Joe Damato <jdamato@fastly.com> > Signed-off-by: Joe Damato <jdamato@fastly.com> > Tested-by: Joe Damato <jdamato@fastly.com> > Tested-by: Martin Karsten <mkarsten@uwaterloo.ca> > Acked-by: Stanislav Fomichev <sdf@fomichev.me> > --- Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
On Sat, 2 Nov 2024 00:52:01 +0000 Martin Karsten <mkarsten@uwaterloo.ca> > > @@ -2005,8 +2032,10 @@ static int ep_poll(struct eventpoll *ep, struct epoll_event __user *events, > * trying again in search of more luck. > */ > res = ep_send_events(ep, events, maxevents); > - if (res) > + if (res) { > + ep_suspend_napi_irqs(ep); Leave napi irq intact in case of -EINTR. > return res; > + } > } > > if (timed_out) > -- > 2.25.1
On Sun, Nov 03, 2024 at 07:39:25AM +0800, Hillf Danton wrote: > On Sat, 2 Nov 2024 00:52:01 +0000 Martin Karsten <mkarsten@uwaterloo.ca> > > > > @@ -2005,8 +2032,10 @@ static int ep_poll(struct eventpoll *ep, struct epoll_event __user *events, > > * trying again in search of more luck. > > */ > > res = ep_send_events(ep, events, maxevents); > > - if (res) > > + if (res) { > > + ep_suspend_napi_irqs(ep); > > Leave napi irq intact in case of -EINTR. (I've added Martin and Sridhar to the CC list) Thanks for pointing out this inconsistency. It's not a big problem, because on receiving EINTR or another error code, the app either retries epoll_wait or, if the app is buggy somehow and doesn't retry epoll_wait, the timer fires and everything proceeds as normal. However, since irqs are not suspended at other points in ep_poll where an error code is returned, it is probably best to be consistent and not suspend here either. We will fix this in the next revision. Sridhar: Since the change is very minor I plan to retain your Reviewed-by, but if you'd like me to drop it, please let me know. The proposed fix will look like: if (res) { if (res > 0) ep_suspend_napi_irqs(ep); return res; } > > > return res; > > + } > > } > > > > if (timed_out) > > -- > > 2.25.1
diff --git a/fs/eventpoll.c b/fs/eventpoll.c index f9e0d9307dad..36a657594352 100644 --- a/fs/eventpoll.c +++ b/fs/eventpoll.c @@ -457,6 +457,8 @@ static bool ep_busy_loop(struct eventpoll *ep, int nonblock) * it back in when we have moved a socket with a valid NAPI * ID onto the ready list. */ + if (prefer_busy_poll) + napi_resume_irqs(napi_id); ep->napi_id = 0; return false; } @@ -540,6 +542,22 @@ static long ep_eventpoll_bp_ioctl(struct file *file, unsigned int cmd, } } +static void ep_suspend_napi_irqs(struct eventpoll *ep) +{ + unsigned int napi_id = READ_ONCE(ep->napi_id); + + if (napi_id >= MIN_NAPI_ID && READ_ONCE(ep->prefer_busy_poll)) + napi_suspend_irqs(napi_id); +} + +static void ep_resume_napi_irqs(struct eventpoll *ep) +{ + unsigned int napi_id = READ_ONCE(ep->napi_id); + + if (napi_id >= MIN_NAPI_ID && READ_ONCE(ep->prefer_busy_poll)) + napi_resume_irqs(napi_id); +} + #else static inline bool ep_busy_loop(struct eventpoll *ep, int nonblock) @@ -557,6 +575,14 @@ static long ep_eventpoll_bp_ioctl(struct file *file, unsigned int cmd, return -EOPNOTSUPP; } +static void ep_suspend_napi_irqs(struct eventpoll *ep) +{ +} + +static void ep_resume_napi_irqs(struct eventpoll *ep) +{ +} + #endif /* CONFIG_NET_RX_BUSY_POLL */ /* @@ -788,6 +814,7 @@ static bool ep_refcount_dec_and_test(struct eventpoll *ep) static void ep_free(struct eventpoll *ep) { + ep_resume_napi_irqs(ep); mutex_destroy(&ep->mtx); free_uid(ep->user); wakeup_source_unregister(ep->ws); @@ -2005,8 +2032,10 @@ static int ep_poll(struct eventpoll *ep, struct epoll_event __user *events, * trying again in search of more luck. */ res = ep_send_events(ep, events, maxevents); - if (res) + if (res) { + ep_suspend_napi_irqs(ep); return res; + } } if (timed_out)