Skip to content

Commit 4a5fe57

Browse files
jmbergdavem330
authored andcommitted
alx: use fine-grained locking instead of RTNL
In the alx driver, all locking depended on the RTNL, but that causes issues with ipconfig ("ip=..." command line) because that waits for the netdev to have a carrier while holding the RTNL, but the alx workers etc. require RTNL, so the carrier won't be set until the RTNL is dropped and can be acquired by alx workers. This causes long delays at boot, as reported by Nikolai Zhubr. Really the only sensible thing to do here is to not use the RTNL for everything, but instead have fine-grained locking for just the driver. Do that, it's not that hard. Reported-by: Nikolai Zhubr <[email protected]> Signed-off-by: Johannes Berg <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 1351170 commit 4a5fe57

File tree

3 files changed

+76
-29
lines changed

3 files changed

+76
-29
lines changed

drivers/net/ethernet/atheros/alx/alx.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,8 @@ struct alx_priv {
137137

138138
/* protects hw.stats */
139139
spinlock_t stats_lock;
140+
141+
struct mutex mtx;
140142
};
141143

142144
extern const struct ethtool_ops alx_ethtool_ops;

drivers/net/ethernet/atheros/alx/ethtool.c

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,10 @@ static int alx_get_link_ksettings(struct net_device *netdev,
163163
}
164164
}
165165

166+
mutex_lock(&alx->mtx);
166167
cmd->base.speed = hw->link_speed;
167168
cmd->base.duplex = hw->duplex;
169+
mutex_unlock(&alx->mtx);
168170

169171
ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported,
170172
supported);
@@ -181,8 +183,7 @@ static int alx_set_link_ksettings(struct net_device *netdev,
181183
struct alx_hw *hw = &alx->hw;
182184
u32 adv_cfg;
183185
u32 advertising;
184-
185-
ASSERT_RTNL();
186+
int ret;
186187

187188
ethtool_convert_link_mode_to_legacy_u32(&advertising,
188189
cmd->link_modes.advertising);
@@ -200,7 +201,12 @@ static int alx_set_link_ksettings(struct net_device *netdev,
200201
}
201202

202203
hw->adv_cfg = adv_cfg;
203-
return alx_setup_speed_duplex(hw, adv_cfg, hw->flowctrl);
204+
205+
mutex_lock(&alx->mtx);
206+
ret = alx_setup_speed_duplex(hw, adv_cfg, hw->flowctrl);
207+
mutex_unlock(&alx->mtx);
208+
209+
return ret;
204210
}
205211

