Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     netdev@vger.kernel.org
Cc:     Ioana Ciornei <ioana.ciornei@nxp.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Russell King <linux@armlinux.org.uk>,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH net-next 00/12] Fix rtnl_mutex deadlock with DPAA2 and SFP modules
Date:   Tue, 29 Nov 2022 16:12:09 +0200
Message-Id: <20221129141221.872653-1-vladimir.oltean@nxp.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?yxTrO2wEnt6Q2bKZcxqKdc+WPUx+6UtBTnxhiMXT6oA/2oCs0fmeq1ipEfim?=
 =?us-ascii?Q?v+XLKlVe/U3I1jCTpgeV0ABxBNLIfbMCQbtINydboZ9HuscOtTUWsnHZXuML?=
 =?us-ascii?Q?IakI47stT4s2vaW+GQP1qiaMY2+0ZxkSrHiGZYvvrS5pJWxWRWV0LyDfeg03?=
 =?us-ascii?Q?ZYsmfHc4+VSaHS5h0iAV46GJHSuct+ZyMlqA99ov1xhYkE0mSF+m0FA9lVTN?=
 =?us-ascii?Q?WwV7HAtsMwdEvlquHRcj5trUn7OEAkTr+STIgA1TRjHBwwCE6cp5Bs0wLw82?=
 =?us-ascii?Q?D2jNZdFgoUyMZ631yT7Jyeo5gvr4M+OIHuE3zSuO0aGLjE1Obvs0Jegbzkmb?=
 =?us-ascii?Q?OGiefmw4gIcDhq9QAHZc1G7r28rIe5An4FAicD+C9dyJdCDxnl590JmJh5d/?=
 =?us-ascii?Q?WkovPm0i43aqq6HDTSLAfwPyaHfOE2SJdDxftUJYwLQ2MRuUqs8SgikYa6wJ?=
 =?us-ascii?Q?JLixx9VgA+Nsvjv0WKAEvdQ69fjeyx3ETt+1/e3qJHH1AnFPxxxW9KRI2JsQ?=
 =?us-ascii?Q?D8Yx9+cBcF+3dbyTPh26QVwg9YK3gLJ2XsBEIZ9a/6QaJJqWnKLu+aEfu+06?=
 =?us-ascii?Q?kBw4kpQ50aGrekKxogY/GoPQtliaZE3SKvMzgNKA2j02cvZ324l5rfwLSCQA?=
 =?us-ascii?Q?UK2c84FA/yeA3+ZFED/To535DgWfUb9qcje6RiCeyNT4mJrr6gGXCB6BwmFT?=
 =?us-ascii?Q?QtPV72BzkVxRuF7bJxhAVkglvg5Qkvw9oajWFyMHXkjIueUsiPTSVOUIasS1?=
 =?us-ascii?Q?fXHa9TrKLwJivxbYgtuG3DBA8q+I5yaaa8y9CloGAFV+jZ5OQKgL7MsmMbF4?=
 =?us-ascii?Q?+vI902CsLNWMjzbHuUBS69Kv/n3xmeVSraqg/e824ja/1htcuEAEAAUnE9UC?=
 =?us-ascii?Q?rOvjKq3a9d3sUlxlT3aP/6JxGzYMZ1keXOCoZj+Gtsu9emMw4FuVv27Mtp3Z?=
 =?us-ascii?Q?+UmaYTth9vPNLI61zNvk3spn8c08io3B+uzzITvhtTtgofrbkzazZE0IUlcJ?=
 =?us-ascii?Q?+YqIJAOjR1ejEMu9NlsNi8iKNdPonTWwDPbgYij9krOUK0249WZtYz8T5+6a?=
 =?us-ascii?Q?zZ4sFIQswk2dQ4dcYY6i88FyLJdixc9+hagmRKTeTZHoEWeljZ9N7lvuJNfr?=
 =?us-ascii?Q?PFtN706KJr3X/LGv9tlGVFeE8qrQFo2VNtH4bYjNO2VJarTGVa05eVOUPIkZ?=
 =?us-ascii?Q?0At94qXnQksu5TovLkodja+F1bKgX1d7pIjl5SI3Jj44UIQp0zermW1AKRvt?=
 =?us-ascii?Q?qM/2Er9JgUfBOiPfPNpOFPb5tL/AJepYKQUWoVkrOu+SwvgiHAR/g5dZ8a0T?=
 =?us-ascii?Q?A8pbcmHJR8+LNq7TkQjIdbWRV+XvPl4G7P1akhtqpOlSIk2o40lhsbQ01bf0?=
 =?us-ascii?Q?MMD3JR7hQbsMT9nQ+6f1lcbObVnI5MUT+aEL/efx7Lc904SMvZuv+XNgXcCi?=
 =?us-ascii?Q?wKvC2WkgeoxV0M2RsWu7UXxZRubbQV4SQJ3wInP7cFOobRpc18Aa/MOmNQJP?=
 =?us-ascii?Q?RFxlsRhLfawokxgZ7SBoE6AH3GQxpaEge0nRcXp+W55feVOQwYg7N5Ria4A+?=
 =?us-ascii?Q?2BoqanGWSnykvrd2KX95F/YSAjjl2feCZyGCLnIr0ZebSQT+xKKWSSJnQk5Q?=
 =?us-ascii?Q?0w=3D=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7b4cb2be-4618-4d2a-025d-08dad213c28a
