diff mbox series

[7/9] softirq,net: Use softirq_needs_break()

Message ID 20230505113315.3307723-8-liujian56@huawei.com (mailing list archive)
State New, archived
Headers show
Series fix softlockup in run_timer_softirq | expand

Commit Message

Liu Jian May 5, 2023, 11:33 a.m. UTC
From: Peter Zijlstra <peterz@infradead.org>

SoftIRQs provide their own timeout/break code now, use that.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Liu Jian <liujian56@huawei.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(-)

Comments

Jason Xing May 8, 2023, 6:21 a.m. UTC | #1
On Fri, May 5, 2023 at 7:27 PM Liu Jian <liujian56@huawei.com> wrote:
>
> From: Peter Zijlstra <peterz@infradead.org>
>
> SoftIRQs provide their own timeout/break code now, use that.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Liu Jian <liujian56@huawei.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
>
>
diff mbox series

Patch

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,
-	},
 	{
 		.procname	= "fb_tunnels_only_for_init_net",
 		.data		= &sysctl_fb_tunnels_only_for_init_net,