206212
static void alx_get_pauseparam(struct net_device *netdev,
@@ -209,10 +215,12 @@ static void alx_get_pauseparam(struct net_device *netdev,
209215
struct alx_priv *alx = netdev_priv(netdev);
210216
struct alx_hw *hw = &alx->hw;
211217

218+
mutex_lock(&alx->mtx);
212219
pause->autoneg = !!(hw->flowctrl & ALX_FC_ANEG &&
213220
hw->adv_cfg & ADVERTISED_Autoneg);
214221
pause->tx_pause = !!(hw->flowctrl & ALX_FC_TX);
215222
pause->rx_pause = !!(hw->flowctrl & ALX_FC_RX);
223+
mutex_unlock(&alx->mtx);
216224
}
217225

218226

@@ -232,7 +240,7 @@ static int alx_set_pauseparam(struct net_device *netdev,
232240
if (pause->autoneg)
233241
fc |= ALX_FC_ANEG;
234242

235-
ASSERT_RTNL();
243+
mutex_lock(&alx->mtx);
236244

237245
/* restart auto-neg for auto-mode */
238246
if (hw->adv_cfg & ADVERTISED_Autoneg) {
@@ -254,6 +262,7 @@ static int alx_set_pauseparam(struct net_device *netdev,
254262
alx_cfg_mac_flowcontrol(hw, fc);
255263

256264
hw->flowctrl = fc;
265+
mutex_unlock(&alx->mtx);
257266

258267
return 0;
259268
}

drivers/net/ethernet/atheros/alx/main.c

Lines changed: 61 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2013 Johannes Berg <[email protected]>
2+
* Copyright (c) 2013, 2021 Johannes Berg <[email protected]>
33
*
44
* This file is free software: you may copy, redistribute and/or modify it
55
* under the terms of the GNU General Public License as published by the
@@ -1091,8 +1091,9 @@ static int alx_init_sw(struct alx_priv *alx)
10911091
ALX_MAC_CTRL_RXFC_EN |
10921092
ALX_MAC_CTRL_TXFC_EN |
10931093
7 << ALX_MAC_CTRL_PRMBLEN_SHIFT;
1094+
mutex_init(&alx->mtx);
10941095

1095-
return err;
1096+
return 0;
10961097
}
10971098

10981099

@@ -1122,6 +1123,8 @@ static void alx_halt(struct alx_priv *alx)
11221123
{
11231124
struct alx_hw *hw = &alx->hw;
11241125

1126+
lockdep_assert_held(&alx->mtx);
1127+
11251128
alx_netif_stop(alx);
11261129
hw->link_speed = SPEED_UNKNOWN;
11271130
hw->duplex = DUPLEX_UNKNOWN;
@@ -1147,6 +1150,8 @@ static void alx_configure(struct alx_priv *alx)
11471150

11481151
static void alx_activate(struct alx_priv *alx)
11491152
{
1153+
lockdep_assert_held(&alx->mtx);
1154+
11501155
/* hardware setting lost, restore it */
11511156
alx_reinit_rings(alx);
11521157
alx_configure(alx);
@@ -1161,7 +1166,7 @@ static void alx_activate(struct alx_priv *alx)
11611166

11621167
static void alx_reinit(struct alx_priv *alx)
11631168
{
1164-
ASSERT_RTNL();
1169+
lockdep_assert_held(&alx->mtx);
11651170

11661171
alx_halt(alx);
11671172
alx_activate(alx);
@@ -1249,6 +1254,8 @@ static int __alx_open(struct alx_priv *alx, bool resume)
12491254

12501255
static void __alx_stop(struct alx_priv *alx)
12511256
{
1257+
lockdep_assert_held(&alx->mtx);
1258+
12521259
alx_free_irq(alx);
12531260

12541261
cancel_work_sync(&alx->link_check_wk);
@@ -1284,6 +1291,8 @@ static void alx_check_link(struct alx_priv *alx)
12841291
int old_speed;
12851292
int err;
12861293

1294+
lockdep_assert_held(&alx->mtx);
1295+
12871296
/* clear PHY internal interrupt status, otherwise the main
12881297
* interrupt status will be asserted forever
12891298
*/
@@ -1338,12 +1347,24 @@ static void alx_check_link(struct alx_priv *alx)
13381347

13391348
static int alx_open(struct net_device *netdev)
13401349
{
1341-
return __alx_open(netdev_priv(netdev), false);
1350+
struct alx_priv *alx = netdev_priv(netdev);
1351+
int ret;
1352+
1353+
mutex_lock(&alx->mtx);
1354+
ret = __alx_open(alx, false);
1355+
mutex_unlock(&alx->mtx);
1356+
1357+
return ret;
13421358
}
13431359

13441360
static int alx_stop(struct net_device *netdev)
13451361
{
1346-
__alx_stop(netdev_priv(netdev));
1362+
struct alx_priv *alx = netdev_priv(netdev);
1363+
1364+
mutex_lock(&alx->mtx);
1365+
__alx_stop(alx);
1366+
mutex_unlock(&alx->mtx);
1367+
13471368
return 0;
13481369
}
13491370

@@ -1353,18 +1374,18 @@ static void alx_link_check(struct work_struct *work)
13531374

13541375
alx = container_of(work, struct alx_priv, link_check_wk);
13551376

1356-
rtnl_lock();
1377+
mutex_lock(&alx->mtx);
13571378
alx_check_link(alx);
1358-
rtnl_unlock();
1379+
mutex_unlock(&alx->mtx);
13591380
}
13601381

13611382
static void alx_reset(struct work_struct *work)
13621383
{
13631384
struct alx_priv *alx = container_of(work, struct alx_priv, reset_wk);
13641385

1365-
rtnl_lock();
1386+
mutex_lock(&alx->mtx);
13661387
alx_reinit(alx);
1367-
rtnl_unlock();
1388+
mutex_unlock(&alx->mtx);
13681389
}
13691390

13701391
static int alx_tpd_req(struct sk_buff *skb)
@@ -1771,6 +1792,8 @@ static int alx_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
17711792
goto out_unmap;
17721793
}
17731794

1795+
mutex_lock(&alx->mtx);
1796+
17741797
alx_reset_pcie(hw);
17751798

17761799
phy_configured = alx_phy_configured(hw);
@@ -1781,7 +1804,7 @@ static int alx_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
17811804
err = alx_reset_mac(hw);
17821805
if (err) {
17831806
dev_err(&pdev->dev, "MAC Reset failed, error = %d\n", err);
1784-
goto out_unmap;
1807+
goto out_unlock;
17851808
}
17861809

17871810
/* setup link to put it in a known good starting state */
@@ -1791,7 +1814,7 @@ static int alx_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
17911814
dev_err(&pdev->dev,
17921815
"failed to configure PHY speed/duplex (err=%d)\n",
17931816
err);
1794-
goto out_unmap;
1817+
goto out_unlock;
17951818
}
17961819
}
17971820

@@ -1824,17 +1847,19 @@ static int alx_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
18241847
if (!alx_get_phy_info(hw)) {
18251848
dev_err(&pdev->dev, "failed to identify PHY\n");
18261849
err = -EIO;
1827-
goto out_unmap;
1850+
goto out_unlock;
18281851
}
18291852

1853+
mutex_unlock(&alx->mtx);
1854+
18301855
INIT_WORK(&alx->link_check_wk, alx_link_check);
18311856
INIT_WORK(&alx->reset_wk, alx_reset);
18321857
netif_carrier_off(netdev);
18331858

18341859
err = register_netdev(netdev);
18351860
if (err) {
18361861
dev_err(&pdev->dev, "register netdevice failed\n");
1837-
goto out_unmap;
1862+
goto out_unlock;
18381863
}
18391864

18401865
netdev_info(netdev,
@@ -1843,6 +1868,8 @@ static int alx_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
18431868

18441869
return 0;
18451870

1871+
out_unlock:
1872+
mutex_unlock(&alx->mtx);
18461873
out_unmap:
18471874
iounmap(hw->hw_addr);
18481875
out_free_netdev:
@@ -1869,6 +1896,8 @@ static void alx_remove(struct pci_dev *pdev)
18691896
pci_disable_pcie_error_reporting(pdev);
18701897
pci_disable_device(pdev);
18711898

1899+
mutex_destroy(&alx->mtx);
1900+
18721901
free_netdev(alx->dev);
18731902
}
18741903

@@ -1880,7 +1909,11 @@ static int alx_suspend(struct device *dev)
18801909
if (!netif_running(alx->dev))
18811910
return 0;
18821911
netif_device_detach(alx->dev);
1912+
1913+
mutex_lock(&alx->mtx);
18831914
__alx_stop(alx);
1915+
mutex_unlock(&alx->mtx);
1916+
18841917
return 0;
18851918
}
18861919

@@ -1890,20 +1923,23 @@ static int alx_resume(struct device *dev)
18901923
struct alx_hw *hw = &alx->hw;
18911924
int err;
18921925

1926+
mutex_lock(&alx->mtx);
18931927
alx_reset_phy(hw);
18941928

1895-
if (!netif_running(alx->dev))
1896-
return 0;
1929+
if (!netif_running(alx->dev)) {
1930+
err = 0;
1931+
goto unlock;
1932+
}
18971933

1898-
rtnl_lock();
18991934
err = __alx_open(alx, true);
1900-
rtnl_unlock();
19011935
if (err)
1902-
return err;
1936+
goto unlock;
19031937

19041938
netif_device_attach(alx->dev);
19051939

1906-
return 0;
1940+
unlock:
1941+
mutex_unlock(&alx->mtx);
1942+
return err;
19071943
}
19081944