X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB5136.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Nov 2022 14:12:33.8569
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: i/TQ+ULxy6biWgpvpJerToEeIp7wTGxYUWq8gDPvp82fGQ736bfD61wXYmsHW5Azqkk8GxC5+YHfCjJH8vgQWg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR04MB8724
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,SPF_HELO_PASS,
        T_SPF_PERMERROR autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

This patch set deliberately targets net-next and lacks Fixes: tags due
to caution on my part.

While testing some SFP modules on the Solidrun Honeycomb LX2K platform,
I noticed that rebooting causes a deadlock:

============================================
WARNING: possible recursive locking detected
6.1.0-rc5-07010-ga9b9500ffaac-dirty #656 Not tainted
--------------------------------------------
systemd-shutdow/1 is trying to acquire lock:
ffffa62db6cf42f0 (rtnl_mutex){+.+.}-{4:4}, at: rtnl_lock+0x1c/0x30

but task is already holding lock:
ffffa62db6cf42f0 (rtnl_mutex){+.+.}-{4:4}, at: rtnl_lock+0x1c/0x30

other info that might help us debug this:
 Possible unsafe locking scenario:

       CPU0
       ----
  lock(rtnl_mutex);
  lock(rtnl_mutex);

 *** DEADLOCK ***

 May be due to missing lock nesting notation

6 locks held by systemd-shutdow/1:
 #0: ffffa62db6863c70 (system_transition_mutex){+.+.}-{4:4}, at: __do_sys_reboot+0xd4/0x260
 #1: ffff2f2b0176f100 (&dev->mutex){....}-{4:4}, at: device_shutdown+0xf4/0x260
 #2: ffff2f2b017be900 (&dev->mutex){....}-{4:4}, at: device_shutdown+0x104/0x260
 #3: ffff2f2b017680f0 (&dev->mutex){....}-{4:4}, at: device_release_driver_internal+0x40/0x260
 #4: ffff2f2b0e1608f0 (&dev->mutex){....}-{4:4}, at: device_release_driver_internal+0x40/0x260
 #5: ffffa62db6cf42f0 (rtnl_mutex){+.+.}-{4:4}, at: rtnl_lock+0x1c/0x30

