| Message ID | 20200812013440.851707-1-edumazet@google.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] net: accept an empty mask in /sys/class/net/*/queues/rx-*/rps_cpus | expand |
Reviewed-by: Maciej Żenczykowski <maze@google.com>
On 8/11/20 7:34 PM, Eric Dumazet wrote: > We must accept an empty mask in store_rps_map(), or we are not able > to disable RPS on a queue. 0 works. Is that not sufficient?
On 8/11/20 9:16 PM, David Ahern wrote: > On 8/11/20 7:34 PM, Eric Dumazet wrote: >> We must accept an empty mask in store_rps_map(), or we are not able >> to disable RPS on a queue. > > 0 works. Is that not sufficient? > To re-phrase: echo 0 > /sys/class/net/*/queues/rx-*/rps_cpus works to disable rps. I don't get the difference in what you are changing here.
Before breakage, post fix: sfp6:~# echo 0 > /sys/class/net/lo/queues/rx-0/rps_cpus With breakage: lpk17:~# echo 0 > /sys/class/net/lo/queues/rx-0/rps_cpus -bash: echo: write error: Invalid argument
On 8/11/20 9:19 PM, Maciej Żenczykowski wrote: > Before breakage, post fix: > > sfp6:~# echo 0 > /sys/class/net/lo/queues/rx-0/rps_cpus > > With breakage: > lpk17:~# echo 0 > /sys/class/net/lo/queues/rx-0/rps_cpus > -bash: echo: write error: Invalid argument > ah, so this is recent breakage with 5.9. I had not hit before hence the question. thanks for clarifying.
On Tue, Aug 11, 2020 at 8:22 PM David Ahern <dsahern@gmail.com> wrote: > > On 8/11/20 9:19 PM, Maciej Żenczykowski wrote: > > Before breakage, post fix: > > > > sfp6:~# echo 0 > /sys/class/net/lo/queues/rx-0/rps_cpus > > > > With breakage: > > lpk17:~# echo 0 > /sys/class/net/lo/queues/rx-0/rps_cpus > > -bash: echo: write error: Invalid argument > > > > ah, so this is recent breakage with 5.9. I had not hit before hence the > question. thanks for clarifying. Yup, 07bbecb34106 ("net: Restrict receive packets queuing to housekeeping CPUs") has been merged only recently. (This was not in David net-next tree btw, this is why it took us so long to come to this issue)
On Tue, Aug 11, 2020 at 06:45:23PM -0700, Maciej Żenczykowski wrote: > On Tue, Aug 11, 2020 at 6:34 PM Eric Dumazet <edumazet@google.com> wrote: > > > We must accept an empty mask in store_rps_map(), or we are not able > > to disable RPS on a queue. > > > > Fixes: 07bbecb34106 ("net: Restrict receive packets queuing to > > housekeeping CPUs") > > Signed-off-by: Eric Dumazet <edumazet@google.com> > > Reported-by: Maciej Żenczykowski <maze@google.com> > > Cc: Alex Belits <abelits@marvell.com> > > Cc: Nitesh Narayan Lal <nitesh@redhat.com> > > Cc: Peter Zijlstra (Intel) <peterz@infradead.org> > > --- > > net/core/net-sysfs.c | 12 +++++++----- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c > > index > > 9de33b594ff2693c054022a42703c90084122444..efec66fa78b70b2fe5b0a5920317eb1d0415d9e3 > > 100644 > > --- a/net/core/net-sysfs.c > > +++ b/net/core/net-sysfs.c > > @@ -757,11 +757,13 @@ static ssize_t store_rps_map(struct netdev_rx_queue > > *queue, > > return err; > > } > > > > - hk_flags = HK_FLAG_DOMAIN | HK_FLAG_WQ; > > - cpumask_and(mask, mask, housekeeping_cpumask(hk_flags)); > > - if (cpumask_empty(mask)) { > > - free_cpumask_var(mask); > > - return -EINVAL; > > + if (!cpumask_empty(mask)) { > > + hk_flags = HK_FLAG_DOMAIN | HK_FLAG_WQ; > > + cpumask_and(mask, mask, housekeeping_cpumask(hk_flags)); > > + if (cpumask_empty(mask)) { > > + free_cpumask_var(mask); > > + return -EINVAL; > > + } > > } Ah indeed! Sorry about that. Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
On 8/11/20 9:34 PM, Eric Dumazet wrote: > We must accept an empty mask in store_rps_map(), or we are not able > to disable RPS on a queue. > > Fixes: 07bbecb34106 ("net: Restrict receive packets queuing to housekeeping CPUs") > Signed-off-by: Eric Dumazet <edumazet@google.com> > Reported-by: Maciej Żenczykowski <maze@google.com> > Cc: Alex Belits <abelits@marvell.com> > Cc: Nitesh Narayan Lal <nitesh@redhat.com> > Cc: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > net/core/net-sysfs.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c > index 9de33b594ff2693c054022a42703c90084122444..efec66fa78b70b2fe5b0a5920317eb1d0415d9e3 100644 > --- a/net/core/net-sysfs.c > +++ b/net/core/net-sysfs.c > @@ -757,11 +757,13 @@ static ssize_t store_rps_map(struct netdev_rx_queue *queue, > return err; > } > > - hk_flags = HK_FLAG_DOMAIN | HK_FLAG_WQ; > - cpumask_and(mask, mask, housekeeping_cpumask(hk_flags)); > - if (cpumask_empty(mask)) { > - free_cpumask_var(mask); > - return -EINVAL; > + if (!cpumask_empty(mask)) { > + hk_flags = HK_FLAG_DOMAIN | HK_FLAG_WQ; > + cpumask_and(mask, mask, housekeeping_cpumask(hk_flags)); > + if (cpumask_empty(mask)) { > + free_cpumask_var(mask); > + return -EINVAL; > + } > } > > map = kzalloc(max_t(unsigned int, Ah! my bad. Thanks for the fix. Acked-by: Nitesh Narayan Lal <nitesh@redhat.com>
On 8/12/20 1:00 AM, peterz@infradead.org wrote: > On Tue, Aug 11, 2020 at 06:45:23PM -0700, Maciej Żenczykowski wrote: >> On Tue, Aug 11, 2020 at 6:34 PM Eric Dumazet <edumazet@google.com> wrote: >> >>> We must accept an empty mask in store_rps_map(), or we are not able >>> to disable RPS on a queue. >>> >>> Fixes: 07bbecb34106 ("net: Restrict receive packets queuing to >>> housekeeping CPUs") >>> Signed-off-by: Eric Dumazet <edumazet@google.com> >>> Reported-by: Maciej Żenczykowski <maze@google.com> >>> Cc: Alex Belits <abelits@marvell.com> >>> Cc: Nitesh Narayan Lal <nitesh@redhat.com> >>> Cc: Peter Zijlstra (Intel) <peterz@infradead.org> >>> --- >>> net/core/net-sysfs.c | 12 +++++++----- >>> 1 file changed, 7 insertions(+), 5 deletions(-) >>> >>> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c >>> index >>> 9de33b594ff2693c054022a42703c90084122444..efec66fa78b70b2fe5b0a5920317eb1d0415d9e3 >>> 100644 >>> --- a/net/core/net-sysfs.c >>> +++ b/net/core/net-sysfs.c >>> @@ -757,11 +757,13 @@ static ssize_t store_rps_map(struct netdev_rx_queue >>> *queue, >>> return err; >>> } >>> >>> - hk_flags = HK_FLAG_DOMAIN | HK_FLAG_WQ; >>> - cpumask_and(mask, mask, housekeeping_cpumask(hk_flags)); >>> - if (cpumask_empty(mask)) { >>> - free_cpumask_var(mask); >>> - return -EINVAL; >>> + if (!cpumask_empty(mask)) { >>> + hk_flags = HK_FLAG_DOMAIN | HK_FLAG_WQ; >>> + cpumask_and(mask, mask, housekeeping_cpumask(hk_flags)); >>> + if (cpumask_empty(mask)) { >>> + free_cpumask_var(mask); >>> + return -EINVAL; >>> + } >>> } > > Ah indeed! Sorry about that. > > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> > No worries, thanks for the review guys. We probably should add a test doing something like echo ffffffff >/sys/class/net/lo/queues/rx-0/rps_cpus echo 0 >/sys/class/net/lo/queues/rx-0/rps_cpus
From: Eric Dumazet <edumazet@google.com> Date: Tue, 11 Aug 2020 18:34:40 -0700 > We must accept an empty mask in store_rps_map(), or we are not able > to disable RPS on a queue. > > Fixes: 07bbecb34106 ("net: Restrict receive packets queuing to housekeeping CPUs") > Signed-off-by: Eric Dumazet <edumazet@google.com> > Reported-by: Maciej Żenczykowski <maze@google.com> Applied, thanks Eric.
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index 9de33b594ff2693c054022a42703c90084122444..efec66fa78b70b2fe5b0a5920317eb1d0415d9e3 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -757,11 +757,13 @@ static ssize_t store_rps_map(struct netdev_rx_queue *queue, return err; } - hk_flags = HK_FLAG_DOMAIN | HK_FLAG_WQ; - cpumask_and(mask, mask, housekeeping_cpumask(hk_flags)); - if (cpumask_empty(mask)) { - free_cpumask_var(mask); - return -EINVAL; + if (!cpumask_empty(mask)) { + hk_flags = HK_FLAG_DOMAIN | HK_FLAG_WQ; + cpumask_and(mask, mask, housekeeping_cpumask(hk_flags)); + if (cpumask_empty(mask)) { + free_cpumask_var(mask); + return -EINVAL; + } } map = kzalloc(max_t(unsigned int,
We must accept an empty mask in store_rps_map(), or we are not able to disable RPS on a queue. Fixes: 07bbecb34106 ("net: Restrict receive packets queuing to housekeeping CPUs") Signed-off-by: Eric Dumazet <edumazet@google.com> Reported-by: Maciej Żenczykowski <maze@google.com> Cc: Alex Belits <abelits@marvell.com> Cc: Nitesh Narayan Lal <nitesh@redhat.com> Cc: Peter Zijlstra (Intel) <peterz@infradead.org> --- net/core/net-sysfs.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)