lkml.org 
[lkml]   [2020]   [Apr]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH AUTOSEL 4.19 13/47] nvme: fix deadlock caused by ANA update wrong locking
    Date
    From: Sagi Grimberg <sagi@grimberg.me>

    [ Upstream commit 657f1975e9d9c880fa13030e88ba6cc84964f1db ]

    The deadlock combines 4 flows in parallel:
    - ns scanning (triggered from reconnect)
    - request timeout
    - ANA update (triggered from reconnect)
    - I/O coming into the mpath device

    (1) ns scanning triggers disk revalidation -> update disk info ->
    freeze queue -> but blocked, due to (2)

    (2) timeout handler reference the g_usage_counter - > but blocks in
    the transport .timeout() handler, due to (3)

    (3) the transport timeout handler (indirectly) calls nvme_stop_queue() ->
    which takes the (down_read) namespaces_rwsem - > but blocks, due to (4)

    (4) ANA update takes the (down_write) namespaces_rwsem -> calls
    nvme_mpath_set_live() -> which synchronize the ns_head srcu
    (see commit 504db087aacc) -> but blocks, due to (5)

    (5) I/O came into nvme_mpath_make_request -> took srcu_read_lock ->
    direct_make_request > blk_queue_enter -> but blocked, due to (1)

    ==> the request queue is under freeze -> deadlock.

    The fix is making ANA update take a read lock as the namespaces list
    is not manipulated, it is just the ns and ns->head that are being
    updated (which is protected with the ns->head lock).

    Fixes: 0d0b660f214dc ("nvme: add ANA support")
    Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
    Reviewed-by: Keith Busch <kbusch@kernel.org>
    Reviewed-by: Hannes Reinecke <hare@suse.de>
    Signed-off-by: Christoph Hellwig <hch@lst.de>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    drivers/nvme/host/multipath.c | 4 ++--
    1 file changed, 2 insertions(+), 2 deletions(-)

    diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
    index e8bc25aed44ca..588864beabd80 100644
    --- a/drivers/nvme/host/multipath.c
    +++ b/drivers/nvme/host/multipath.c
    @@ -402,7 +402,7 @@ static int nvme_update_ana_state(struct nvme_ctrl *ctrl,
    if (!nr_nsids)
    return 0;

    - down_write(&ctrl->namespaces_rwsem);
    + down_read(&ctrl->namespaces_rwsem);
    list_for_each_entry(ns, &ctrl->namespaces, list) {
    unsigned nsid = le32_to_cpu(desc->nsids[n]);

    @@ -413,7 +413,7 @@ static int nvme_update_ana_state(struct nvme_ctrl *ctrl,
    if (++n == nr_nsids)
    break;
    }
    - up_write(&ctrl->namespaces_rwsem);
    + up_read(&ctrl->namespaces_rwsem);
    return 0;
    }

    --
    2.20.1
    \
     
     \ /
      Last update: 2020-04-18 16:52    [W:2.372 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site