| Message ID | 20230523111518.21512-1-kurt@linutronix.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c857946a4e262e0f798fe7625fadf85bf1190fc4 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net/core: Enable socket busy polling on -RT | expand |
Hi, Tue, 2023-05-23 at 13:15 +0200, Kurt Kanzenbach wrote: > Busy polling is currently not allowed on PREEMPT_RT, because it disables > preemption while invoking the NAPI callback. It is not possible to acquire > sleeping locks with disabled preemption. For details see commit > 20ab39d13e2e ("net/core: disable NET_RX_BUSY_POLL on PREEMPT_RT"). > > However, strict cyclic and/or low latency network applications may prefer busy > polling e.g., using AF_XDP instead of interrupt driven communication. > > The preempt_disable() is used in order to prevent the poll_owner and NAPI owner > to be preempted while owning the resource to ensure progress. Netpoll performs > busy polling in order to acquire the lock. NAPI is locked by setting the > NAPIF_STATE_SCHED flag. There is no busy polling if the flag is set and the > "owner" is preempted. Worst case is that the task owning NAPI gets preempted and > NAPI processing stalls. This is can be prevented by properly prioritising the > tasks within the system. > > Allow RX_BUSY_POLL on PREEMPT_RT if NETPOLL is disabled. Don't disable > preemption on PREEMPT_RT within the busy poll loop. > > Tested on x86 hardware with v6.1-RT and v6.3-RT on Intel i225 (igc) with > AF_XDP/ZC sockets configured to run in busy polling mode. > > Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de> The patch looks reasonable to me, but it would be great to hear a second opinion from someone from RT side. CC: Juri Thanks! Paolo
On 2023-05-25 13:16:46 [+0200], Paolo Abeni wrote: > Hi, Hi Paolo, > > Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > > Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de> > > The patch looks reasonable to me, but it would be great to hear a > second opinion from someone from RT side. I suggested and reviewed this and crafted parts of the commit message before Kurt sent it. Is this enough or do you look for someone in particular? > CC: Juri > > > Thanks! > > Paolo Sebastian
Hello: This patch was applied to netdev/net-next.git (main) by David S. Miller <davem@davemloft.net>: On Tue, 23 May 2023 13:15:18 +0200 you wrote: > Busy polling is currently not allowed on PREEMPT_RT, because it disables > preemption while invoking the NAPI callback. It is not possible to acquire > sleeping locks with disabled preemption. For details see commit > 20ab39d13e2e ("net/core: disable NET_RX_BUSY_POLL on PREEMPT_RT"). > > However, strict cyclic and/or low latency network applications may prefer busy > polling e.g., using AF_XDP instead of interrupt driven communication. > > [...] Here is the summary with links: - [net-next] net/core: Enable socket busy polling on -RT https://git.kernel.org/netdev/net-next/c/c857946a4e26 You are awesome, thank you!
Hi Kurt, On Tue, 2023-05-23 at 13:15 +0200, Kurt Kanzenbach wrote: > Busy polling is currently not allowed on PREEMPT_RT, because it disables > preemption while invoking the NAPI callback. It is not possible to acquire > sleeping locks with disabled preemption. For details see commit > 20ab39d13e2e ("net/core: disable NET_RX_BUSY_POLL on PREEMPT_RT"). Is that something that we could consider as Bug-Fix for 6.1 and request a backport, or would you consider that as new feature? > > However, strict cyclic and/or low latency network applications may prefer busy > polling e.g., using AF_XDP instead of interrupt driven communication. > > The preempt_disable() is used in order to prevent the poll_owner and NAPI owner > to be preempted while owning the resource to ensure progress. Netpoll performs > busy polling in order to acquire the lock. NAPI is locked by setting the > NAPIF_STATE_SCHED flag. There is no busy polling if the flag is set and the > "owner" is preempted. Worst case is that the task owning NAPI gets preempted and > NAPI processing stalls. This is can be prevented by properly prioritising the > tasks within the system. > > Allow RX_BUSY_POLL on PREEMPT_RT if NETPOLL is disabled. Don't disable > preemption on PREEMPT_RT within the busy poll loop. > > Tested on x86 hardware with v6.1-RT and v6.3-RT on Intel i225 (igc) with > AF_XDP/ZC sockets configured to run in busy polling mode. That is exactly our use case as well and we would like to have it in 6.1. Any (technical) reasons that prevent a backport? As some time has already passed since patch submission I will not cut the rest... Best regards, Florian > > Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de> > --- > > Changes since RFC: > > * Commit message > > Previous version: > > * https://lore.kernel.org/all/20230517110950.78322-1-kurt@linutronix.de/ > > net/Kconfig | 2 +- > net/core/dev.c | 9 ++++++--- > 2 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/net/Kconfig b/net/Kconfig > index 7d39c1773eb4..2fb25b534df5 100644 > --- a/net/Kconfig > +++ b/net/Kconfig > @@ -324,7 +324,7 @@ config CGROUP_NET_CLASSID > > config NET_RX_BUSY_POLL > bool > - default y if !PREEMPT_RT > + default y if !PREEMPT_RT || (PREEMPT_RT && !NETCONSOLE) > > config BQL > bool > diff --git a/net/core/dev.c b/net/core/dev.c > index b3c13e041935..3393c2f3dbe8 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -6197,7 +6197,8 @@ void napi_busy_loop(unsigned int napi_id, > if (!napi) > goto out; > > - preempt_disable(); > + if (!IS_ENABLED(CONFIG_PREEMPT_RT)) > + preempt_disable(); > for (;;) { > int work = 0; > > @@ -6239,7 +6240,8 @@ void napi_busy_loop(unsigned int napi_id, > if (unlikely(need_resched())) { > if (napi_poll) > busy_poll_stop(napi, have_poll_lock, prefer_busy_poll, budget); > - preempt_enable(); > + if (!IS_ENABLED(CONFIG_PREEMPT_RT)) > + preempt_enable(); > rcu_read_unlock(); > cond_resched(); > if (loop_end(loop_end_arg, start_time)) > @@ -6250,7 +6252,8 @@ void napi_busy_loop(unsigned int napi_id, > } > if (napi_poll) > busy_poll_stop(napi, have_poll_lock, prefer_busy_poll, budget); > - preempt_enable(); > + if (!IS_ENABLED(CONFIG_PREEMPT_RT)) > + preempt_enable(); > out: > rcu_read_unlock(); > } > -- > 2.30.2 >
Hi Florian, On Fri Oct 27 2023, Florian Bezdeka wrote: > On Tue, 2023-05-23 at 13:15 +0200, Kurt Kanzenbach wrote: >> Busy polling is currently not allowed on PREEMPT_RT, because it disables >> preemption while invoking the NAPI callback. It is not possible to acquire >> sleeping locks with disabled preemption. For details see commit >> 20ab39d13e2e ("net/core: disable NET_RX_BUSY_POLL on PREEMPT_RT"). > > Is that something that we could consider as Bug-Fix for 6.1 and request > a backport, or would you consider that as new feature? IMO it is in category "never worked". Hence it is not stable material. > >> >> However, strict cyclic and/or low latency network applications may prefer busy >> polling e.g., using AF_XDP instead of interrupt driven communication. >> >> The preempt_disable() is used in order to prevent the poll_owner and NAPI owner >> to be preempted while owning the resource to ensure progress. Netpoll performs >> busy polling in order to acquire the lock. NAPI is locked by setting the >> NAPIF_STATE_SCHED flag. There is no busy polling if the flag is set and the >> "owner" is preempted. Worst case is that the task owning NAPI gets preempted and >> NAPI processing stalls. This is can be prevented by properly prioritising the >> tasks within the system. >> >> Allow RX_BUSY_POLL on PREEMPT_RT if NETPOLL is disabled. Don't disable >> preemption on PREEMPT_RT within the busy poll loop. >> >> Tested on x86 hardware with v6.1-RT and v6.3-RT on Intel i225 (igc) with >> AF_XDP/ZC sockets configured to run in busy polling mode. > > That is exactly our use case as well and we would like to have it in > 6.1. Any (technical) reasons that prevent a backport? There is no technical reason which prevents a backport to v6.1. In fact, we're using this with v6.1-RT LTS. Thanks, Kurt
Hi Kurt, On Sat, 2023-10-28 at 12:09 +0200, Kurt Kanzenbach wrote: > Hi Florian, > > On Fri Oct 27 2023, Florian Bezdeka wrote: > > On Tue, 2023-05-23 at 13:15 +0200, Kurt Kanzenbach wrote: > > > Busy polling is currently not allowed on PREEMPT_RT, because it disables > > > preemption while invoking the NAPI callback. It is not possible to acquire > > > sleeping locks with disabled preemption. For details see commit > > > 20ab39d13e2e ("net/core: disable NET_RX_BUSY_POLL on PREEMPT_RT"). > > > > Is that something that we could consider as Bug-Fix for 6.1 and request > > a backport, or would you consider that as new feature? > > IMO it is in category "never worked". Hence it is not stable material. > > > > > > > > > However, strict cyclic and/or low latency network applications may prefer busy > > > polling e.g., using AF_XDP instead of interrupt driven communication. > > > > > > The preempt_disable() is used in order to prevent the poll_owner and NAPI owner > > > to be preempted while owning the resource to ensure progress. Netpoll performs > > > busy polling in order to acquire the lock. NAPI is locked by setting the > > > NAPIF_STATE_SCHED flag. There is no busy polling if the flag is set and the > > > "owner" is preempted. Worst case is that the task owning NAPI gets preempted and > > > NAPI processing stalls. This is can be prevented by properly prioritising the > > > tasks within the system. > > > > > > Allow RX_BUSY_POLL on PREEMPT_RT if NETPOLL is disabled. Don't disable > > > preemption on PREEMPT_RT within the busy poll loop. Sorry, I need one more information here: We try to re-use the kernel and its configuration from Debian whenever possible. NETPOLL/NETCONSOLE is build as module there. Will this limitation be addressed in the future? Is someone already working on that? Is that maybe on the radar for the ongoing printk() work? (Assuming printk() with NETCONSOLE enabled is the underlying problem) We don't use NETPOLL/NETCONSOLE during runtime but it is enabled at build time. Sadly we can not use busy polling mode in combination with XDP now. (Ignoring the fact that we could adjust the kernel configuration, build on our own, ...) Would love to hear your thoughts about that. Thanks a lot! > > > > > > Tested on x86 hardware with v6.1-RT and v6.3-RT on Intel i225 (igc) with > > > AF_XDP/ZC sockets configured to run in busy polling mode. > > > > That is exactly our use case as well and we would like to have it in > > 6.1. Any (technical) reasons that prevent a backport? > > There is no technical reason which prevents a backport to v6.1. In fact, > we're using this with v6.1-RT LTS. > > Thanks, > Kurt
Hi Florian, On Mon Oct 30 2023, Florian Bezdeka wrote: >> > > Allow RX_BUSY_POLL on PREEMPT_RT if NETPOLL is disabled. Don't disable >> > > preemption on PREEMPT_RT within the busy poll loop. > > Sorry, I need one more information here: We try to re-use the kernel > and its configuration from Debian whenever possible. NETPOLL/NETCONSOLE > is build as module there. > > Will this limitation be addressed in the future? Is someone already > working on that? Is that maybe on the radar for the ongoing printk() > work? (Assuming printk() with NETCONSOLE enabled is the underlying > problem) > > We don't use NETPOLL/NETCONSOLE during runtime but it is enabled at > build time. Sadly we can not use busy polling mode in combination with > XDP now. (Ignoring the fact that we could adjust the kernel > configuration, build on our own, ...) > > Would love to hear your thoughts about that. Thanks a lot! Yes, the busy polling conflicts with netpoll due to the locking. At the moment you have to disable it in the kernel configuration and re-compile. I don't think anyone is working on solving this limitation yet. Thanks, Kurt
diff --git a/net/Kconfig b/net/Kconfig index 7d39c1773eb4..2fb25b534df5 100644 --- a/net/Kconfig +++ b/net/Kconfig @@ -324,7 +324,7 @@ config CGROUP_NET_CLASSID config NET_RX_BUSY_POLL bool - default y if !PREEMPT_RT + default y if !PREEMPT_RT || (PREEMPT_RT && !NETCONSOLE) config BQL bool diff --git a/net/core/dev.c b/net/core/dev.c index b3c13e041935..3393c2f3dbe8 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -6197,7 +6197,8 @@ void napi_busy_loop(unsigned int napi_id, if (!napi) goto out; - preempt_disable(); + if (!IS_ENABLED(CONFIG_PREEMPT_RT)) + preempt_disable(); for (;;) { int work = 0; @@ -6239,7 +6240,8 @@ void napi_busy_loop(unsigned int napi_id, if (unlikely(need_resched())) { if (napi_poll) busy_poll_stop(napi, have_poll_lock, prefer_busy_poll, budget); - preempt_enable(); + if (!IS_ENABLED(CONFIG_PREEMPT_RT)) + preempt_enable(); rcu_read_unlock(); cond_resched(); if (loop_end(loop_end_arg, start_time)) @@ -6250,7 +6252,8 @@ void napi_busy_loop(unsigned int napi_id, } if (napi_poll) busy_poll_stop(napi, have_poll_lock, prefer_busy_poll, budget); - preempt_enable(); + if (!IS_ENABLED(CONFIG_PREEMPT_RT)) + preempt_enable(); out: rcu_read_unlock(); }
Busy polling is currently not allowed on PREEMPT_RT, because it disables preemption while invoking the NAPI callback. It is not possible to acquire sleeping locks with disabled preemption. For details see commit 20ab39d13e2e ("net/core: disable NET_RX_BUSY_POLL on PREEMPT_RT"). However, strict cyclic and/or low latency network applications may prefer busy polling e.g., using AF_XDP instead of interrupt driven communication. The preempt_disable() is used in order to prevent the poll_owner and NAPI owner to be preempted while owning the resource to ensure progress. Netpoll performs busy polling in order to acquire the lock. NAPI is locked by setting the NAPIF_STATE_SCHED flag. There is no busy polling if the flag is set and the "owner" is preempted. Worst case is that the task owning NAPI gets preempted and NAPI processing stalls. This is can be prevented by properly prioritising the tasks within the system. Allow RX_BUSY_POLL on PREEMPT_RT if NETPOLL is disabled. Don't disable preemption on PREEMPT_RT within the busy poll loop. Tested on x86 hardware with v6.1-RT and v6.3-RT on Intel i225 (igc) with AF_XDP/ZC sockets configured to run in busy polling mode. Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de> --- Changes since RFC: * Commit message Previous version: * https://lore.kernel.org/all/20230517110950.78322-1-kurt@linutronix.de/ net/Kconfig | 2 +- net/core/dev.c | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-)