Commit e164471d authored by Sagi Grimberg's avatar Sagi Grimberg Committed by Christoph Hellwig

nvme: don't protect ns mutation with ns->head->lock

Right now ns->head->lock is protecting namespace mutation
which is wrong and unneeded. Move it to only protect
against head mutations. While we're at it, remove unnecessary
ns->head reference as we already have head pointer.

The problem with this is that the head->lock spans
mpath disk node I/O that may block under some conditions (if
for example the controller is disconnecting or the path
became inaccessible), The locking scheme does not allow any
other path to enable itself, preventing blocked I/O to complete
and forward-progress from there.

This is a preparation patch for the fix in a subsequent patch
where the disk I/O will also be done outside the head->lock.

Fixes: 0d0b660f ("nvme: add ANA support")
Signed-off-by: default avatarAnton Eidelman <anton@lightbitslabs.com>
Signed-off-by: default avatarSagi Grimberg <sagi@grimberg.me>
Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
parent 489dd102
...@@ -409,11 +409,10 @@ static void nvme_mpath_set_live(struct nvme_ns *ns) ...@@ -409,11 +409,10 @@ static void nvme_mpath_set_live(struct nvme_ns *ns)
{ {
struct nvme_ns_head *head = ns->head; struct nvme_ns_head *head = ns->head;
lockdep_assert_held(&ns->head->lock);
if (!head->disk) if (!head->disk)
return; return;
mutex_lock(&head->lock);
if (!(head->disk->flags & GENHD_FL_UP)) if (!(head->disk->flags & GENHD_FL_UP))
device_add_disk(&head->subsys->dev, head->disk, device_add_disk(&head->subsys->dev, head->disk,
nvme_ns_id_attr_groups); nvme_ns_id_attr_groups);
...@@ -426,9 +425,10 @@ static void nvme_mpath_set_live(struct nvme_ns *ns) ...@@ -426,9 +425,10 @@ static void nvme_mpath_set_live(struct nvme_ns *ns)
__nvme_find_path(head, node); __nvme_find_path(head, node);
srcu_read_unlock(&head->srcu, srcu_idx); srcu_read_unlock(&head->srcu, srcu_idx);
} }
mutex_unlock(&head->lock);
synchronize_srcu(&ns->head->srcu); synchronize_srcu(&head->srcu);
kblockd_schedule_work(&ns->head->requeue_work); kblockd_schedule_work(&head->requeue_work);
} }
static int nvme_parse_ana_log(struct nvme_ctrl *ctrl, void *data, static int nvme_parse_ana_log(struct nvme_ctrl *ctrl, void *data,
...@@ -483,14 +483,12 @@ static inline bool nvme_state_is_live(enum nvme_ana_state state) ...@@ -483,14 +483,12 @@ static inline bool nvme_state_is_live(enum nvme_ana_state state)
static void nvme_update_ns_ana_state(struct nvme_ana_group_desc *desc, static void nvme_update_ns_ana_state(struct nvme_ana_group_desc *desc,
struct nvme_ns *ns) struct nvme_ns *ns)
{ {
mutex_lock(&ns->head->lock);
ns->ana_grpid = le32_to_cpu(desc->grpid); ns->ana_grpid = le32_to_cpu(desc->grpid);
ns->ana_state = desc->state; ns->ana_state = desc->state;
clear_bit(NVME_NS_ANA_PENDING, &ns->flags); clear_bit(NVME_NS_ANA_PENDING, &ns->flags);
if (nvme_state_is_live(ns->ana_state)) if (nvme_state_is_live(ns->ana_state))
nvme_mpath_set_live(ns); nvme_mpath_set_live(ns);
mutex_unlock(&ns->head->lock);
} }
static int nvme_update_ana_state(struct nvme_ctrl *ctrl, static int nvme_update_ana_state(struct nvme_ctrl *ctrl,
...@@ -669,10 +667,8 @@ void nvme_mpath_add_disk(struct nvme_ns *ns, struct nvme_id_ns *id) ...@@ -669,10 +667,8 @@ void nvme_mpath_add_disk(struct nvme_ns *ns, struct nvme_id_ns *id)
nvme_update_ns_ana_state(&desc, ns); nvme_update_ns_ana_state(&desc, ns);
} }
} else { } else {
mutex_lock(&ns->head->lock);
ns->ana_state = NVME_ANA_OPTIMIZED; ns->ana_state = NVME_ANA_OPTIMIZED;
nvme_mpath_set_live(ns); nvme_mpath_set_live(ns);
mutex_unlock(&ns->head->lock);
} }
if (bdi_cap_stable_pages_required(ns->queue->backing_dev_info)) { if (bdi_cap_stable_pages_required(ns->queue->backing_dev_info)) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment