* [PATCH] nvme-multipath: call del_gendisk() in nvme_mpath_check_last_path()
@ 2021-05-31 7:09 Hannes Reinecke
2021-06-03 7:22 ` Christoph Hellwig
0 siblings, 1 reply; 4+ messages in thread
From: Hannes Reinecke @ 2021-05-31 7:09 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke, Yi Zhang
We cannot call del_gendisk() during the final nvme_put_ns_head(), as
this might be called from blkdev_put(). Doing so will deadlock on bd_mutex():
[ 1108.529767] __mutex_lock.constprop.0+0x2a4/0x470
[ 1108.534472] ? __kernfs_remove.part.0+0x174/0x1f0
[ 1108.539178] ? kernfs_remove_by_name_ns+0x5c/0x90
[ 1108.543885] del_gendisk+0x99/0x230
[ 1108.547378] nvme_mpath_remove_disk+0x97/0xb0 [nvme_core]
[ 1108.552787] nvme_put_ns_head+0x2a/0xb0 [nvme_core]
[ 1108.557664] __blkdev_put+0x115/0x160
[ 1108.561339] blkdev_put+0x4c/0x130
[ 1108.564745] blkdev_close+0x22/0x30
[ 1108.568238] __fput+0x94/0x240
So call 'del_gendisk()' when checking if the last path has been removed
to avoid this deadlock.
Reported-by: Yi Zhang <yi.zhang@redhat.com>
Tested-by: Yi Zhang <yi.zhang@redhat.com>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/nvme/host/multipath.c | 13 +++++++++++--
drivers/nvme/host/nvme.h | 9 +--------
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 127a17b4c13d..22e2febf86bc 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -776,14 +776,23 @@ void nvme_mpath_add_disk(struct nvme_ns *ns, struct nvme_id_ns *id)
#endif
}
-void nvme_mpath_remove_disk(struct nvme_ns_head *head)
+void nvme_mpath_check_last_path(struct nvme_ns *ns)
{
+ struct nvme_ns_head *head = ns->head;
+
if (!head->disk)
return;
- if (head->disk->flags & GENHD_FL_UP) {
+
+ if (list_empty(&head->list) && head->disk->flags & GENHD_FL_UP) {
nvme_cdev_del(&head->cdev, &head->cdev_device);
del_gendisk(head->disk);
}
+}
+
+void nvme_mpath_remove_disk(struct nvme_ns_head *head)
+{
+ if (!head->disk)
+ return;
blk_set_queue_dying(head->disk->queue);
/* make sure all pending bios are cleaned up */
kblockd_schedule_work(&head->requeue_work);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 1f397ecba16c..8a497a420ea2 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -716,14 +716,7 @@ void nvme_mpath_uninit(struct nvme_ctrl *ctrl);
void nvme_mpath_stop(struct nvme_ctrl *ctrl);
bool nvme_mpath_clear_current_path(struct nvme_ns *ns);
void nvme_mpath_clear_ctrl_paths(struct nvme_ctrl *ctrl);
-
-static inline void nvme_mpath_check_last_path(struct nvme_ns *ns)
-{
- struct nvme_ns_head *head = ns->head;
-
- if (head->disk && list_empty(&head->list))
- kblockd_schedule_work(&head->requeue_work);
-}
+void nvme_mpath_check_last_path(struct nvme_ns *ns);
static inline void nvme_trace_bio_complete(struct request *req)
{
--
2.29.2
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] nvme-multipath: call del_gendisk() in nvme_mpath_check_last_path()
2021-05-31 7:09 [PATCH] nvme-multipath: call del_gendisk() in nvme_mpath_check_last_path() Hannes Reinecke
@ 2021-06-03 7:22 ` Christoph Hellwig
2021-06-03 7:31 ` Christoph Hellwig
2021-06-07 9:46 ` Hannes Reinecke
0 siblings, 2 replies; 4+ messages in thread
From: Christoph Hellwig @ 2021-06-03 7:22 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme, Yi Zhang
> +void nvme_mpath_check_last_path(struct nvme_ns *ns)
> {
> + struct nvme_ns_head *head = ns->head;
> +
> if (!head->disk)
> return;
> - if (head->disk->flags & GENHD_FL_UP) {
> +
> + if (list_empty(&head->list) && head->disk->flags & GENHD_FL_UP) {
Please add the braces around the "&".
> nvme_cdev_del(&head->cdev, &head->cdev_device);
> del_gendisk(head->disk);
> }
> +}
Also it seems like we can just pass the head here, no need for ns.
> - if (head->disk && list_empty(&head->list))
> - kblockd_schedule_work(&head->requeue_work);
Did this get lost in the new version?
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] nvme-multipath: call del_gendisk() in nvme_mpath_check_last_path()
2021-06-03 7:22 ` Christoph Hellwig
@ 2021-06-03 7:31 ` Christoph Hellwig
2021-06-07 9:46 ` Hannes Reinecke
1 sibling, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2021-06-03 7:31 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme, Yi Zhang
As I had to rebase nvme-5.14 anyway, I've dropped the original patch to
keep the nshead around. Can you resend it including the fixup in a single
patch?
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] nvme-multipath: call del_gendisk() in nvme_mpath_check_last_path()
2021-06-03 7:22 ` Christoph Hellwig
2021-06-03 7:31 ` Christoph Hellwig
@ 2021-06-07 9:46 ` Hannes Reinecke
1 sibling, 0 replies; 4+ messages in thread
From: Hannes Reinecke @ 2021-06-07 9:46 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, Yi Zhang
On 6/3/21 9:22 AM, Christoph Hellwig wrote:
>> +void nvme_mpath_check_last_path(struct nvme_ns *ns)
>> {
>> + struct nvme_ns_head *head = ns->head;
>> +
>> if (!head->disk)
>> return;
>> - if (head->disk->flags & GENHD_FL_UP) {
>> +
>> + if (list_empty(&head->list) && head->disk->flags & GENHD_FL_UP) {
>
> Please add the braces around the "&".
>
Ok.
>> nvme_cdev_del(&head->cdev, &head->cdev_device);
>> del_gendisk(head->disk);
>> }
>> +}
>
> Also it seems like we can just pass the head here, no need for ns.
>
Ok.
>> - if (head->disk && list_empty(&head->list))
>> - kblockd_schedule_work(&head->requeue_work);
>
> Did this get lost in the new version?
>
I had removed it as it'll be called from nvme_mpath_remove_disk()
anyway, but seeing that the refcount might induce a different lifetime
I'll be re-adding it.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, 90409 Nürnberg
GF: F. Imendörffer, HRB 36809 (AG Nürnberg)
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-07 9:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-31 7:09 [PATCH] nvme-multipath: call del_gendisk() in nvme_mpath_check_last_path() Hannes Reinecke
2021-06-03 7:22 ` Christoph Hellwig
2021-06-03 7:31 ` Christoph Hellwig
2021-06-07 9:46 ` Hannes Reinecke
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.