* [PATCH net-next] net: remove RTNL use for /proc/sys/net/core/rps_default_mask
@ 2025-06-27 13:08 Eric Dumazet
2025-06-27 21:18 ` Kuniyuki Iwashima
2025-07-02 0:46 ` Jakub Kicinski
0 siblings, 2 replies; 4+ messages in thread
From: Eric Dumazet @ 2025-06-27 13:08 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Kuniyuki Iwashima, netdev, eric.dumazet,
Eric Dumazet
Use a dedicated mutex instead.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/core/net-sysfs.c | 15 ++++++++++++---
net/core/net-sysfs.h | 4 ++++
net/core/sysctl_net_core.c | 36 +++++++++++++-----------------------
3 files changed, 29 insertions(+), 26 deletions(-)
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index c9b96938639999ddc1fe52560e0ba2c4f1adff1f..8f897e2c8b4fe125a941f869709458830310169d 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -1210,12 +1210,21 @@ static int rx_queue_default_mask(struct net_device *dev,
struct netdev_rx_queue *queue)
{
#if IS_ENABLED(CONFIG_RPS) && IS_ENABLED(CONFIG_SYSCTL)
- struct cpumask *rps_default_mask = READ_ONCE(dev_net(dev)->core.rps_default_mask);
+ struct cpumask *rps_default_mask;
+ int res = 0;
+ mutex_lock(&rps_default_mask_mutex);
+
+ rps_default_mask = dev_net(dev)->core.rps_default_mask;
if (rps_default_mask && !cpumask_empty(rps_default_mask))
- return netdev_rx_queue_set_rps_mask(queue, rps_default_mask);
-#endif
+ res = netdev_rx_queue_set_rps_mask(queue, rps_default_mask);
+
+ mutex_unlock(&rps_default_mask_mutex);
+
+ return res;
+#else
return 0;
+#endif
}
static int rx_queue_add_kobject(struct net_device *dev, int index)
diff --git a/net/core/net-sysfs.h b/net/core/net-sysfs.h
index 8a5b04c2699aaee13ccc3a5b1543eecd0fc10d29..ff3440d721963b2f90b6a83666a63b3f95e61421 100644
--- a/net/core/net-sysfs.h
+++ b/net/core/net-sysfs.h
@@ -11,4 +11,8 @@ int netdev_queue_update_kobjects(struct net_device *net,
int netdev_change_owner(struct net_device *, const struct net *net_old,
const struct net *net_new);
+#if IS_ENABLED(CONFIG_SYSCTL) && IS_ENABLED(CONFIG_RPS)
+extern struct mutex rps_default_mask_mutex;
+#endif
+
#endif
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 5dbb2c6f371defbf79d4581f9b6c1c3fb13fa9d9..672520e43fefadf4c8c667ff6c77acf3935bc567 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -96,50 +96,40 @@ static int dump_cpumask(void *buffer, size_t *lenp, loff_t *ppos,
#ifdef CONFIG_RPS
-static struct cpumask *rps_default_mask_cow_alloc(struct net *net)
-{
- struct cpumask *rps_default_mask;
-
- if (net->core.rps_default_mask)
- return net->core.rps_default_mask;
-
- rps_default_mask = kzalloc(cpumask_size(), GFP_KERNEL);
- if (!rps_default_mask)
- return NULL;
-
- /* pairs with READ_ONCE in rx_queue_default_mask() */
- WRITE_ONCE(net->core.rps_default_mask, rps_default_mask);
- return rps_default_mask;
-}
+DEFINE_MUTEX(rps_default_mask_mutex);
static int rps_default_mask_sysctl(const struct ctl_table *table, int write,
void *buffer, size_t *lenp, loff_t *ppos)
{
struct net *net = (struct net *)table->data;
+ struct cpumask *mask;
int err = 0;
- rtnl_lock();
+ mutex_lock(&rps_default_mask_mutex);
+ mask = net->core.rps_default_mask;
if (write) {
- struct cpumask *rps_default_mask = rps_default_mask_cow_alloc(net);
-
+ if (!mask) {
+ mask = kzalloc(cpumask_size(), GFP_KERNEL);
+ net->core.rps_default_mask = mask;
+ }
err = -ENOMEM;
- if (!rps_default_mask)
+ if (!mask)
goto done;
- err = cpumask_parse(buffer, rps_default_mask);
+ err = cpumask_parse(buffer, mask);
if (err)
goto done;
- err = rps_cpumask_housekeeping(rps_default_mask);
+ err = rps_cpumask_housekeeping(mask);
if (err)
goto done;
} else {
err = dump_cpumask(buffer, lenp, ppos,
- net->core.rps_default_mask ? : cpu_none_mask);
+ mask ?: cpu_none_mask);
}
done:
- rtnl_unlock();
+ mutex_unlock(&rps_default_mask_mutex);
return err;
}
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH net-next] net: remove RTNL use for /proc/sys/net/core/rps_default_mask
2025-06-27 13:08 [PATCH net-next] net: remove RTNL use for /proc/sys/net/core/rps_default_mask Eric Dumazet
@ 2025-06-27 21:18 ` Kuniyuki Iwashima
2025-07-02 0:46 ` Jakub Kicinski
1 sibling, 0 replies; 4+ messages in thread
From: Kuniyuki Iwashima @ 2025-06-27 21:18 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
netdev, eric.dumazet
On Fri, Jun 27, 2025 at 6:08 AM Eric Dumazet <edumazet@google.com> wrote:
>
> Use a dedicated mutex instead.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
Thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] net: remove RTNL use for /proc/sys/net/core/rps_default_mask
2025-06-27 13:08 [PATCH net-next] net: remove RTNL use for /proc/sys/net/core/rps_default_mask Eric Dumazet
2025-06-27 21:18 ` Kuniyuki Iwashima
@ 2025-07-02 0:46 ` Jakub Kicinski
2025-07-02 6:07 ` Eric Dumazet
1 sibling, 1 reply; 4+ messages in thread
From: Jakub Kicinski @ 2025-07-02 0:46 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Paolo Abeni, Simon Horman, Kuniyuki Iwashima,
netdev, eric.dumazet
On Fri, 27 Jun 2025 13:08:39 +0000 Eric Dumazet wrote:
> diff --git a/net/core/net-sysfs.h b/net/core/net-sysfs.h
> index 8a5b04c2699aaee13ccc3a5b1543eecd0fc10d29..ff3440d721963b2f90b6a83666a63b3f95e61421 100644
> --- a/net/core/net-sysfs.h
> +++ b/net/core/net-sysfs.h
> @@ -11,4 +11,8 @@ int netdev_queue_update_kobjects(struct net_device *net,
> int netdev_change_owner(struct net_device *, const struct net *net_old,
> const struct net *net_new);
>
> +#if IS_ENABLED(CONFIG_SYSCTL) && IS_ENABLED(CONFIG_RPS)
> +extern struct mutex rps_default_mask_mutex;
> +#endif
Perhaps subjective but hiding definitions under ifdefs often forces
the ifdef to spread, IOW it prevents us from using:
if (IS_ENABLED(CONFIG_..))
and relying on compiler to remove the dead code. So I'd skip the ifdef.
> #endif
> diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> index 5dbb2c6f371defbf79d4581f9b6c1c3fb13fa9d9..672520e43fefadf4c8c667ff6c77acf3935bc567 100644
> --- a/net/core/sysctl_net_core.c
> +++ b/net/core/sysctl_net_core.c
> @@ -96,50 +96,40 @@ static int dump_cpumask(void *buffer, size_t *lenp, loff_t *ppos,
>
> #ifdef CONFIG_RPS
>
> -static struct cpumask *rps_default_mask_cow_alloc(struct net *net)
> -{
> - struct cpumask *rps_default_mask;
> -
> - if (net->core.rps_default_mask)
> - return net->core.rps_default_mask;
> -
> - rps_default_mask = kzalloc(cpumask_size(), GFP_KERNEL);
> - if (!rps_default_mask)
> - return NULL;
> -
> - /* pairs with READ_ONCE in rx_queue_default_mask() */
> - WRITE_ONCE(net->core.rps_default_mask, rps_default_mask);
> - return rps_default_mask;
> -}
> +DEFINE_MUTEX(rps_default_mask_mutex);
nit: sparse says ../sysfs.h is not included here so it doesn't see the
declaration for the header:
net/core/sysctl_net_core.c:99:1: warning: symbol 'rps_default_mask_mutex' was not declared. Should it be static?
--
pw-bot: cr
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net-next] net: remove RTNL use for /proc/sys/net/core/rps_default_mask
2025-07-02 0:46 ` Jakub Kicinski
@ 2025-07-02 6:07 ` Eric Dumazet
0 siblings, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2025-07-02 6:07 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David S . Miller, Paolo Abeni, Simon Horman, Kuniyuki Iwashima,
netdev, eric.dumazet
On Tue, Jul 1, 2025 at 5:46 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 27 Jun 2025 13:08:39 +0000 Eric Dumazet wrote:
> > diff --git a/net/core/net-sysfs.h b/net/core/net-sysfs.h
> > index 8a5b04c2699aaee13ccc3a5b1543eecd0fc10d29..ff3440d721963b2f90b6a83666a63b3f95e61421 100644
> > --- a/net/core/net-sysfs.h
> > +++ b/net/core/net-sysfs.h
> > @@ -11,4 +11,8 @@ int netdev_queue_update_kobjects(struct net_device *net,
> > int netdev_change_owner(struct net_device *, const struct net *net_old,
> > const struct net *net_new);
> >
> > +#if IS_ENABLED(CONFIG_SYSCTL) && IS_ENABLED(CONFIG_RPS)
> > +extern struct mutex rps_default_mask_mutex;
> > +#endif
>
> Perhaps subjective but hiding definitions under ifdefs often forces
> the ifdef to spread, IOW it prevents us from using:
>
> if (IS_ENABLED(CONFIG_..))
>
> and relying on compiler to remove the dead code. So I'd skip the ifdef.
Yes, I will remove it in V2.
>
> > #endif
> > diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> > index 5dbb2c6f371defbf79d4581f9b6c1c3fb13fa9d9..672520e43fefadf4c8c667ff6c77acf3935bc567 100644
> > --- a/net/core/sysctl_net_core.c
> > +++ b/net/core/sysctl_net_core.c
> > @@ -96,50 +96,40 @@ static int dump_cpumask(void *buffer, size_t *lenp, loff_t *ppos,
> >
> > #ifdef CONFIG_RPS
> >
> > -static struct cpumask *rps_default_mask_cow_alloc(struct net *net)
> > -{
> > - struct cpumask *rps_default_mask;
> > -
> > - if (net->core.rps_default_mask)
> > - return net->core.rps_default_mask;
> > -
> > - rps_default_mask = kzalloc(cpumask_size(), GFP_KERNEL);
> > - if (!rps_default_mask)
> > - return NULL;
> > -
> > - /* pairs with READ_ONCE in rx_queue_default_mask() */
> > - WRITE_ONCE(net->core.rps_default_mask, rps_default_mask);
> > - return rps_default_mask;
> > -}
> > +DEFINE_MUTEX(rps_default_mask_mutex);
>
> nit: sparse says ../sysfs.h is not included here so it doesn't see the
> declaration for the header:
>
> net/core/sysctl_net_core.c:99:1: warning: symbol 'rps_default_mask_mutex' was not declared. Should it be static?
Thanks, I will fix this in V2.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-07-02 6:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-27 13:08 [PATCH net-next] net: remove RTNL use for /proc/sys/net/core/rps_default_mask Eric Dumazet
2025-06-27 21:18 ` Kuniyuki Iwashima
2025-07-02 0:46 ` Jakub Kicinski
2025-07-02 6:07 ` Eric Dumazet
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.