| Message ID | 20241101-am65-cpsw-multi-rx-j7-fix-v3-2-338fdd6a55da@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | ba3b7ac4f7143568ed6480180a847dc752780ece |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ethernet: ti: am65-cpsw: Fixes to multi queue RX feature | expand |
On Fri, Nov 01, 2024 at 12:18:51PM +0200, Roger Quadros wrote: > flow->irq is initialized to 0 which is a valid IRQ. Set it to -EINVAL > in error path of am65_cpsw_nuss_init_rx_chns() so we do not try > to free an unallocated IRQ in am65_cpsw_nuss_remove_rx_chns(). > > If user tried to change number of RX queues and am65_cpsw_nuss_init_rx_chns() > failed due to any reason, the warning will happen if user tries to change > the number of RX queues after the error condition. > > root@am62xx-evm:~# ethtool -L eth0 rx 3 > [ 40.385293] am65-cpsw-nuss 8000000.ethernet: set new flow-id-base 19 > [ 40.393211] am65-cpsw-nuss 8000000.ethernet: Failed to init rx flow2 > netlink error: Invalid argument > root@am62xx-evm:~# ethtool -L eth0 rx 2 > [ 82.306427] ------------[ cut here ]------------ > [ 82.311075] WARNING: CPU: 0 PID: 378 at kernel/irq/devres.c:144 devm_free_irq+0x84/0x90 > [ 82.469770] Call trace: > [ 82.472208] devm_free_irq+0x84/0x90 > [ 82.475777] am65_cpsw_nuss_remove_rx_chns+0x6c/0xac [ti_am65_cpsw_nuss] > [ 82.482487] am65_cpsw_nuss_update_tx_rx_chns+0x2c/0x9c [ti_am65_cpsw_nuss] > [ 82.489442] am65_cpsw_set_channels+0x30/0x4c [ti_am65_cpsw_nuss] > [ 82.495531] ethnl_set_channels+0x224/0x2dc > [ 82.499713] ethnl_default_set_doit+0xb8/0x1b8 > [ 82.504149] genl_family_rcv_msg_doit+0xc0/0x124 > [ 82.508757] genl_rcv_msg+0x1f0/0x284 > [ 82.512409] netlink_rcv_skb+0x58/0x130 > [ 82.516239] genl_rcv+0x38/0x50 > [ 82.519374] netlink_unicast+0x1d0/0x2b0 > [ 82.523289] netlink_sendmsg+0x180/0x3c4 > [ 82.527205] __sys_sendto+0xe4/0x158 > [ 82.530779] __arm64_sys_sendto+0x28/0x38 > [ 82.534782] invoke_syscall+0x44/0x100 > [ 82.538526] el0_svc_common.constprop.0+0xc0/0xe0 > [ 82.543221] do_el0_svc+0x1c/0x28 > [ 82.546528] el0_svc+0x28/0x98 > [ 82.549578] el0t_64_sync_handler+0xc0/0xc4 > [ 82.553752] el0t_64_sync+0x190/0x194 > [ 82.557407] ---[ end trace 0000000000000000 ]--- > > Fixes: da70d184a8c3 ("net: ethernet: ti: am65-cpsw: Introduce multi queue Rx") Hi Roger, I wonder if the problem predates the cited commit and was, rather, introduced by: Fixes: 24bc19b05f1f ("net: ethernet: ti: am65-cpsw: Add suspend/resume support") ...
Hi Simon, On 05/11/2024 15:30, Simon Horman wrote: > On Fri, Nov 01, 2024 at 12:18:51PM +0200, Roger Quadros wrote: >> flow->irq is initialized to 0 which is a valid IRQ. Set it to -EINVAL >> in error path of am65_cpsw_nuss_init_rx_chns() so we do not try >> to free an unallocated IRQ in am65_cpsw_nuss_remove_rx_chns(). >> >> If user tried to change number of RX queues and am65_cpsw_nuss_init_rx_chns() >> failed due to any reason, the warning will happen if user tries to change >> the number of RX queues after the error condition. >> >> root@am62xx-evm:~# ethtool -L eth0 rx 3 >> [ 40.385293] am65-cpsw-nuss 8000000.ethernet: set new flow-id-base 19 >> [ 40.393211] am65-cpsw-nuss 8000000.ethernet: Failed to init rx flow2 >> netlink error: Invalid argument >> root@am62xx-evm:~# ethtool -L eth0 rx 2 >> [ 82.306427] ------------[ cut here ]------------yes. >> [ 82.311075] WARNING: CPU: 0 PID: 378 at kernel/irq/devres.c:144 devm_free_irq+0x84/0x90 >> [ 82.469770] Call trace: >> [ 82.472208] devm_free_irq+0x84/0x90 >> [ 82.475777] am65_cpsw_nuss_remove_rx_chns+0x6c/0xac [ti_am65_cpsw_nuss] >> [ 82.482487] am65_cpsw_nuss_update_tx_rx_chns+0x2c/0x9c [ti_am65_cpsw_nuss] >> [ 82.489442] am65_cpsw_set_channels+0x30/0x4c [ti_am65_cpsw_nuss] >> [ 82.495531] ethnl_set_channels+0x224/0x2dc >> [ 82.499713] ethnl_default_set_doit+0xb8/0x1b8 >> [ 82.504149] genl_family_rcv_msg_doit+0xc0/0x124 >> [ 82.508757] genl_rcv_msg+0x1f0/0x284 >> [ 82.512409] netlink_rcv_skb+0x58/0x130 >> [ 82.516239] genl_rcv+0x38/0x50 >> [ 82.519374] netlink_unicast+0x1d0/0x2b0 >> [ 82.523289] netlink_sendmsg+0x180/0x3c4 >> [ 82.527205] __sys_sendto+0xe4/0x158 >> [ 82.530779] __arm64_sys_sendto+0x28/0x38 >> [ 82.534782] invoke_syscall+0x44/0x100 >> [ 82.538526] el0_svc_common.constprop.0+0xc0/0xe0 >> [ 82.543221] do_el0_svc+0x1c/0x28 >> [ 82.546528] el0_svc+0x28/0x98 >> [ 82.549578] el0t_64_sync_handler+0xc0/0xc4 >> [ 82.553752] el0t_64_sync+0x190/0x194 >> [ 82.557407] ---[ end trace 0000000000000000 ]--- >> >> Fixes: da70d184a8c3 ("net: ethernet: ti: am65-cpsw: Introduce multi queue Rx") > > Hi Roger, > > I wonder if the problem predates the cited commit and was, rather, > introduced by: > > Fixes: 24bc19b05f1f ("net: ethernet: ti: am65-cpsw: Add suspend/resume support") Partly yes. But at that commit AM65_CPSW_MAX_RX_FLOWS is 1 so there is no opportunity for user to change the number of RX queues.
diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c index 70aea654c79f..ba6db61dd227 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c @@ -2441,6 +2441,7 @@ static int am65_cpsw_nuss_init_rx_chns(struct am65_cpsw_common *common) flow = &rx_chn->flows[i]; flow->id = i; flow->common = common; + flow->irq = -EINVAL; rx_flow_cfg.ring_rxfdq0_id = fdqring_id; rx_flow_cfg.rx_cfg.size = max_desc_num; @@ -2483,6 +2484,7 @@ static int am65_cpsw_nuss_init_rx_chns(struct am65_cpsw_common *common) if (ret) { dev_err(dev, "failure requesting rx %d irq %u, %d\n", i, flow->irq, ret); + flow->irq = -EINVAL; goto err; } }
flow->irq is initialized to 0 which is a valid IRQ. Set it to -EINVAL in error path of am65_cpsw_nuss_init_rx_chns() so we do not try to free an unallocated IRQ in am65_cpsw_nuss_remove_rx_chns(). If user tried to change number of RX queues and am65_cpsw_nuss_init_rx_chns() failed due to any reason, the warning will happen if user tries to change the number of RX queues after the error condition. root@am62xx-evm:~# ethtool -L eth0 rx 3 [ 40.385293] am65-cpsw-nuss 8000000.ethernet: set new flow-id-base 19 [ 40.393211] am65-cpsw-nuss 8000000.ethernet: Failed to init rx flow2 netlink error: Invalid argument root@am62xx-evm:~# ethtool -L eth0 rx 2 [ 82.306427] ------------[ cut here ]------------ [ 82.311075] WARNING: CPU: 0 PID: 378 at kernel/irq/devres.c:144 devm_free_irq+0x84/0x90 [ 82.469770] Call trace: [ 82.472208] devm_free_irq+0x84/0x90 [ 82.475777] am65_cpsw_nuss_remove_rx_chns+0x6c/0xac [ti_am65_cpsw_nuss] [ 82.482487] am65_cpsw_nuss_update_tx_rx_chns+0x2c/0x9c [ti_am65_cpsw_nuss] [ 82.489442] am65_cpsw_set_channels+0x30/0x4c [ti_am65_cpsw_nuss] [ 82.495531] ethnl_set_channels+0x224/0x2dc [ 82.499713] ethnl_default_set_doit+0xb8/0x1b8 [ 82.504149] genl_family_rcv_msg_doit+0xc0/0x124 [ 82.508757] genl_rcv_msg+0x1f0/0x284 [ 82.512409] netlink_rcv_skb+0x58/0x130 [ 82.516239] genl_rcv+0x38/0x50 [ 82.519374] netlink_unicast+0x1d0/0x2b0 [ 82.523289] netlink_sendmsg+0x180/0x3c4 [ 82.527205] __sys_sendto+0xe4/0x158 [ 82.530779] __arm64_sys_sendto+0x28/0x38 [ 82.534782] invoke_syscall+0x44/0x100 [ 82.538526] el0_svc_common.constprop.0+0xc0/0xe0 [ 82.543221] do_el0_svc+0x1c/0x28 [ 82.546528] el0_svc+0x28/0x98 [ 82.549578] el0t_64_sync_handler+0xc0/0xc4 [ 82.553752] el0t_64_sync+0x190/0x194 [ 82.557407] ---[ end trace 0000000000000000 ]--- Fixes: da70d184a8c3 ("net: ethernet: ti: am65-cpsw: Introduce multi queue Rx") Signed-off-by: Roger Quadros <rogerq@kernel.org> --- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 2 ++ 1 file changed, 2 insertions(+)