[PATCH v2] NVMe: eliminate potential deadlock by nvme_get_ns_from_disk invoking nvme_free_ns
Christoph Hellwig
hch at lst.de
Wed May 4 02:50:04 PDT 2016
On Wed, May 04, 2016 at 09:08:34AM +0800, Wang Sheng-Hui wrote:
> Release dev_list_lock before enter nvme_free_ns from
> nvme_get_ns_from_disk to avoid potential deadlock.
>
> Signed-off-by: Wang Sheng-Hui <shhuiw at foxmail.com>
> ---
> drivers/nvme/host/core.c | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 643f457..ab12892 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -87,20 +87,17 @@ static struct nvme_ns *nvme_get_ns_from_disk(struct gendisk *disk)
> spin_lock(&dev_list_lock);
> ns = disk->private_data;
> if (ns) {
> + if (!try_module_get(ns->ctrl->ops->module)) {
> + spin_unlock(&dev_list_lock);
> + kref_put(&ns->kref, nvme_free_ns);
> + return NULL;
> + }
> if (!kref_get_unless_zero(&ns->kref))
> + ns = NULL;
> }
> spin_unlock(&dev_list_lock);
>
> return ns;
> }
This is incorrect - if kref_get_unless_zero fails we now fail to to to
drop the reference again. What about this variant instead?
ns = disk->private_data;
if (ns) {
if (!kref_get_unless_zero(&ns->kref))
ns = NULL;
else if (!try_module_get(ns->ctrl->ops->module))
goto out_put_ns;
}
spin_unlock(&dev_list_lock);
return ns;
out_put_ns:
spin_unlock(&dev_list_lock);
kref_put(&ns->kref, nvme_free_ns);
return NULL;
More information about the Linux-nvme
mailing list