[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL+tcoCKBcM8PnZ3_u-TFs8EyAGtuu7be7_akXBCX-gq0R2-+A@mail.gmail.com>
Date: Mon, 8 May 2023 14:21:52 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Liu Jian <liujian56@...wei.com>
Cc: corbet@....net, paulmck@...nel.org, frederic@...nel.org,
quic_neeraju@...cinc.com, joel@...lfernandes.org,
josh@...htriplett.org, boqun.feng@...il.com, rostedt@...dmis.org,
mathieu.desnoyers@...icios.com, jiangshanlai@...il.com,
qiang1.zhang@...el.com, jstultz@...gle.com, tglx@...utronix.de,
sboyd@...nel.org, davem@...emloft.net, edumazet@...gle.com,
kuba@...nel.org, pabeni@...hat.com, peterz@...radead.org,
frankwoo@...gle.com, Rhinewuwu@...gle.com,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
rcu@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH 7/9] softirq,net: Use softirq_needs_break()
On Fri, May 5, 2023 at 7:27 PM Liu Jian <liujian56@...wei.com> wrote:
>
> From: Peter Zijlstra <peterz@...radead.org>
>
> SoftIRQs provide their own timeout/break code now, use that.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> Signed-off-by: Liu Jian <liujian56@...wei.com>
> ---
> Documentation/admin-guide/sysctl/net.rst | 11 +----------
> net/core/dev.c | 6 +-----
> net/core/dev.h | 1 -
> net/core/sysctl_net_core.c | 8 --------
> 4 files changed, 2 insertions(+), 24 deletions(-)
>
> diff --git a/Documentation/admin-guide/sysctl/net.rst b/Documentation/admin-guide/sysctl/net.rst
> index 466c560b0c30..095c60788c61 100644
> --- a/Documentation/admin-guide/sysctl/net.rst
> +++ b/Documentation/admin-guide/sysctl/net.rst
> @@ -267,16 +267,7 @@ netdev_budget
>
> Maximum number of packets taken from all interfaces in one polling cycle (NAPI
> poll). In one polling cycle interfaces which are registered to polling are
> -probed in a round-robin manner. Also, a polling cycle may not exceed
> -netdev_budget_usecs microseconds, even if netdev_budget has not been
> -exhausted.
> -
> -netdev_budget_usecs
> ----------------------
> -
> -Maximum number of microseconds in one NAPI polling cycle. Polling
> -will exit when either netdev_budget_usecs have elapsed during the
> -poll cycle or the number of packets processed reaches netdev_budget.
> +probed in a round-robin manner.
>
> netdev_max_backlog
> ------------------
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 735096d42c1d..70b6726beee6 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4321,7 +4321,6 @@ int netdev_tstamp_prequeue __read_mostly = 1;
> unsigned int sysctl_skb_defer_max __read_mostly = 64;
> int netdev_budget __read_mostly = 300;
> /* Must be at least 2 jiffes to guarantee 1 jiffy timeout */
> -unsigned int __read_mostly netdev_budget_usecs = 2 * USEC_PER_SEC / HZ;
> int weight_p __read_mostly = 64; /* old backlog weight */
> int dev_weight_rx_bias __read_mostly = 1; /* bias for backlog weight */
> int dev_weight_tx_bias __read_mostly = 1; /* bias for output_queue quota */
> @@ -6659,8 +6658,6 @@ static int napi_threaded_poll(void *data)
> static __latent_entropy void net_rx_action(struct softirq_action *h)
> {
> struct softnet_data *sd = this_cpu_ptr(&softnet_data);
> - unsigned long time_limit = jiffies +
> - usecs_to_jiffies(READ_ONCE(netdev_budget_usecs));
> int budget = READ_ONCE(netdev_budget);
> LIST_HEAD(list);
> LIST_HEAD(repoll);
> @@ -6699,8 +6696,7 @@ static __latent_entropy void net_rx_action(struct softirq_action *h)
> * Allow this to run for 2 jiffies since which will allow
> * an average latency of 1.5/HZ.
> */
> - if (unlikely(budget <= 0 ||
> - time_after_eq(jiffies, time_limit))) {
> + if (unlikely(budget <= 0 || softirq_needs_break(h))) {
> sd->time_squeeze++;
> break;
> }
> diff --git a/net/core/dev.h b/net/core/dev.h
> index e075e198092c..e64a60c767ab 100644
> --- a/net/core/dev.h
> +++ b/net/core/dev.h
> @@ -39,7 +39,6 @@ void dev_addr_check(struct net_device *dev);
>
> /* sysctls not referred to from outside net/core/ */
> extern int netdev_budget;
> -extern unsigned int netdev_budget_usecs;
> extern unsigned int sysctl_skb_defer_max;
> extern int netdev_tstamp_prequeue;
> extern int netdev_unregister_timeout_secs;
> diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> index 782273bb93c2..59765c805f5b 100644
> --- a/net/core/sysctl_net_core.c
> +++ b/net/core/sysctl_net_core.c
> @@ -595,14 +595,6 @@ static struct ctl_table net_core_table[] = {
> .extra1 = SYSCTL_ONE,
> .extra2 = &max_skb_frags,
> },
> - {
> - .procname = "netdev_budget_usecs",
> - .data = &netdev_budget_usecs,
> - .maxlen = sizeof(unsigned int),
> - .mode = 0644,
> - .proc_handler = proc_dointvec_minmax,
> - .extra1 = SYSCTL_ZERO,
> - },
I cannot help asking whether we really need to remove the sysctl knob
because it can let some users tune this. It's useful for some cases, I
believe. Do you have any evidence/number to prove we can get rid of
this?
Thanks,
Jason
> {
> .procname = "fb_tunnels_only_for_init_net",
> .data = &sysctl_fb_tunnels_only_for_init_net,
> --
> 2.34.1
>
>
Powered by blists - more mailing lists