lkml.org 
[lkml]   [2018]   [Nov]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.14 099/222] driver/dma/ioat: Call del_timer_sync() without holding prep_lock
    Date
    4.14-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Waiman Long <longman@redhat.com>

    [ Upstream commit cfb03be6c7e8a1591285849c361d67b09f5149f7 ]

    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] #2: (&dev->mutex){....}, at: [<0000000068331147>] device_shutdown+0x1d6/0x660
    [ 1223.954000] #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>
    ---
    drivers/dma/ioat/init.c | 9 ++++++++-
    1 file changed, 8 insertions(+), 1 deletion(-)

    --- a/drivers/dma/ioat/init.c
    +++ b/drivers/dma/ioat/init.c
    @@ -1205,8 +1205,15 @@ static void ioat_shutdown(struct pci_dev

    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);
    + /*
    + * Synchronization rule for del_timer_sync():
    + * - The caller must not hold locks which would prevent
    + * completion of the timer's handler.
    + * So prep_lock cannot be held before calling it.
    + */
    + del_timer_sync(&ioat_chan->timer);
    +
    /* this should quiesce then reset */
    ioat_reset_hw(ioat_chan);
    }

    \
     
     \ /
      Last update: 2018-11-11 23:57    [W:2.830 / U:0.564 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site