• Alexander Sverdlin's avatar
    net: dsa: lan9303: consequently nested-lock physical MDIO · 5a22fbcc
    Alexander Sverdlin authored
    When LAN9303 is MDIO-connected two callchains exist into
    mdio->bus->write():
    
    1. switch ports 1&2 ("physical" PHYs):
    
    virtual (switch-internal) MDIO bus (lan9303_switch_ops->phy_{read|write})->
      lan9303_mdio_phy_{read|write} -> mdiobus_{read|write}_nested
    
    2. LAN9303 virtual PHY:
    
    virtual MDIO bus (lan9303_phy_{read|write}) ->
      lan9303_virt_phy_reg_{read|write} -> regmap -> lan9303_mdio_{read|write}
    
    If the latter functions just take
    mutex_lock(&sw_dev->device->bus->mdio_lock) it triggers a LOCKDEP
    false-positive splat. It's false-positive because the first
    mdio_lock in the second callchain above belongs to virtual MDIO bus, the
    second mdio_lock belongs to physical MDIO bus.
    
    Consequent annotation in lan9303_mdio_{read|write} as nested lock
    (similar to lan9303_mdio_phy_{read|write}, it's the same physical MDIO bus)
    prevents the following splat:
    
    WARNING: possible circular locking dependency detected
    5.15.71 #1 Not tainted
    ------------------------------------------------------
    kworker/u4:3/609 is trying to acquire lock:
    ffff000011531c68 (lan9303_mdio:131:(&lan9303_mdio_regmap_config)->lock){+.+.}-{3:3}, at: regmap_lock_mutex
    but task is already holding lock:
    ffff0000114c44d8 (&bus->mdio_lock){+.+.}-{3:3}, at: mdiobus_read
    which lock already depends on the new lock.
    the existing dependency chain (in reverse order) is:
    -> #1 (&bus->mdio_lock){+.+.}-{3:3}:
           lock_acquire
           __mutex_lock
           mutex_lock_nested
           lan9303_mdio_read
           _regmap_read
           regmap_read
           lan9303_probe
           lan9303_mdio_probe
           mdio_probe
           really_probe
           __driver_probe_device
           driver_probe_device
           __device_attach_driver
           bus_for_each_drv
           __device_attach
           device_initial_probe
           bus_probe_device
           deferred_probe_work_func
           process_one_work
           worker_thread
           kthread
           ret_from_fork
    -> #0 (lan9303_mdio:131:(&lan9303_mdio_regmap_config)->lock){+.+.}-{3:3}:
           __lock_acquire
           lock_acquire.part.0
           lock_acquire
           __mutex_lock
           mutex_lock_nested
           regmap_lock_mutex
           regmap_read
           lan9303_phy_read
           dsa_slave_phy_read
           __mdiobus_read
           mdiobus_read
           get_phy_device
           mdiobus_scan
           __mdiobus_register
           dsa_register_switch
           lan9303_probe
           lan9303_mdio_probe
           mdio_probe
           really_probe
           __driver_probe_device
           driver_probe_device
           __device_attach_driver
           bus_for_each_drv
           __device_attach
           device_initial_probe
           bus_probe_device
           deferred_probe_work_func
           process_one_work
           worker_thread
           kthread
           ret_from_fork
    other info that might help us debug this:
     Possible unsafe locking scenario:
           CPU0                    CPU1
           ----                    ----
      lock(&bus->mdio_lock);
                                   lock(lan9303_mdio:131:(&lan9303_mdio_regmap_config)->lock);
                                   lock(&bus->mdio_lock);
      lock(lan9303_mdio:131:(&lan9303_mdio_regmap_config)->lock);
    *** DEADLOCK ***
    5 locks held by kworker/u4:3/609:
     #0: ffff000002842938 ((wq_completion)events_unbound){+.+.}-{0:0}, at: process_one_work
     #1: ffff80000bacbd60 (deferred_probe_work){+.+.}-{0:0}, at: process_one_work
     #2: ffff000007645178 (&dev->mutex){....}-{3:3}, at: __device_attach
     #3: ffff8000096e6e78 (dsa2_mutex){+.+.}-{3:3}, at: dsa_register_switch
     #4: ffff0000114c44d8 (&bus->mdio_lock){+.+.}-{3:3}, at: mdiobus_read
    stack backtrace:
    CPU: 1 PID: 609 Comm: kworker/u4:3 Not tainted 5.15.71 #1
    Workqueue: events_unbound deferred_probe_work_func
    Call trace:
     dump_backtrace
     show_stack
     dump_stack_lvl
     dump_stack
     print_circular_bug
     check_noncircular
     __lock_acquire
     lock_acquire.part.0
     lock_acquire
     __mutex_lock
     mutex_lock_nested
     regmap_lock_mutex
     regmap_read
     lan9303_phy_read
     dsa_slave_phy_read
     __mdiobus_read
     mdiobus_read
     get_phy_device
     mdiobus_scan
     __mdiobus_register
     dsa_register_switch
     lan9303_probe
     lan9303_mdio_probe
    ...
    
    Cc: stable@vger.kernel.org
    Fixes: dc700583
    
     ("net: dsa: LAN9303: add MDIO managed mode support")
    Signed-off-by: default avatarAlexander Sverdlin <alexander.sverdlin@siemens.com>
    Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
    Link: https://lore.kernel.org/r/20231027065741.534971-1-alexander.sverdlin@siemens.com
    
    Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
    5a22fbcc
lan9303_mdio.c 4.69 KB