Skip to content

Commit 84a8404

Browse files
Waiman-Longgregkh
authored andcommitted
driver/dma/ioat: Call del_timer_sync() without holding prep_lock
[ Upstream commit cfb03be ] The following lockdep splat was observed: [ 1222.241750] ====================================================== [ 1222.271301] WARNING: possible circular locking dependency detected [ 1222.301060] 4.16.0-10.el8+5.x86_64+debug #1 Not tainted [ 1222.326659] ------------------------------------------------------ [ 1222.356565] systemd-shutdow/1 is trying to acquire lock: [ 1222.382660] ((&ioat_chan->timer)){+.-.}, at: [<00000000f71e1a28>] del_timer_sync+0x5/0xf0 [ 1222.422928] [ 1222.422928] but task is already holding lock: [ 1222.451743] (&(&ioat_chan->prep_lock)->rlock){+.-.}, at: [<000000008ea98b12>] ioat_shutdown+0x86/0x100 [ioatdma] : [ 1223.524987] Chain exists of: [ 1223.524987] (&ioat_chan->timer) --> &(&ioat_chan->cleanup_lock)->rlock --> &(&ioat_chan->prep_lock)->rlock [ 1223.524987] [ 1223.594082] Possible unsafe locking scenario: [ 1223.594082] [ 1223.622630] CPU0 CPU1 [ 1223.645080] ---- ---- [ 1223.667404] lock(&(&ioat_chan->prep_lock)->rlock); [ 1223.691535] lock(&(&ioat_chan->cleanup_lock)->rlock); [ 1223.728657] lock(&(&ioat_chan->prep_lock)->rlock); [ 1223.765122] lock((&ioat_chan->timer)); [ 1223.784095] [ 1223.784095] *** DEADLOCK *** [ 1223.784095] [ 1223.813492] 4 locks held by systemd-shutdow/1: [ 1223.834677] #0: (reboot_mutex){+.+.}, at: [<0000000056d33456>] SYSC_reboot+0x10f/0x300 [ 1223.873310] #1: (&dev->mutex){....}, at: [<00000000258dfdd7>] device_shutdown+0x1c8/0x660 [ 1223.913604] fail0verflow#2: (&dev->mutex){....}, at: [<0000000068331147>] device_shutdown+0x1d6/0x660 [ 1223.954000] fail0verflow#3: (&(&ioat_chan->prep_lock)->rlock){+.-.}, at: [<000000008ea98b12>] ioat_shutdown+0x86/0x100 [ioatdma] In the ioat_shutdown() function: spin_lock_bh(&ioat_chan->prep_lock); set_bit(IOAT_CHAN_DOWN, &ioat_chan->state); del_timer_sync(&ioat_chan->timer); spin_unlock_bh(&ioat_chan->prep_lock); According to the synchronization rule for the del_timer_sync() function, the caller must not hold locks which would prevent completion of the timer's handler. The timer structure has its own lock that manages its synchronization. Setting the IOAT_CHAN_DOWN bit should prevent other CPUs from trying to use that device anyway, there is probably no need to call del_timer_sync() while holding the prep_lock. So the del_timer_sync() call is now moved outside of the prep_lock critical section to prevent the circular lock dependency. Signed-off-by: Waiman Long <longman@redhat.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> Signed-off-by: Vinod Koul <vkoul@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 7a47855 commit 84a8404

1 file changed

Lines changed: 8 additions & 1 deletion

File tree

drivers/dma/ioat/init.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1205,8 +1205,15 @@ static void ioat_shutdown(struct pci_dev *pdev)
12051205

12061206
spin_lock_bh(&ioat_chan->prep_lock);
12071207
set_bit(IOAT_CHAN_DOWN, &ioat_chan->state);
1208-
del_timer_sync(&ioat_chan->timer);
12091208
spin_unlock_bh(&ioat_chan->prep_lock);
1209+
/*
1210+
* Synchronization rule for del_timer_sync():
1211+
* - The caller must not hold locks which would prevent
1212+
* completion of the timer's handler.
1213+
* So prep_lock cannot be held before calling it.
1214+
*/
1215+
del_timer_sync(&ioat_chan->timer);
1216+
12101217
/* this should quiesce then reset */
12111218
ioat_reset_hw(ioat_chan);
12121219
}

0 commit comments

Comments
 (0)