[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d085757ed5607e82b1cd09d10d4c9f73bbdf3154.camel@siemens.com>
Date: Fri, 27 Oct 2023 13:43:27 +0200
From: Florian Bezdeka <florian.bezdeka@...mens.com>
To: Kurt Kanzenbach <kurt@...utronix.de>, "David S. Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
<kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Subject: Re: [PATCH net-next] net/core: Enable socket busy polling on -RT
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@...utronix.de>
> Signed-off-by: Kurt Kanzenbach <kurt@...utronix.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
>
Powered by blists - more mailing lists