Commit 120f28a
iavf: get rid of the crit lock
Get rid of the crit lock.
That frees us from the error prone logic of try_locks.
Thanks to netdev_lock() by Jakub it is now easy, and in most cases we were
protected by it already - replace crit lock by netdev lock when it was not
the case.
Lockdep reports that we should cancel the work under crit_lock [splat1],
and that was the scheme we have mostly followed since [1] by Slawomir.
But when that is done we still got into deadlocks [splat2]. So instead
we should look at the bigger problem, namely "weird locking/scheduling"
of the iavf. The first step to fix that is to remove the crit lock.
I will followup with a -next series that simplifies scheduling/tasks.
Cancel the work without netdev lock (weird unlock+lock scheme),
to fix the [splat2] (which would be totally ugly if we would kept
the crit lock).
Extend protected part of iavf_watchdog_task() to include scheduling
more work.
Note that the removed comment in iavf_reset_task() was misplaced,
it belonged to inside of the removed if condition, so it's gone now.
[splat1] - w/o this patch - The deadlock during VF removal:
WARNING: possible circular locking dependency detected
sh/3825 is trying to acquire lock:
((work_completion)(&(&adapter->watchdog_task)->work)){+.+.}-{0:0}, at: start_flush_work+0x1a1/0x470
but task is already holding lock:
(&adapter->crit_lock){+.+.}-{4:4}, at: iavf_remove+0xd1/0x690 [iavf]
which lock already depends on the new lock.
[splat2] - when cancelling work under crit lock, w/o this series,
see [2] for the band aid attempt
WARNING: possible circular locking dependency detected
sh/3550 is trying to acquire lock:
((wq_completion)iavf){+.+.}-{0:0}, at: touch_wq_lockdep_map+0x26/0x90
but task is already holding lock:
(&dev->lock){+.+.}-{4:4}, at: iavf_remove+0xa6/0x6e0 [iavf]
which lock already depends on the new lock.
[1] fc2e6b3 ("iavf: Rework mutexes for better synchronisation")
[2] pkitszel/linux@52dddbfc2bb60294083f5711a158a
Fixes: d1639a1 ("iavf: fix a deadlock caused by rtnl and driver's lock circular dependencies")
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>1 parent 05702b5 commit 120f28a
File tree
3 files changed
+38
-151
lines changed- drivers/net/ethernet/intel/iavf
3 files changed
+38
-151
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
268 | 268 | | |
269 | 269 | | |
270 | 270 | | |
271 | | - | |
272 | 271 | | |
273 | 272 | | |
274 | 273 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1258 | 1258 | | |
1259 | 1259 | | |
1260 | 1260 | | |
1261 | | - | |
1262 | 1261 | | |
1263 | 1262 | | |
1264 | 1263 | | |
| |||
1281 | 1280 | | |
1282 | 1281 | | |
1283 | 1282 | | |
1284 | | - | |
1285 | | - | |
1286 | | - | |
1287 | | - | |
1288 | | - | |
1289 | | - | |
1290 | | - | |
1291 | | - | |
1292 | 1283 | | |
1293 | 1284 | | |
1294 | 1285 | | |
1295 | 1286 | | |
1296 | 1287 | | |
1297 | 1288 | | |
1298 | 1289 | | |
1299 | | - | |
1300 | 1290 | | |
1301 | 1291 | | |
1302 | 1292 | | |
| |||
1439 | 1429 | | |
1440 | 1430 | | |
1441 | 1431 | | |
1442 | | - | |
1443 | 1432 | | |
1444 | 1433 | | |
| 1434 | + | |
1445 | 1435 | | |
1446 | 1436 | | |
1447 | 1437 | | |
| |||
1469 | 1459 | | |
1470 | 1460 | | |
1471 | 1461 | | |
1472 | | - | |
1473 | | - | |
1474 | | - | |
1475 | | - | |
1476 | | - | |
1477 | | - | |
1478 | | - | |
1479 | | - | |
1480 | | - | |
1481 | 1462 | | |
1482 | 1463 | | |
1483 | 1464 | | |
| |||
1506 | 1487 | | |
1507 | 1488 | | |
1508 | 1489 | | |
1509 | | - | |
1510 | | - | |
1511 | 1490 | | |
1512 | 1491 | | |
1513 | 1492 | | |
| |||
0 commit comments