summaryrefslogtreecommitdiff
path: root/drivers
diff options
context:
space:
mode:
authorVladimir Oltean <olteanv@gmail.com>2019-08-31 15:46:19 +0300
committerDavid S. Miller <davem@davemloft.net>2019-09-02 11:59:29 -0700
commit4ba0ebbc6cdecb9fad7c551a3d97b172ebc7b2fa (patch)
tree9c916da84bff3525eeadb6cdcb859f537da5a8d4 /drivers
parenta21cf11bc57fca136f98e81ebc6a6eaf52952b8f (diff)
net: dsa: Fix off-by-one number of calls to devlink_port_unregister
When a function such as dsa_slave_create fails, currently the following stack trace can be seen: [ 2.038342] sja1105 spi0.1: Probed switch chip: SJA1105T [ 2.054556] sja1105 spi0.1: Reset switch and programmed static config [ 2.063837] sja1105 spi0.1: Enabled switch tagging [ 2.068706] fsl-gianfar soc:ethernet@2d90000 eth2: error -19 setting up slave phy [ 2.076371] ------------[ cut here ]------------ [ 2.080973] WARNING: CPU: 1 PID: 21 at net/core/devlink.c:6184 devlink_free+0x1b4/0x1c0 [ 2.088954] Modules linked in: [ 2.092005] CPU: 1 PID: 21 Comm: kworker/1:1 Not tainted 5.3.0-rc6-01360-g41b52e38d2b6-dirty #1746 [ 2.100912] Hardware name: Freescale LS1021A [ 2.105162] Workqueue: events deferred_probe_work_func [ 2.110287] [<c03133a4>] (unwind_backtrace) from [<c030d8cc>] (show_stack+0x10/0x14) [ 2.117992] [<c030d8cc>] (show_stack) from [<c10b08d8>] (dump_stack+0xb4/0xc8) [ 2.125180] [<c10b08d8>] (dump_stack) from [<c0349d04>] (__warn+0xe0/0xf8) [ 2.132018] [<c0349d04>] (__warn) from [<c0349e34>] (warn_slowpath_null+0x40/0x48) [ 2.139549] [<c0349e34>] (warn_slowpath_null) from [<c0f19d74>] (devlink_free+0x1b4/0x1c0) [ 2.147772] [<c0f19d74>] (devlink_free) from [<c1064fc0>] (dsa_switch_teardown+0x60/0x6c) [ 2.155907] [<c1064fc0>] (dsa_switch_teardown) from [<c1065950>] (dsa_register_switch+0x8e4/0xaa8) [ 2.164821] [<c1065950>] (dsa_register_switch) from [<c0ba7fe4>] (sja1105_probe+0x21c/0x2ec) [ 2.173216] [<c0ba7fe4>] (sja1105_probe) from [<c0b35948>] (spi_drv_probe+0x80/0xa4) [ 2.180920] [<c0b35948>] (spi_drv_probe) from [<c0a4c1cc>] (really_probe+0x108/0x400) [ 2.188711] [<c0a4c1cc>] (really_probe) from [<c0a4c694>] (driver_probe_device+0x78/0x1bc) [ 2.196933] [<c0a4c694>] (driver_probe_device) from [<c0a4a3dc>] (bus_for_each_drv+0x58/0xb8) [ 2.205414] [<c0a4a3dc>] (bus_for_each_drv) from [<c0a4c024>] (__device_attach+0xd0/0x168) [ 2.213637] [<c0a4c024>] (__device_attach) from [<c0a4b1d0>] (bus_probe_device+0x84/0x8c) [ 2.221772] [<c0a4b1d0>] (bus_probe_device) from [<c0a4b72c>] (deferred_probe_work_func+0x84/0xc4) [ 2.230686] [<c0a4b72c>] (deferred_probe_work_func) from [<c03650a4>] (process_one_work+0x218/0x510) [ 2.239772] [<c03650a4>] (process_one_work) from [<c03660d8>] (worker_thread+0x2a8/0x5c0) [ 2.247908] [<c03660d8>] (worker_thread) from [<c036b348>] (kthread+0x148/0x150) [ 2.255265] [<c036b348>] (kthread) from [<c03010e8>] (ret_from_fork+0x14/0x2c) [ 2.262444] Exception stack(0xea965fb0 to 0xea965ff8) [ 2.267466] 5fa0: 00000000 00000000 00000000 00000000 [ 2.275598] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 2.283729] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000 [ 2.290333] ---[ end trace ca5d506728a0581a ]--- devlink_free is complaining right here: WARN_ON(!list_empty(&devlink->port_list)); This happens because devlink_port_unregister is no longer done right away in dsa_port_setup when a DSA_PORT_TYPE_USER has failed. Vivien said about this change that: Also no need to call devlink_port_unregister from within dsa_port_setup as this step is inconditionally handled by dsa_port_teardown on error. which is not really true. The devlink_port_unregister function _is_ being called unconditionally from within dsa_port_setup, but not for this port that just failed, just for the previous ones which were set up. ports_teardown: for (i = 0; i < port; i++) dsa_port_teardown(&ds->ports[i]); Initially I was tempted to fix this by extending the "for" loop to also cover the port that failed during setup. But this could have potentially unforeseen consequences unrelated to devlink_port or even other types of ports than user ports, which I can't really test for. For example, if for some reason devlink_port_register itself would fail, then unconditionally unregistering it in dsa_port_teardown would not be a smart idea. The list might go on. So just make dsa_port_setup undo the setup it had done upon failure, and let the for loop undo the work of setting up the previous ports, which are guaranteed to be brought up to a consistent state. Fixes: 955222ca5281 ("net: dsa: use a single switch statement for port setup") Signed-off-by: Vladimir Oltean <olteanv@gmail.com> Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'drivers')
0 files changed, 0 insertions, 0 deletions