19091945
static SIMPLE_DEV_PM_OPS(alx_pm_ops, alx_suspend, alx_resume);
@@ -1922,7 +1958,7 @@ static pci_ers_result_t alx_pci_error_detected(struct pci_dev *pdev,
19221958

19231959
dev_info(&pdev->dev, "pci error detected\n");
19241960

1925-
rtnl_lock();
1961+
mutex_lock(&alx->mtx);
19261962

19271963
if (netif_running(netdev)) {
19281964
netif_device_detach(netdev);
@@ -1934,7 +1970,7 @@ static pci_ers_result_t alx_pci_error_detected(struct pci_dev *pdev,
19341970
else
19351971
pci_disable_device(pdev);
19361972

1937-
rtnl_unlock();
1973+
mutex_unlock(&alx->mtx);
19381974

19391975
return rc;
19401976
}
@@ -1947,7 +1983,7 @@ static pci_ers_result_t alx_pci_error_slot_reset(struct pci_dev *pdev)
19471983

19481984
dev_info(&pdev->dev, "pci error slot reset\n");
19491985

1950-
rtnl_lock();
1986+
mutex_lock(&alx->mtx);
19511987

19521988
if (pci_enable_device(pdev)) {
19531989
dev_err(&pdev->dev, "Failed to re-enable PCI device after reset\n");
@@ -1960,7 +1996,7 @@ static pci_ers_result_t alx_pci_error_slot_reset(struct pci_dev *pdev)
19601996
if (!alx_reset_mac(hw))
19611997
rc = PCI_ERS_RESULT_RECOVERED;
19621998
out:
1963-
rtnl_unlock();
1999+
mutex_unlock(&alx->mtx);
19642000

19652001
return rc;
19662002
}
@@ -1972,14 +2008,14 @@ static void alx_pci_error_resume(struct pci_dev *pdev)
19722008

19732009
dev_info(&pdev->dev, "pci error resume\n");
19742010

1975-
rtnl_lock();
2011+
mutex_lock(&alx->mtx);
19762012

19772013
if (netif_running(netdev)) {
19782014
alx_activate(alx);
19792015
netif_device_attach(netdev);
19802016
}
19812017

1982-
rtnl_unlock();
2018+
mutex_unlock(&alx->mtx);
19832019
}
19842020

19852021
static const struct pci_error_handlers alx_err_handlers = {

0 commit comments

Comments
 (0)