stack backtrace:
CPU: 6 PID: 1 Comm: systemd-shutdow Not tainted 6.1.0-rc5-07010-ga9b9500ffaac-dirty #656
Hardware name: SolidRun LX2160A Honeycomb (DT)
Call trace:
 lock_acquire+0x68/0x84
 __mutex_lock+0x98/0x460
 mutex_lock_nested+0x2c/0x40
 rtnl_lock+0x1c/0x30
 sfp_bus_del_upstream+0x1c/0xac
 phylink_destroy+0x1c/0x50
 dpaa2_mac_disconnect+0x28/0x70
 dpaa2_eth_remove+0x1dc/0x1f0
 fsl_mc_driver_remove+0x24/0x60
 device_remove+0x70/0x80
 device_release_driver_internal+0x1f0/0x260
 device_links_unbind_consumers+0xe0/0x110
 device_release_driver_internal+0x138/0x260
 device_release_driver+0x18/0x24
 bus_remove_device+0x12c/0x13c
 device_del+0x16c/0x424
 fsl_mc_device_remove+0x28/0x40
 __fsl_mc_device_remove+0x10/0x20
 device_for_each_child+0x5c/0xac
 dprc_remove+0x94/0xb4
 fsl_mc_driver_remove+0x24/0x60
 device_remove+0x70/0x80
 device_release_driver_internal+0x1f0/0x260
 device_release_driver+0x18/0x24
 bus_remove_device+0x12c/0x13c
 device_del+0x16c/0x424
 fsl_mc_bus_remove+0x8c/0x10c
 fsl_mc_bus_shutdown+0x10/0x20
 platform_shutdown+0x24/0x3c
 device_shutdown+0x15c/0x260
 kernel_restart+0x40/0xa4
 __do_sys_reboot+0x1e4/0x260
 __arm64_sys_reboot+0x24/0x30

But fixing this appears to be not so simple. The patch set represents my
attempt to address it.

In short, the problem is that dpaa2_mac_connect() and dpaa2_mac_disconnect()
call 2 phylink functions in a row, one takes rtnl_lock() itself -
phylink_create(), and one which requires rtnl_lock() to be held by the
caller - phylink_fwnode_phy_connect(). The existing approach in the
drivers is too simple. We take rtnl_lock() when calling dpaa2_mac_connect(),
which is what results in the deadlock.

Fixing just that creates another problem. The drivers make use of
rtnl_lock() for serializing with other code paths too. I think I've
found all those code paths, and established other mechanisms for
serializing with them.

Vladimir Oltean (12):
  net: dpaa2-eth: don't use -ENOTSUPP error code
  net: dpaa2: replace dpaa2_mac_is_type_fixed() with
    dpaa2_mac_is_type_phy()
  net: dpaa2-mac: absorb phylink_start() call into dpaa2_mac_start()
  net: dpaa2-mac: remove defensive check in dpaa2_mac_disconnect()
  net: dpaa2-eth: assign priv->mac after dpaa2_mac_connect() call
  net: dpaa2-switch: assign port_priv->mac after dpaa2_mac_connect()
    call
  net: dpaa2: publish MAC stringset to ethtool -S even if MAC is missing
  net: dpaa2-switch replace direct MAC access with
    dpaa2_switch_port_has_mac()
  net: dpaa2-eth: connect to MAC before requesting the "endpoint
    changed" IRQ
  net: dpaa2-eth: serialize changes to priv->mac with a mutex
  net: dpaa2-switch: serialize changes to priv->mac with a mutex
  net: dpaa2-mac: move rtnl_lock() only around
    phylink_{,dis}connect_phy()

 .../freescale/dpaa2/mac-phy-support.rst       |  9 +-
 .../net/ethernet/freescale/dpaa2/dpaa2-eth.c  | 87 ++++++++++++-------
 .../net/ethernet/freescale/dpaa2/dpaa2-eth.h  | 11 +--
 .../ethernet/freescale/dpaa2/dpaa2-ethtool.c  | 70 ++++++++++-----
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  | 16 +++-
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.h  | 10 ++-
 .../freescale/dpaa2/dpaa2-switch-ethtool.c    | 45 ++++++----
 .../ethernet/freescale/dpaa2/dpaa2-switch.c   | 59 +++++++++----
 .../ethernet/freescale/dpaa2/dpaa2-switch.h   |  9 +-
 9 files changed, 212 insertions(+), 104 deletions(-)

-- 
2.34.1