[PATCH v2] NVMe: eliminate potential deadlock by nvme_get_ns_from_disk invoking nvme_free_ns

[Busch, Keith]

I can't be bothered to fix up my dev machines this week, so I get to reply with outlook. :(


> -----Original Message-----
> From: Christoph Hellwig [mailto:hch at lst.de]
> Sent: Wednesday, May 04, 2016 3:50 AM
> To: Wang Sheng-Hui
> Cc: hch at lst.de; sagig at mellanox.com; Busch, Keith; axboe at fb.com; linux-nvme at lists.infradead.org
> Subject: Re: [PATCH v2] NVMe: eliminate potential deadlock by nvme_get_ns_from_disk invoking
> nvme_free_ns

<snip>

> 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;

I was trying to suggest a smaller change by swapping the module and ns reference order, then unwinding failure is simpler without goto's:

---
	spin_lock(&dev_list_lock);
	ns = disk->private_data;
	if (ns) {
		if (!try_module_get(ns->ctrl->ops->module))
			ns = NULL;
		else if (!kref_get_unless_zero(&ns->kref)) {
			module_put(ns->ctrl->ops->module);
			ns = NULL;
		}
	}
	spin_unlock(&dev_list_lock);

	return ns;
--