* [PATCH V2 0/2] net: Fixup cpu_mask usage
@ 2022-10-14 3:04 guoren
2022-10-14 3:04 ` [PATCH V2 1/2] net: Fixup netif_attrmask_next_and warning guoren
2022-10-14 3:04 ` [PATCH V2 2/2] net: Fixup virtnet_set_affinity() cause cpumask warning guoren
0 siblings, 2 replies; 21+ messages in thread
From: guoren @ 2022-10-14 3:04 UTC (permalink / raw)
To: andriy.shevchenko, davem, edumazet, kuba, pabeni, linux,
yury.norov, caraitto, willemb, jonolson, amritha.nambiar
Cc: linux-kernel, netdev, Guo Ren
From: Guo Ren <guoren@linux.alibaba.com>
Yury Norov repaired cpumask_check() in [1], which eveals the
problem in other code. The series fixup them one by one clearly.
[1] https://lore.kernel.org/all/20220919210559.1509179-1-yury.norov@gmail.com/
Changes:
v2:
- Fixup fixes tag in commit log with Yury Norov advice.
v1:
https://lore.kernel.org/all/20221013163857.3086718-1-guoren@kernel.org/
https://lore.kernel.org/all/20221013171434.3132854-1-guoren@kernel.org/
Guo Ren (2):
net: Fixup netif_attrmask_next_and warning
net: Fixup virtnet_set_affinity() cause cpumask warning
drivers/net/virtio_net.c | 2 ++
net/core/dev.c | 4 ++--
2 files changed, 4 insertions(+), 2 deletions(-)
--
2.36.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH V2 1/2] net: Fixup netif_attrmask_next_and warning
2022-10-14 3:04 [PATCH V2 0/2] net: Fixup cpu_mask usage guoren
@ 2022-10-14 3:04 ` guoren
2022-10-14 3:35 ` Jakub Kicinski
2022-10-14 10:00 ` Andy Shevchenko
2022-10-14 3:04 ` [PATCH V2 2/2] net: Fixup virtnet_set_affinity() cause cpumask warning guoren
1 sibling, 2 replies; 21+ messages in thread
From: guoren @ 2022-10-14 3:04 UTC (permalink / raw)
To: andriy.shevchenko, davem, edumazet, kuba, pabeni, linux,
yury.norov, caraitto, willemb, jonolson, amritha.nambiar
Cc: linux-kernel, netdev, Guo Ren, Guo Ren
From: Guo Ren <guoren@linux.alibaba.com>
Don't pass nr_bits as arg1, cpu_max_bits_warn would cause warning
now 854701ba4c39 ("net: fix cpu_max_bits_warn() usage in
netif_attrmask_next{,_and}").
------------[ cut here ]------------
WARNING: CPU: 2 PID: 1 at include/linux/cpumask.h:110 __netif_set_xps_queue+0x14e/0x770
Modules linked in:
CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.0.0-rc4-00018-g854701ba4c39 #324
Hardware name: riscv-virtio,qemu (DT)
epc : __netif_set_xps_queue+0x14e/0x770
ra : __netif_set_xps_queue+0x552/0x770
epc : ffffffff806fe448 ra : ffffffff806fe84c sp : ff600000023279d0
gp : ffffffff815fff88 tp : ff600000023a0000 t0 : ff6000000308ab40
t1 : 0000000000000003 t2 : 0000000000000000 s0 : ff60000002327a90
s1 : 0000000000000000 a0 : ff6000000308ab00 a1 : ff6000000308ab00
a2 : ff6000000308a8e8 a3 : 0000000000000004 a4 : 0000000000000000
a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000000000000
s2 : 0000000000000000 s3 : 0000000000000000 s4 : ff60000002327aa0
s5 : ffffffff816031c8 s6 : 0000000000000000 s7 : 0000000000000001
s8 : 0000000000000000 s9 : 0000000000000004 s10: ff6000000308a8c0
s11: 0000000000000004 t3 : 0000000000000000 t4 : 0000000000000014
t5 : 0000000000000000 t6 : 0000000000000000
status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003
[<ffffffff805e5824>] virtnet_set_affinity+0x14a/0x1c0
[<ffffffff805e7b04>] virtnet_probe+0x7fc/0xee2
[<ffffffff8050e120>] virtio_dev_probe+0x164/0x2de
[<ffffffff8055b69e>] really_probe+0x82/0x224
[<ffffffff8055b89a>] __driver_probe_device+0x5a/0xaa
[<ffffffff8055b916>] driver_probe_device+0x2c/0xb8
[<ffffffff8055bf34>] __driver_attach+0x76/0x108
[<ffffffff805597c0>] bus_for_each_dev+0x4a/0x8e
[<ffffffff8055b072>] driver_attach+0x1a/0x28
[<ffffffff8055ab8c>] bus_add_driver+0x13c/0x1a6
[<ffffffff8055c722>] driver_register+0x4a/0xfc
[<ffffffff8050dc34>] register_virtio_driver+0x1c/0x2c
[<ffffffff80a2bae4>] virtio_net_driver_init+0x7a/0xb0
[<ffffffff80002840>] do_one_initcall+0x66/0x2e4
[<ffffffff80a01212>] kernel_init_freeable+0x28a/0x304
[<ffffffff808b21e2>] kernel_init+0x1e/0x110
[<ffffffff80003c46>] ret_from_exception+0x0/0x10
---[ end trace 0000000000000000 ]---
Fixes: 80d19669ecd3 ("net: Refactor XPS for CPUs and Rx queues")
Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Signed-off-by: Guo Ren <guoren@kernel.org>
---
net/core/dev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index fa53830d0683..9ec8b10ae329 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2589,8 +2589,8 @@ int __netif_set_xps_queue(struct net_device *dev, const unsigned long *mask,
copy = true;
/* allocate memory for queue storage */
- for (j = -1; j = netif_attrmask_next_and(j, online_mask, mask, nr_ids),
- j < nr_ids;) {
+ for (j = -1; j < nr_ids;
+ j = netif_attrmask_next_and(j, online_mask, mask, nr_ids)) {
if (!new_dev_maps) {
new_dev_maps = kzalloc(maps_sz, GFP_KERNEL);
if (!new_dev_maps) {
--
2.36.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH V2 2/2] net: Fixup virtnet_set_affinity() cause cpumask warning
2022-10-14 3:04 [PATCH V2 0/2] net: Fixup cpu_mask usage guoren
2022-10-14 3:04 ` [PATCH V2 1/2] net: Fixup netif_attrmask_next_and warning guoren
@ 2022-10-14 3:04 ` guoren
2022-10-14 10:00 ` Andy Shevchenko
2022-10-17 1:58 ` Xuan Zhuo
1 sibling, 2 replies; 21+ messages in thread
From: guoren @ 2022-10-14 3:04 UTC (permalink / raw)
To: andriy.shevchenko, davem, edumazet, kuba, pabeni, linux,
yury.norov, caraitto, willemb, jonolson, amritha.nambiar
Cc: linux-kernel, netdev, Guo Ren, Guo Ren
From: Guo Ren <guoren@linux.alibaba.com>
Don't pass nr_bits-1 as arg1 for cpumask_next_wrap, which would
cause warning now 78e5a3399421 ("cpumask: fix checking valid
cpu range").
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at include/linux/cpumask.h:110 cpumask_next_wrap+0x5c/0x80
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.0.0-11659-ge7e38f6cce55-dirty #328
Hardware name: riscv-virtio,qemu (DT)
epc : cpumask_next_wrap+0x5c/0x80
ra : virtnet_set_affinity+0x1ba/0x1fc
epc : ffffffff808992ca ra : ffffffff805d84ca sp : ff60000002327a50
gp : ffffffff81602390 tp : ff600000023a0000 t0 : 5f74656e74726976
t1 : 0000000000000000 t2 : 735f74656e747269 s0 : ff60000002327a90
s1 : 0000000000000003 a0 : 0000000000000003 a1 : ffffffff816051c0
a2 : 0000000000000004 a3 : 0000000000000000 a4 : 0000000000000000
a5 : 0000000000000004 a6 : 0000000000000000 a7 : 0000000000000000
s2 : 0000000000000000 s3 : ffffffff816051c0 s4 : ffffffff8160224c
s5 : 0000000000000004 s6 : 0000000000000004 s7 : 0000000000000000
s8 : 0000000000000003 s9 : ffffffff810aa398 s10: ffffffff80e97d20
s11: 0000000000000004 t3 : ffffffff819acc97 t4 : ffffffff819acc97
t5 : ffffffff819acc98 t6 : ff60000002327878
status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003
[<ffffffff805d84ca>] virtnet_set_affinity+0x1ba/0x1fc
[<ffffffff805da7ac>] virtnet_probe+0x832/0xf1e
[<ffffffff804fe61c>] virtio_dev_probe+0x164/0x2de
[<ffffffff8054c4c4>] really_probe+0x82/0x224
[<ffffffff8054c6c0>] __driver_probe_device+0x5a/0xaa
[<ffffffff8054c73c>] driver_probe_device+0x2c/0xb8
[<ffffffff8054cd66>] __driver_attach+0x76/0x108
[<ffffffff8054a482>] bus_for_each_dev+0x52/0x9a
[<ffffffff8054be8c>] driver_attach+0x1a/0x28
[<ffffffff8054b996>] bus_add_driver+0x154/0x1c2
[<ffffffff8054d592>] driver_register+0x52/0x108
[<ffffffff804fe120>] register_virtio_driver+0x1c/0x2c
[<ffffffff80a29142>] virtio_net_driver_init+0x7a/0xb0
[<ffffffff80002854>] do_one_initcall+0x66/0x2e4
[<ffffffff80a01222>] kernel_init_freeable+0x28a/0x304
[<ffffffff808cb1be>] kernel_init+0x1e/0x110
[<ffffffff80003c4e>] ret_from_exception+0x0/0x10
---[ end trace 0000000000000000 ]---
Fixes: 2ca653d607ce ("virtio_net: Stripe queue affinities across cores.")
Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Signed-off-by: Guo Ren <guoren@kernel.org>
---
drivers/net/virtio_net.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 7106932c6f88..e4b56523b2b5 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2300,6 +2300,8 @@ static void virtnet_set_affinity(struct virtnet_info *vi)
for (j = 0; j < group_size; j++) {
cpumask_set_cpu(cpu, mask);
+ if (cpu == (nr_cpu_ids - 1))
+ break;
cpu = cpumask_next_wrap(cpu, cpu_online_mask,
nr_cpu_ids, false);
}
--
2.36.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH V2 1/2] net: Fixup netif_attrmask_next_and warning
2022-10-14 3:04 ` [PATCH V2 1/2] net: Fixup netif_attrmask_next_and warning guoren
@ 2022-10-14 3:35 ` Jakub Kicinski
2022-10-14 3:39 ` Jakub Kicinski
2022-10-14 6:38 ` Guo Ren
2022-10-14 10:00 ` Andy Shevchenko
1 sibling, 2 replies; 21+ messages in thread
From: Jakub Kicinski @ 2022-10-14 3:35 UTC (permalink / raw)
To: guoren
Cc: andriy.shevchenko, davem, edumazet, pabeni, linux, yury.norov,
caraitto, willemb, jonolson, amritha.nambiar, linux-kernel,
netdev, Guo Ren
On Thu, 13 Oct 2022 23:04:58 -0400 guoren@kernel.org wrote:
> - for (j = -1; j = netif_attrmask_next_and(j, online_mask, mask, nr_ids),
> - j < nr_ids;) {
> + for (j = -1; j < nr_ids;
> + j = netif_attrmask_next_and(j, online_mask, mask, nr_ids)) {
This does not look equivalent, have you tested it?
nr_ids is unsigned, doesn't it mean we'll never enter the loop?
Can we instead revert 854701ba4c and take the larger rework Yury
has posted a week ago into net-next?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 1/2] net: Fixup netif_attrmask_next_and warning
2022-10-14 3:35 ` Jakub Kicinski
@ 2022-10-14 3:39 ` Jakub Kicinski
2022-10-14 4:42 ` Yury Norov
2022-10-14 6:38 ` Guo Ren
1 sibling, 1 reply; 21+ messages in thread
From: Jakub Kicinski @ 2022-10-14 3:39 UTC (permalink / raw)
To: guoren
Cc: andriy.shevchenko, davem, edumazet, pabeni, linux, yury.norov,
caraitto, willemb, jonolson, amritha.nambiar, linux-kernel,
netdev, Guo Ren
On Thu, 13 Oct 2022 20:35:44 -0700 Jakub Kicinski wrote:
> Can we instead revert 854701ba4c and take the larger rework Yury
> has posted a week ago into net-next?
Oh, it was reposted today:
https://lore.kernel.org/all/20221013234349.1165689-2-yury.norov@gmail.com/
But we need a revert of 854701ba4c as well to cover the issue back up
for 6.1, AFAIU.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 1/2] net: Fixup netif_attrmask_next_and warning
2022-10-14 3:39 ` Jakub Kicinski
@ 2022-10-14 4:42 ` Yury Norov
2022-10-14 6:42 ` Guo Ren
2022-10-14 16:03 ` Jakub Kicinski
0 siblings, 2 replies; 21+ messages in thread
From: Yury Norov @ 2022-10-14 4:42 UTC (permalink / raw)
To: Jakub Kicinski
Cc: guoren, andriy.shevchenko, davem, edumazet, pabeni, linux,
caraitto, willemb, jonolson, amritha.nambiar, linux-kernel,
netdev, Guo Ren
On Thu, Oct 13, 2022 at 08:39:11PM -0700, Jakub Kicinski wrote:
> On Thu, 13 Oct 2022 20:35:44 -0700 Jakub Kicinski wrote:
> > Can we instead revert 854701ba4c and take the larger rework Yury
> > has posted a week ago into net-next?
>
> Oh, it was reposted today:
>
> https://lore.kernel.org/all/20221013234349.1165689-2-yury.norov@gmail.com/
>
> But we need a revert of 854701ba4c as well to cover the issue back up
> for 6.1, AFAIU.
The patch 854701ba4c is technically correct. I fixed most of warnings in
advance, but nobody can foresee everything, right? I expected some noise,
and now we have just a few things to fix. This is what for -rc releases
exist, didn't they?
I suggest to keep the patch, because this is the only way to make
cpumask_check()-related issues visible to people. If things will go as
they go now, I expect that -rc3 will be clean from cpumask_check()
warnings.
Thanks,
Yury
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 1/2] net: Fixup netif_attrmask_next_and warning
2022-10-14 3:35 ` Jakub Kicinski
2022-10-14 3:39 ` Jakub Kicinski
@ 2022-10-14 6:38 ` Guo Ren
2022-10-14 15:52 ` Jakub Kicinski
1 sibling, 1 reply; 21+ messages in thread
From: Guo Ren @ 2022-10-14 6:38 UTC (permalink / raw)
To: Jakub Kicinski
Cc: andriy.shevchenko, davem, edumazet, pabeni, linux, yury.norov,
caraitto, willemb, jonolson, amritha.nambiar, linux-kernel,
netdev, Guo Ren
On Fri, Oct 14, 2022 at 11:35 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 13 Oct 2022 23:04:58 -0400 guoren@kernel.org wrote:
> > - for (j = -1; j = netif_attrmask_next_and(j, online_mask, mask, nr_ids),
> > - j < nr_ids;) {
> > + for (j = -1; j < nr_ids;
> > + j = netif_attrmask_next_and(j, online_mask, mask, nr_ids)) {
>
> This does not look equivalent, have you tested it?
>
> nr_ids is unsigned, doesn't it mean we'll never enter the loop?
Yes, you are right. Any unsigned int would break the result.
(gdb) p (int)-1 < (int)2
$1 = 1
(gdb) p (int)-1 < (unsigned int)2
$2 = 0
(gdb) p (unsigned int)-1 < (int)2
$4 = 0
So it should be:
- for (j = -1; j = netif_attrmask_next_and(j, online_mask, mask, nr_ids),
- j < nr_ids;) {
+ for (j = -1; j < (int)nr_ids;
+ j = netif_attrmask_next_and(j, online_mask, mask, nr_ids)) {
Right? Of cause, nr_ids couldn't be 0xffffffff (-1).
>
> Can we instead revert 854701ba4c and take the larger rework Yury
> has posted a week ago into net-next?
--
Best Regards
Guo Ren
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 1/2] net: Fixup netif_attrmask_next_and warning
2022-10-14 4:42 ` Yury Norov
@ 2022-10-14 6:42 ` Guo Ren
2022-10-14 16:03 ` Jakub Kicinski
1 sibling, 0 replies; 21+ messages in thread
From: Guo Ren @ 2022-10-14 6:42 UTC (permalink / raw)
To: Yury Norov
Cc: Jakub Kicinski, andriy.shevchenko, davem, edumazet, pabeni,
linux, caraitto, willemb, jonolson, amritha.nambiar,
linux-kernel, netdev, Guo Ren
On Fri, Oct 14, 2022 at 12:42 PM Yury Norov <yury.norov@gmail.com> wrote:
>
> On Thu, Oct 13, 2022 at 08:39:11PM -0700, Jakub Kicinski wrote:
> > On Thu, 13 Oct 2022 20:35:44 -0700 Jakub Kicinski wrote:
> > > Can we instead revert 854701ba4c and take the larger rework Yury
> > > has posted a week ago into net-next?
> >
> > Oh, it was reposted today:
> >
> > https://lore.kernel.org/all/20221013234349.1165689-2-yury.norov@gmail.com/
> >
> > But we need a revert of 854701ba4c as well to cover the issue back up
> > for 6.1, AFAIU.
>
> The patch 854701ba4c is technically correct. I fixed most of warnings in
> advance, but nobody can foresee everything, right? I expected some noise,
> and now we have just a few things to fix. This is what for -rc releases
> exist, didn't they?
Your job is great, I just want to help with some fixes. Fixes them in
-rc would be a good point.
>
> I suggest to keep the patch, because this is the only way to make
> cpumask_check()-related issues visible to people. If things will go as
> they go now, I expect that -rc3 will be clean from cpumask_check()
> warnings.
>
> Thanks,
> Yury
--
Best Regards
Guo Ren
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 2/2] net: Fixup virtnet_set_affinity() cause cpumask warning
2022-10-14 3:04 ` [PATCH V2 2/2] net: Fixup virtnet_set_affinity() cause cpumask warning guoren
@ 2022-10-14 10:00 ` Andy Shevchenko
2022-10-14 10:04 ` Guo Ren
2022-10-17 1:58 ` Xuan Zhuo
1 sibling, 1 reply; 21+ messages in thread
From: Andy Shevchenko @ 2022-10-14 10:00 UTC (permalink / raw)
To: guoren
Cc: davem, edumazet, kuba, pabeni, linux, yury.norov, caraitto,
willemb, jonolson, amritha.nambiar, linux-kernel, netdev,
Guo Ren
On Thu, Oct 13, 2022 at 11:04:59PM -0400, guoren@kernel.org wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
>
> Don't pass nr_bits-1 as arg1 for cpumask_next_wrap, which would
> cause warning now 78e5a3399421 ("cpumask: fix checking valid
> cpu range").
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at include/linux/cpumask.h:110 cpumask_next_wrap+0x5c/0x80
> Modules linked in:
Submitting Patches document suggests to cut this huge warning to only what
makes sense to see in the report.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 1/2] net: Fixup netif_attrmask_next_and warning
2022-10-14 3:04 ` [PATCH V2 1/2] net: Fixup netif_attrmask_next_and warning guoren
2022-10-14 3:35 ` Jakub Kicinski
@ 2022-10-14 10:00 ` Andy Shevchenko
2022-10-14 10:04 ` Guo Ren
1 sibling, 1 reply; 21+ messages in thread
From: Andy Shevchenko @ 2022-10-14 10:00 UTC (permalink / raw)
To: guoren
Cc: davem, edumazet, kuba, pabeni, linux, yury.norov, caraitto,
willemb, jonolson, amritha.nambiar, linux-kernel, netdev,
Guo Ren
On Thu, Oct 13, 2022 at 11:04:58PM -0400, guoren@kernel.org wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
>
> Don't pass nr_bits as arg1, cpu_max_bits_warn would cause warning
> now 854701ba4c39 ("net: fix cpu_max_bits_warn() usage in
> netif_attrmask_next{,_and}").
>
> ------------[ cut here ]------------
> WARNING: CPU: 2 PID: 1 at include/linux/cpumask.h:110 __netif_set_xps_queue+0x14e/0x770
> Modules linked in:
Submitting Patches documentation suggests to cut this to only what makes sense
for the report.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 1/2] net: Fixup netif_attrmask_next_and warning
2022-10-14 10:00 ` Andy Shevchenko
@ 2022-10-14 10:04 ` Guo Ren
0 siblings, 0 replies; 21+ messages in thread
From: Guo Ren @ 2022-10-14 10:04 UTC (permalink / raw)
To: Andy Shevchenko
Cc: davem, edumazet, kuba, pabeni, linux, yury.norov, caraitto,
willemb, jonolson, amritha.nambiar, linux-kernel, netdev,
Guo Ren
On Fri, Oct 14, 2022 at 6:01 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Oct 13, 2022 at 11:04:58PM -0400, guoren@kernel.org wrote:
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > Don't pass nr_bits as arg1, cpu_max_bits_warn would cause warning
> > now 854701ba4c39 ("net: fix cpu_max_bits_warn() usage in
> > netif_attrmask_next{,_and}").
> >
> > ------------[ cut here ]------------
> > WARNING: CPU: 2 PID: 1 at include/linux/cpumask.h:110 __netif_set_xps_queue+0x14e/0x770
> > Modules linked in:
>
> Submitting Patches documentation suggests to cut this to only what makes sense
> for the report.
Right, thx for mentioning.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
--
Best Regards
Guo Ren
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 2/2] net: Fixup virtnet_set_affinity() cause cpumask warning
2022-10-14 10:00 ` Andy Shevchenko
@ 2022-10-14 10:04 ` Guo Ren
0 siblings, 0 replies; 21+ messages in thread
From: Guo Ren @ 2022-10-14 10:04 UTC (permalink / raw)
To: Andy Shevchenko
Cc: davem, edumazet, kuba, pabeni, linux, yury.norov, caraitto,
willemb, jonolson, amritha.nambiar, linux-kernel, netdev,
Guo Ren
On Fri, Oct 14, 2022 at 6:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Oct 13, 2022 at 11:04:59PM -0400, guoren@kernel.org wrote:
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > Don't pass nr_bits-1 as arg1 for cpumask_next_wrap, which would
> > cause warning now 78e5a3399421 ("cpumask: fix checking valid
> > cpu range").
>
> > ------------[ cut here ]------------
> > WARNING: CPU: 0 PID: 1 at include/linux/cpumask.h:110 cpumask_next_wrap+0x5c/0x80
> > Modules linked in:
>
> Submitting Patches document suggests to cut this huge warning to only what
> makes sense to see in the report.
Okay
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
--
Best Regards
Guo Ren
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 1/2] net: Fixup netif_attrmask_next_and warning
2022-10-14 6:38 ` Guo Ren
@ 2022-10-14 15:52 ` Jakub Kicinski
2022-10-15 1:38 ` Guo Ren
0 siblings, 1 reply; 21+ messages in thread
From: Jakub Kicinski @ 2022-10-14 15:52 UTC (permalink / raw)
To: Guo Ren
Cc: andriy.shevchenko, davem, edumazet, pabeni, linux, yury.norov,
caraitto, willemb, jonolson, amritha.nambiar, linux-kernel,
netdev, Guo Ren
On Fri, 14 Oct 2022 14:38:56 +0800 Guo Ren wrote:
> > This does not look equivalent, have you tested it?
> >
> > nr_ids is unsigned, doesn't it mean we'll never enter the loop?
>
> Yes, you are right. Any unsigned int would break the result.
> (gdb) p (int)-1 < (int)2
> $1 = 1
> (gdb) p (int)-1 < (unsigned int)2
> $2 = 0
> (gdb) p (unsigned int)-1 < (int)2
> $4 = 0
>
> So it should be:
> - for (j = -1; j = netif_attrmask_next_and(j, online_mask, mask, nr_ids),
> - j < nr_ids;) {
> + for (j = -1; j < (int)nr_ids;
> + j = netif_attrmask_next_and(j, online_mask, mask, nr_ids)) {
>
> Right? Of cause, nr_ids couldn't be 0xffffffff (-1).
No. You can't enter the loop with -1 as the iterator either.
Let's move on.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 1/2] net: Fixup netif_attrmask_next_and warning
2022-10-14 4:42 ` Yury Norov
2022-10-14 6:42 ` Guo Ren
@ 2022-10-14 16:03 ` Jakub Kicinski
2022-10-14 16:16 ` Yury Norov
2022-10-15 1:41 ` Guo Ren
1 sibling, 2 replies; 21+ messages in thread
From: Jakub Kicinski @ 2022-10-14 16:03 UTC (permalink / raw)
To: Yury Norov
Cc: guoren, andriy.shevchenko, davem, edumazet, pabeni, linux,
caraitto, willemb, jonolson, amritha.nambiar, linux-kernel,
netdev, Guo Ren
On Thu, 13 Oct 2022 21:42:41 -0700 Yury Norov wrote:
> > Oh, it was reposted today:
> >
> > https://lore.kernel.org/all/20221013234349.1165689-2-yury.norov@gmail.com/
> >
> > But we need a revert of 854701ba4c as well to cover the issue back up
> > for 6.1, AFAIU.
>
> The patch 854701ba4c is technically correct. I fixed most of warnings in
> advance, but nobody can foresee everything, right? I expected some noise,
> and now we have just a few things to fix.
I got 6 warnings booting my machine after pulling back from Linus
(which included your patches in net for the first time).
And that's not including the XPS and the virtio warning.
> This is what for -rc releases exist, didn't they?
>
> I suggest to keep the patch, because this is the only way to make
> cpumask_check()-related issues visible to people. If things will go as
> they go now, I expect that -rc3 will be clean from cpumask_check()
> warnings.
This sounds too close to saying that "it's okay for -rc1 to be broken".
Why were your changes not in linux-next for a month before the merge
window? :(
We will not be merging a refactoring series into net to silence an
arguably over-eager warning. We need a minimal fix, Guo Ren's patches
seem to miss the mark so I reckon the best use of everyone's time is
to just drop the exposing patch and retry in -next 🤷
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 1/2] net: Fixup netif_attrmask_next_and warning
2022-10-14 16:03 ` Jakub Kicinski
@ 2022-10-14 16:16 ` Yury Norov
2022-10-14 18:03 ` Jakub Kicinski
2022-10-15 1:41 ` Guo Ren
1 sibling, 1 reply; 21+ messages in thread
From: Yury Norov @ 2022-10-14 16:16 UTC (permalink / raw)
To: Jakub Kicinski
Cc: guoren, andriy.shevchenko, davem, edumazet, pabeni, linux,
caraitto, willemb, jonolson, amritha.nambiar, linux-kernel,
netdev, Guo Ren
On Fri, Oct 14, 2022 at 9:03 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 13 Oct 2022 21:42:41 -0700 Yury Norov wrote:
> > > Oh, it was reposted today:
> > >
> > > https://lore.kernel.org/all/20221013234349.1165689-2-yury.norov@gmail.com/
> > >
> > > But we need a revert of 854701ba4c as well to cover the issue back up
> > > for 6.1, AFAIU.
> >
> > The patch 854701ba4c is technically correct. I fixed most of warnings in
> > advance, but nobody can foresee everything, right? I expected some noise,
> > and now we have just a few things to fix.
>
> I got 6 warnings booting my machine after pulling back from Linus
> (which included your patches in net for the first time).
> And that's not including the XPS and the virtio warning.
>
> > This is what for -rc releases exist, didn't they?
> >
> > I suggest to keep the patch, because this is the only way to make
> > cpumask_check()-related issues visible to people. If things will go as
> > they go now, I expect that -rc3 will be clean from cpumask_check()
> > warnings.
>
> This sounds too close to saying that "it's okay for -rc1 to be broken".
> Why were your changes not in linux-next for a month before the merge
> window? :(
They spent about a month in -next. Nobody cared.
> We will not be merging a refactoring series into net to silence an
> arguably over-eager warning. We need a minimal fix, Guo Ren's patches
> seem to miss the mark so I reckon the best use of everyone's time is
> to just drop the exposing patch and retry in -next 🤷
If you prefer treating symptoms rather than the disease - I have nothing
to add.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 1/2] net: Fixup netif_attrmask_next_and warning
2022-10-14 16:16 ` Yury Norov
@ 2022-10-14 18:03 ` Jakub Kicinski
0 siblings, 0 replies; 21+ messages in thread
From: Jakub Kicinski @ 2022-10-14 18:03 UTC (permalink / raw)
To: Yury Norov
Cc: guoren, andriy.shevchenko, davem, edumazet, pabeni, linux,
caraitto, willemb, jonolson, amritha.nambiar, linux-kernel,
netdev, Guo Ren
On Fri, 14 Oct 2022 09:16:01 -0700 Yury Norov wrote:
> > We will not be merging a refactoring series into net to silence an
> > arguably over-eager warning. We need a minimal fix, Guo Ren's patches
> > seem to miss the mark so I reckon the best use of everyone's time is
> > to just drop the exposing patch and retry in -next 🤷
>
> If you prefer treating symptoms rather than the disease - I have nothing
> to add.
I don't, but we may consider different things to be "the disease".
Please do not insinuate that I don't care about fixing bugs.
What I can grok from the history and your commit messages is that
you want to catch people who pass what you consider invalid inputs
to the helpers, but nothing will crash/OOB access here, because
the helper double checks that the input is < nr_bits.
So it's a nice cleanup and refactoring, sure, but not an urgent fix
that needs to go to Linus ASAP.
If that's not what you're fixing please explain, I believe I already
asked you to clarify before. And the commit message aren't exactly
informative either.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 1/2] net: Fixup netif_attrmask_next_and warning
2022-10-14 15:52 ` Jakub Kicinski
@ 2022-10-15 1:38 ` Guo Ren
0 siblings, 0 replies; 21+ messages in thread
From: Guo Ren @ 2022-10-15 1:38 UTC (permalink / raw)
To: Jakub Kicinski
Cc: andriy.shevchenko, davem, edumazet, pabeni, linux, yury.norov,
caraitto, willemb, jonolson, amritha.nambiar, linux-kernel,
netdev, Guo Ren
On Fri, Oct 14, 2022 at 11:52 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 14 Oct 2022 14:38:56 +0800 Guo Ren wrote:
> > > This does not look equivalent, have you tested it?
> > >
> > > nr_ids is unsigned, doesn't it mean we'll never enter the loop?
> >
> > Yes, you are right. Any unsigned int would break the result.
> > (gdb) p (int)-1 < (int)2
> > $1 = 1
> > (gdb) p (int)-1 < (unsigned int)2
> > $2 = 0
> > (gdb) p (unsigned int)-1 < (int)2
> > $4 = 0
> >
> > So it should be:
> > - for (j = -1; j = netif_attrmask_next_and(j, online_mask, mask, nr_ids),
> > - j < nr_ids;) {
> > + for (j = -1; j < (int)nr_ids;
> > + j = netif_attrmask_next_and(j, online_mask, mask, nr_ids)) {
> >
> > Right? Of cause, nr_ids couldn't be 0xffffffff (-1).
>
> No. You can't enter the loop with -1 as the iterator either.
> Let's move on.
Oops, how about the below:
for (j = netif_attrmask_next_and(-1, online_mask, mask, nr_ids);
j < (int)nr_ids;
j = netif_attrmask_next_and(j, online_mask, mask, nr_ids)) {
--
Best Regards
Guo Ren
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 1/2] net: Fixup netif_attrmask_next_and warning
2022-10-14 16:03 ` Jakub Kicinski
2022-10-14 16:16 ` Yury Norov
@ 2022-10-15 1:41 ` Guo Ren
1 sibling, 0 replies; 21+ messages in thread
From: Guo Ren @ 2022-10-15 1:41 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Yury Norov, andriy.shevchenko, davem, edumazet, pabeni, linux,
caraitto, willemb, jonolson, amritha.nambiar, linux-kernel,
netdev, Guo Ren
On Sat, Oct 15, 2022 at 12:03 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 13 Oct 2022 21:42:41 -0700 Yury Norov wrote:
> > > Oh, it was reposted today:
> > >
> > > https://lore.kernel.org/all/20221013234349.1165689-2-yury.norov@gmail.com/
> > >
> > > But we need a revert of 854701ba4c as well to cover the issue back up
> > > for 6.1, AFAIU.
> >
> > The patch 854701ba4c is technically correct. I fixed most of warnings in
> > advance, but nobody can foresee everything, right? I expected some noise,
> > and now we have just a few things to fix.
>
> I got 6 warnings booting my machine after pulling back from Linus
> (which included your patches in net for the first time).
> And that's not including the XPS and the virtio warning.
Oh, that's a wide effect than we thought.
>
> > This is what for -rc releases exist, didn't they?
> >
> > I suggest to keep the patch, because this is the only way to make
> > cpumask_check()-related issues visible to people. If things will go as
> > they go now, I expect that -rc3 will be clean from cpumask_check()
> > warnings.
>
> This sounds too close to saying that "it's okay for -rc1 to be broken".
> Why were your changes not in linux-next for a month before the merge
> window? :(
>
> We will not be merging a refactoring series into net to silence an
> arguably over-eager warning. We need a minimal fix, Guo Ren's patches
> seem to miss the mark so I reckon the best use of everyone's time is
> to just drop the exposing patch and retry in -next 🤷
--
Best Regards
Guo Ren
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 2/2] net: Fixup virtnet_set_affinity() cause cpumask warning
2022-10-14 3:04 ` [PATCH V2 2/2] net: Fixup virtnet_set_affinity() cause cpumask warning guoren
2022-10-14 10:00 ` Andy Shevchenko
@ 2022-10-17 1:58 ` Xuan Zhuo
2022-10-17 2:46 ` Guo Ren
1 sibling, 1 reply; 21+ messages in thread
From: Xuan Zhuo @ 2022-10-17 1:58 UTC (permalink / raw)
To: guoren
Cc: linux-kernel, netdev, Guo Ren, Guo Ren, andriy.shevchenko, davem,
edumazet, kuba, pabeni, linux, yury.norov, caraitto, willemb,
jonolson, amritha.nambiar
On Thu, 13 Oct 2022 23:04:59 -0400, guoren@kernel.org wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
>
> Don't pass nr_bits-1 as arg1 for cpumask_next_wrap, which would
> cause warning now 78e5a3399421 ("cpumask: fix checking valid
> cpu range").
>
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at include/linux/cpumask.h:110 cpumask_next_wrap+0x5c/0x80
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.0.0-11659-ge7e38f6cce55-dirty #328
> Hardware name: riscv-virtio,qemu (DT)
> epc : cpumask_next_wrap+0x5c/0x80
> ra : virtnet_set_affinity+0x1ba/0x1fc
> epc : ffffffff808992ca ra : ffffffff805d84ca sp : ff60000002327a50
> gp : ffffffff81602390 tp : ff600000023a0000 t0 : 5f74656e74726976
> t1 : 0000000000000000 t2 : 735f74656e747269 s0 : ff60000002327a90
> s1 : 0000000000000003 a0 : 0000000000000003 a1 : ffffffff816051c0
> a2 : 0000000000000004 a3 : 0000000000000000 a4 : 0000000000000000
> a5 : 0000000000000004 a6 : 0000000000000000 a7 : 0000000000000000
> s2 : 0000000000000000 s3 : ffffffff816051c0 s4 : ffffffff8160224c
> s5 : 0000000000000004 s6 : 0000000000000004 s7 : 0000000000000000
> s8 : 0000000000000003 s9 : ffffffff810aa398 s10: ffffffff80e97d20
> s11: 0000000000000004 t3 : ffffffff819acc97 t4 : ffffffff819acc97
> t5 : ffffffff819acc98 t6 : ff60000002327878
> status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003
> [<ffffffff805d84ca>] virtnet_set_affinity+0x1ba/0x1fc
> [<ffffffff805da7ac>] virtnet_probe+0x832/0xf1e
> [<ffffffff804fe61c>] virtio_dev_probe+0x164/0x2de
> [<ffffffff8054c4c4>] really_probe+0x82/0x224
> [<ffffffff8054c6c0>] __driver_probe_device+0x5a/0xaa
> [<ffffffff8054c73c>] driver_probe_device+0x2c/0xb8
> [<ffffffff8054cd66>] __driver_attach+0x76/0x108
> [<ffffffff8054a482>] bus_for_each_dev+0x52/0x9a
> [<ffffffff8054be8c>] driver_attach+0x1a/0x28
> [<ffffffff8054b996>] bus_add_driver+0x154/0x1c2
> [<ffffffff8054d592>] driver_register+0x52/0x108
> [<ffffffff804fe120>] register_virtio_driver+0x1c/0x2c
> [<ffffffff80a29142>] virtio_net_driver_init+0x7a/0xb0
> [<ffffffff80002854>] do_one_initcall+0x66/0x2e4
> [<ffffffff80a01222>] kernel_init_freeable+0x28a/0x304
> [<ffffffff808cb1be>] kernel_init+0x1e/0x110
> [<ffffffff80003c4e>] ret_from_exception+0x0/0x10
> ---[ end trace 0000000000000000 ]---
>
> Fixes: 2ca653d607ce ("virtio_net: Stripe queue affinities across cores.")
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Signed-off-by: Guo Ren <guoren@kernel.org>
> ---
> drivers/net/virtio_net.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 7106932c6f88..e4b56523b2b5 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2300,6 +2300,8 @@ static void virtnet_set_affinity(struct virtnet_info *vi)
>
> for (j = 0; j < group_size; j++) {
> cpumask_set_cpu(cpu, mask);
> + if (cpu == (nr_cpu_ids - 1))
> + break;
The problem seems to be a problem with cpumask_next_wrap(), I'm not particularly
sure.
But I think there is something wrong with your modification, which will cause
subsequent queues to be bound to (nr_cpu_ids - 1).
Thanks.
> cpu = cpumask_next_wrap(cpu, cpu_online_mask,
> nr_cpu_ids, false);
> }
> --
> 2.36.1
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 2/2] net: Fixup virtnet_set_affinity() cause cpumask warning
2022-10-17 1:58 ` Xuan Zhuo
@ 2022-10-17 2:46 ` Guo Ren
2022-10-17 2:58 ` Xuan Zhuo
0 siblings, 1 reply; 21+ messages in thread
From: Guo Ren @ 2022-10-17 2:46 UTC (permalink / raw)
To: Xuan Zhuo
Cc: linux-kernel, netdev, Guo Ren, andriy.shevchenko, davem,
edumazet, kuba, pabeni, linux, yury.norov, caraitto, willemb,
jonolson, amritha.nambiar
On Mon, Oct 17, 2022 at 10:00 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> On Thu, 13 Oct 2022 23:04:59 -0400, guoren@kernel.org wrote:
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > Don't pass nr_bits-1 as arg1 for cpumask_next_wrap, which would
> > cause warning now 78e5a3399421 ("cpumask: fix checking valid
> > cpu range").
> >
> > ------------[ cut here ]------------
> > WARNING: CPU: 0 PID: 1 at include/linux/cpumask.h:110 cpumask_next_wrap+0x5c/0x80
> > Modules linked in:
> > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.0.0-11659-ge7e38f6cce55-dirty #328
> > Hardware name: riscv-virtio,qemu (DT)
> > epc : cpumask_next_wrap+0x5c/0x80
> > ra : virtnet_set_affinity+0x1ba/0x1fc
> > epc : ffffffff808992ca ra : ffffffff805d84ca sp : ff60000002327a50
> > gp : ffffffff81602390 tp : ff600000023a0000 t0 : 5f74656e74726976
> > t1 : 0000000000000000 t2 : 735f74656e747269 s0 : ff60000002327a90
> > s1 : 0000000000000003 a0 : 0000000000000003 a1 : ffffffff816051c0
> > a2 : 0000000000000004 a3 : 0000000000000000 a4 : 0000000000000000
> > a5 : 0000000000000004 a6 : 0000000000000000 a7 : 0000000000000000
> > s2 : 0000000000000000 s3 : ffffffff816051c0 s4 : ffffffff8160224c
> > s5 : 0000000000000004 s6 : 0000000000000004 s7 : 0000000000000000
> > s8 : 0000000000000003 s9 : ffffffff810aa398 s10: ffffffff80e97d20
> > s11: 0000000000000004 t3 : ffffffff819acc97 t4 : ffffffff819acc97
> > t5 : ffffffff819acc98 t6 : ff60000002327878
> > status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003
> > [<ffffffff805d84ca>] virtnet_set_affinity+0x1ba/0x1fc
> > [<ffffffff805da7ac>] virtnet_probe+0x832/0xf1e
> > [<ffffffff804fe61c>] virtio_dev_probe+0x164/0x2de
> > [<ffffffff8054c4c4>] really_probe+0x82/0x224
> > [<ffffffff8054c6c0>] __driver_probe_device+0x5a/0xaa
> > [<ffffffff8054c73c>] driver_probe_device+0x2c/0xb8
> > [<ffffffff8054cd66>] __driver_attach+0x76/0x108
> > [<ffffffff8054a482>] bus_for_each_dev+0x52/0x9a
> > [<ffffffff8054be8c>] driver_attach+0x1a/0x28
> > [<ffffffff8054b996>] bus_add_driver+0x154/0x1c2
> > [<ffffffff8054d592>] driver_register+0x52/0x108
> > [<ffffffff804fe120>] register_virtio_driver+0x1c/0x2c
> > [<ffffffff80a29142>] virtio_net_driver_init+0x7a/0xb0
> > [<ffffffff80002854>] do_one_initcall+0x66/0x2e4
> > [<ffffffff80a01222>] kernel_init_freeable+0x28a/0x304
> > [<ffffffff808cb1be>] kernel_init+0x1e/0x110
> > [<ffffffff80003c4e>] ret_from_exception+0x0/0x10
> > ---[ end trace 0000000000000000 ]---
> >
> > Fixes: 2ca653d607ce ("virtio_net: Stripe queue affinities across cores.")
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Signed-off-by: Guo Ren <guoren@kernel.org>
> > ---
> > drivers/net/virtio_net.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 7106932c6f88..e4b56523b2b5 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -2300,6 +2300,8 @@ static void virtnet_set_affinity(struct virtnet_info *vi)
> >
> > for (j = 0; j < group_size; j++) {
> > cpumask_set_cpu(cpu, mask);
> > + if (cpu == (nr_cpu_ids - 1))
> > + break;
>
> The problem seems to be a problem with cpumask_next_wrap(), I'm not particularly
> sure.
>
> But I think there is something wrong with your modification, which will cause
> subsequent queues to be bound to (nr_cpu_ids - 1).
Yes, it would lose cpu[nr_cpu_ids - 1]. We've moved to reverting the
patch to fix problem:
https://lore.kernel.org/all/20221015130548.3634468-1-guoren@kernel.org/
>
> Thanks.
>
>
> > cpu = cpumask_next_wrap(cpu, cpu_online_mask,
> > nr_cpu_ids, false);
> > }
> > --
> > 2.36.1
> >
--
Best Regards
Guo Ren
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 2/2] net: Fixup virtnet_set_affinity() cause cpumask warning
2022-10-17 2:46 ` Guo Ren
@ 2022-10-17 2:58 ` Xuan Zhuo
0 siblings, 0 replies; 21+ messages in thread
From: Xuan Zhuo @ 2022-10-17 2:58 UTC (permalink / raw)
To: Guo Ren
Cc: linux-kernel, netdev, Guo Ren, andriy.shevchenko, davem,
edumazet, kuba, pabeni, linux, yury.norov, caraitto, willemb,
jonolson, amritha.nambiar
On Mon, 17 Oct 2022 10:46:59 +0800, Guo Ren <guoren@kernel.org> wrote:
> On Mon, Oct 17, 2022 at 10:00 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > On Thu, 13 Oct 2022 23:04:59 -0400, guoren@kernel.org wrote:
> > > From: Guo Ren <guoren@linux.alibaba.com>
> > >
> > > Don't pass nr_bits-1 as arg1 for cpumask_next_wrap, which would
> > > cause warning now 78e5a3399421 ("cpumask: fix checking valid
> > > cpu range").
> > >
> > > ------------[ cut here ]------------
> > > WARNING: CPU: 0 PID: 1 at include/linux/cpumask.h:110 cpumask_next_wrap+0x5c/0x80
> > > Modules linked in:
> > > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.0.0-11659-ge7e38f6cce55-dirty #328
> > > Hardware name: riscv-virtio,qemu (DT)
> > > epc : cpumask_next_wrap+0x5c/0x80
> > > ra : virtnet_set_affinity+0x1ba/0x1fc
> > > epc : ffffffff808992ca ra : ffffffff805d84ca sp : ff60000002327a50
> > > gp : ffffffff81602390 tp : ff600000023a0000 t0 : 5f74656e74726976
> > > t1 : 0000000000000000 t2 : 735f74656e747269 s0 : ff60000002327a90
> > > s1 : 0000000000000003 a0 : 0000000000000003 a1 : ffffffff816051c0
> > > a2 : 0000000000000004 a3 : 0000000000000000 a4 : 0000000000000000
> > > a5 : 0000000000000004 a6 : 0000000000000000 a7 : 0000000000000000
> > > s2 : 0000000000000000 s3 : ffffffff816051c0 s4 : ffffffff8160224c
> > > s5 : 0000000000000004 s6 : 0000000000000004 s7 : 0000000000000000
> > > s8 : 0000000000000003 s9 : ffffffff810aa398 s10: ffffffff80e97d20
> > > s11: 0000000000000004 t3 : ffffffff819acc97 t4 : ffffffff819acc97
> > > t5 : ffffffff819acc98 t6 : ff60000002327878
> > > status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003
> > > [<ffffffff805d84ca>] virtnet_set_affinity+0x1ba/0x1fc
> > > [<ffffffff805da7ac>] virtnet_probe+0x832/0xf1e
> > > [<ffffffff804fe61c>] virtio_dev_probe+0x164/0x2de
> > > [<ffffffff8054c4c4>] really_probe+0x82/0x224
> > > [<ffffffff8054c6c0>] __driver_probe_device+0x5a/0xaa
> > > [<ffffffff8054c73c>] driver_probe_device+0x2c/0xb8
> > > [<ffffffff8054cd66>] __driver_attach+0x76/0x108
> > > [<ffffffff8054a482>] bus_for_each_dev+0x52/0x9a
> > > [<ffffffff8054be8c>] driver_attach+0x1a/0x28
> > > [<ffffffff8054b996>] bus_add_driver+0x154/0x1c2
> > > [<ffffffff8054d592>] driver_register+0x52/0x108
> > > [<ffffffff804fe120>] register_virtio_driver+0x1c/0x2c
> > > [<ffffffff80a29142>] virtio_net_driver_init+0x7a/0xb0
> > > [<ffffffff80002854>] do_one_initcall+0x66/0x2e4
> > > [<ffffffff80a01222>] kernel_init_freeable+0x28a/0x304
> > > [<ffffffff808cb1be>] kernel_init+0x1e/0x110
> > > [<ffffffff80003c4e>] ret_from_exception+0x0/0x10
> > > ---[ end trace 0000000000000000 ]---
> > >
> > > Fixes: 2ca653d607ce ("virtio_net: Stripe queue affinities across cores.")
> > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > Signed-off-by: Guo Ren <guoren@kernel.org>
> > > ---
> > > drivers/net/virtio_net.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index 7106932c6f88..e4b56523b2b5 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -2300,6 +2300,8 @@ static void virtnet_set_affinity(struct virtnet_info *vi)
> > >
> > > for (j = 0; j < group_size; j++) {
> > > cpumask_set_cpu(cpu, mask);
> > > + if (cpu == (nr_cpu_ids - 1))
> > > + break;
> >
> > The problem seems to be a problem with cpumask_next_wrap(), I'm not particularly
> > sure.
> >
> > But I think there is something wrong with your modification, which will cause
> > subsequent queues to be bound to (nr_cpu_ids - 1).
> Yes, it would lose cpu[nr_cpu_ids - 1]. We've moved to reverting the
> patch to fix problem:
> https://lore.kernel.org/all/20221015130548.3634468-1-guoren@kernel.org/
Great!!
Thanks.
>
>
> >
> > Thanks.
> >
> >
> > > cpu = cpumask_next_wrap(cpu, cpu_online_mask,
> > > nr_cpu_ids, false);
> > > }
> > > --
> > > 2.36.1
> > >
>
>
>
> --
> Best Regards
> Guo Ren
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2022-10-17 2:58 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-14 3:04 [PATCH V2 0/2] net: Fixup cpu_mask usage guoren
2022-10-14 3:04 ` [PATCH V2 1/2] net: Fixup netif_attrmask_next_and warning guoren
2022-10-14 3:35 ` Jakub Kicinski
2022-10-14 3:39 ` Jakub Kicinski
2022-10-14 4:42 ` Yury Norov
2022-10-14 6:42 ` Guo Ren
2022-10-14 16:03 ` Jakub Kicinski
2022-10-14 16:16 ` Yury Norov
2022-10-14 18:03 ` Jakub Kicinski
2022-10-15 1:41 ` Guo Ren
2022-10-14 6:38 ` Guo Ren
2022-10-14 15:52 ` Jakub Kicinski
2022-10-15 1:38 ` Guo Ren
2022-10-14 10:00 ` Andy Shevchenko
2022-10-14 10:04 ` Guo Ren
2022-10-14 3:04 ` [PATCH V2 2/2] net: Fixup virtnet_set_affinity() cause cpumask warning guoren
2022-10-14 10:00 ` Andy Shevchenko
2022-10-14 10:04 ` Guo Ren
2022-10-17 1:58 ` Xuan Zhuo
2022-10-17 2:46 ` Guo Ren
2022-10-17 2:58 ` Xuan Zhuo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).