nvmet: Fix possible infinite loop triggered on hot namespace removal
When removing a namespace we delete it from the subsystem namespaces list with list_del_init which allows us to know if it is enabled or not. The problem is that list_del_init initialize the list next and does not respect the RCU list-traversal we do on the IO path for locating a namespace. Instead we need to use list_del_rcu which is allowed to run concurrently with the _rcu list-traversal primitives (keeps list next intact) and guarantees concurrent nvmet_find_naespace forward progress. By changing that, we cannot rely on ns->dev_link for knowing if the namspace is enabled, so add enabled indicator entry to nvmet_ns for that. Signed-off-by: Sagi Grimberg <sagi@grimberg.me> Signed-off-by: Solganik Alexander <sashas@lightbitslabs.com> Cc: <stable@vger.kernel.org> # v4.8+
This commit is contained in:

committed by
Sagi Grimberg

parent
f3116d8f1e
commit
e4fcf07cca
@@ -271,7 +271,7 @@ static ssize_t nvmet_ns_device_path_store(struct config_item *item,
|
|||||||
|
|
||||||
mutex_lock(&subsys->lock);
|
mutex_lock(&subsys->lock);
|
||||||
ret = -EBUSY;
|
ret = -EBUSY;
|
||||||
if (nvmet_ns_enabled(ns))
|
if (ns->enabled)
|
||||||
goto out_unlock;
|
goto out_unlock;
|
||||||
|
|
||||||
kfree(ns->device_path);
|
kfree(ns->device_path);
|
||||||
@@ -307,7 +307,7 @@ static ssize_t nvmet_ns_device_nguid_store(struct config_item *item,
|
|||||||
int ret = 0;
|
int ret = 0;
|
||||||
|
|
||||||
mutex_lock(&subsys->lock);
|
mutex_lock(&subsys->lock);
|
||||||
if (nvmet_ns_enabled(ns)) {
|
if (ns->enabled) {
|
||||||
ret = -EBUSY;
|
ret = -EBUSY;
|
||||||
goto out_unlock;
|
goto out_unlock;
|
||||||
}
|
}
|
||||||
@@ -339,7 +339,7 @@ CONFIGFS_ATTR(nvmet_ns_, device_nguid);
|
|||||||
|
|
||||||
static ssize_t nvmet_ns_enable_show(struct config_item *item, char *page)
|
static ssize_t nvmet_ns_enable_show(struct config_item *item, char *page)
|
||||||
{
|
{
|
||||||
return sprintf(page, "%d\n", nvmet_ns_enabled(to_nvmet_ns(item)));
|
return sprintf(page, "%d\n", to_nvmet_ns(item)->enabled);
|
||||||
}
|
}
|
||||||
|
|
||||||
static ssize_t nvmet_ns_enable_store(struct config_item *item,
|
static ssize_t nvmet_ns_enable_store(struct config_item *item,
|
||||||
|
@@ -264,7 +264,7 @@ int nvmet_ns_enable(struct nvmet_ns *ns)
|
|||||||
int ret = 0;
|
int ret = 0;
|
||||||
|
|
||||||
mutex_lock(&subsys->lock);
|
mutex_lock(&subsys->lock);
|
||||||
if (!list_empty(&ns->dev_link))
|
if (ns->enabled)
|
||||||
goto out_unlock;
|
goto out_unlock;
|
||||||
|
|
||||||
ns->bdev = blkdev_get_by_path(ns->device_path, FMODE_READ | FMODE_WRITE,
|
ns->bdev = blkdev_get_by_path(ns->device_path, FMODE_READ | FMODE_WRITE,
|
||||||
@@ -309,6 +309,7 @@ int nvmet_ns_enable(struct nvmet_ns *ns)
|
|||||||
list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry)
|
list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry)
|
||||||
nvmet_add_async_event(ctrl, NVME_AER_TYPE_NOTICE, 0, 0);
|
nvmet_add_async_event(ctrl, NVME_AER_TYPE_NOTICE, 0, 0);
|
||||||
|
|
||||||
|
ns->enabled = true;
|
||||||
ret = 0;
|
ret = 0;
|
||||||
out_unlock:
|
out_unlock:
|
||||||
mutex_unlock(&subsys->lock);
|
mutex_unlock(&subsys->lock);
|
||||||
@@ -325,11 +326,11 @@ void nvmet_ns_disable(struct nvmet_ns *ns)
|
|||||||
struct nvmet_ctrl *ctrl;
|
struct nvmet_ctrl *ctrl;
|
||||||
|
|
||||||
mutex_lock(&subsys->lock);
|
mutex_lock(&subsys->lock);
|
||||||
if (list_empty(&ns->dev_link)) {
|
if (!ns->enabled)
|
||||||
mutex_unlock(&subsys->lock);
|
goto out_unlock;
|
||||||
return;
|
|
||||||
}
|
ns->enabled = false;
|
||||||
list_del_init(&ns->dev_link);
|
list_del_rcu(&ns->dev_link);
|
||||||
mutex_unlock(&subsys->lock);
|
mutex_unlock(&subsys->lock);
|
||||||
|
|
||||||
/*
|
/*
|
||||||
@@ -351,6 +352,7 @@ void nvmet_ns_disable(struct nvmet_ns *ns)
|
|||||||
|
|
||||||
if (ns->bdev)
|
if (ns->bdev)
|
||||||
blkdev_put(ns->bdev, FMODE_WRITE|FMODE_READ);
|
blkdev_put(ns->bdev, FMODE_WRITE|FMODE_READ);
|
||||||
|
out_unlock:
|
||||||
mutex_unlock(&subsys->lock);
|
mutex_unlock(&subsys->lock);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@@ -47,6 +47,7 @@ struct nvmet_ns {
|
|||||||
loff_t size;
|
loff_t size;
|
||||||
u8 nguid[16];
|
u8 nguid[16];
|
||||||
|
|
||||||
|
bool enabled;
|
||||||
struct nvmet_subsys *subsys;
|
struct nvmet_subsys *subsys;
|
||||||
const char *device_path;
|
const char *device_path;
|
||||||
|
|
||||||
@@ -61,11 +62,6 @@ static inline struct nvmet_ns *to_nvmet_ns(struct config_item *item)
|
|||||||
return container_of(to_config_group(item), struct nvmet_ns, group);
|
return container_of(to_config_group(item), struct nvmet_ns, group);
|
||||||
}
|
}
|
||||||
|
|
||||||
static inline bool nvmet_ns_enabled(struct nvmet_ns *ns)
|
|
||||||
{
|
|
||||||
return !list_empty_careful(&ns->dev_link);
|
|
||||||
}
|
|
||||||
|
|
||||||
struct nvmet_cq {
|
struct nvmet_cq {
|
||||||
u16 qid;
|
u16 qid;
|
||||||
u16 size;
|
u16 size;
|
||||||
|
Reference in New Issue
Block a user