* [PATCH] nvme-multipath: Reset bi_disk to ns head when failover
@ 2021-05-03 12:57 ` Daniel Wagner
0 siblings, 0 replies; 9+ messages in thread
From: Daniel Wagner @ 2021-05-03 12:57 UTC (permalink / raw)
To: linux-nvme
Cc: linux-kernel, Hannes Reinecke, Keith Busch, Jens Axboe,
Christoph Hellwig, Daniel Wagner
The path can be stale when we failover. If we don't reset the bdev to
the ns head and the I/O finally completes in end_io() it will triggers
a crash. By resetting the to ns head disk so that the submit path can
map the request to an active path.
Signed-off-by: Daniel Wagner <dwagner@suse.de>
---
The patch is against nvme-5.13.
[ 6552.155244] Call Trace:
[ 6552.155251] bio_endio+0x74/0x120
[ 6552.155260] nvme_ns_head_submit_bio+0x36f/0x3e0 [nvme_core]
[ 6552.155266] ? __switch_to_asm+0x34/0x70
[ 6552.155269] ? __switch_to_asm+0x40/0x70
[ 6552.155271] submit_bio_noacct+0x175/0x490
[ 6552.155274] ? __switch_to_asm+0x34/0x70
[ 6552.155277] ? __switch_to_asm+0x34/0x70
[ 6552.155284] ? nvme_requeue_work+0x5a/0x70 [nvme_core]
[ 6552.155290] nvme_requeue_work+0x5a/0x70 [nvme_core]
[ 6552.155296] process_one_work+0x1f4/0x3e0
[ 6552.155299] worker_thread+0x2d/0x3e0
[ 6552.155302] ? process_one_work+0x3e0/0x3e0
[ 6552.155305] kthread+0x10d/0x130
[ 6552.155307] ? kthread_park+0xa0/0xa0
[ 6552.155311] ret_from_fork+0x35/0x40
drivers/nvme/host/multipath.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 0d0de3433f37..0faf267faa58 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -69,7 +69,9 @@ void nvme_failover_req(struct request *req)
{
struct nvme_ns *ns = req->q->queuedata;
u16 status = nvme_req(req)->status & 0x7ff;
+ struct block_device *bdev;
unsigned long flags;
+ struct bio *bio;
nvme_mpath_clear_current_path(ns);
@@ -83,9 +85,13 @@ void nvme_failover_req(struct request *req)
queue_work(nvme_wq, &ns->ctrl->ana_work);
}
+ bdev = bdget_disk(ns->head->disk, 0);
spin_lock_irqsave(&ns->head->requeue_lock, flags);
+ for (bio = req->bio; bio; bio = bio->bi_next)
+ bio_set_dev(bio, bdev);
blk_steal_bios(&ns->head->requeue_list, req);
spin_unlock_irqrestore(&ns->head->requeue_lock, flags);
+ bdput(bdev);
blk_mq_end_request(req, 0);
kblockd_schedule_work(&ns->head->requeue_work);
--
2.29.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] nvme-multipath: Reset bi_disk to ns head when failover
2021-05-03 12:57 ` Daniel Wagner
@ 2021-05-03 13:33 ` Hannes Reinecke
-1 siblings, 0 replies; 9+ messages in thread
From: Hannes Reinecke @ 2021-05-03 13:33 UTC (permalink / raw)
To: Daniel Wagner, linux-nvme
Cc: linux-kernel, Keith Busch, Jens Axboe, Christoph Hellwig
On 5/3/21 2:57 PM, Daniel Wagner wrote:
> The path can be stale when we failover. If we don't reset the bdev to
> the ns head and the I/O finally completes in end_io() it will triggers
> a crash. By resetting the to ns head disk so that the submit path can
> map the request to an active path.
>
> Signed-off-by: Daniel Wagner <dwagner@suse.de>
> ---
>
> The patch is against nvme-5.13.
>
> [ 6552.155244] Call Trace:
> [ 6552.155251] bio_endio+0x74/0x120
> [ 6552.155260] nvme_ns_head_submit_bio+0x36f/0x3e0 [nvme_core]
> [ 6552.155266] ? __switch_to_asm+0x34/0x70
> [ 6552.155269] ? __switch_to_asm+0x40/0x70
> [ 6552.155271] submit_bio_noacct+0x175/0x490
> [ 6552.155274] ? __switch_to_asm+0x34/0x70
> [ 6552.155277] ? __switch_to_asm+0x34/0x70
> [ 6552.155284] ? nvme_requeue_work+0x5a/0x70 [nvme_core]
> [ 6552.155290] nvme_requeue_work+0x5a/0x70 [nvme_core]
> [ 6552.155296] process_one_work+0x1f4/0x3e0
> [ 6552.155299] worker_thread+0x2d/0x3e0
> [ 6552.155302] ? process_one_work+0x3e0/0x3e0
> [ 6552.155305] kthread+0x10d/0x130
> [ 6552.155307] ? kthread_park+0xa0/0xa0
> [ 6552.155311] ret_from_fork+0x35/0x40
>
> drivers/nvme/host/multipath.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index 0d0de3433f37..0faf267faa58 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -69,7 +69,9 @@ void nvme_failover_req(struct request *req)
> {
> struct nvme_ns *ns = req->q->queuedata;
> u16 status = nvme_req(req)->status & 0x7ff;
> + struct block_device *bdev;
> unsigned long flags;
> + struct bio *bio;
>
> nvme_mpath_clear_current_path(ns);
>
> @@ -83,9 +85,13 @@ void nvme_failover_req(struct request *req)
> queue_work(nvme_wq, &ns->ctrl->ana_work);
> }
>
> + bdev = bdget_disk(ns->head->disk, 0);
> spin_lock_irqsave(&ns->head->requeue_lock, flags);
> + for (bio = req->bio; bio; bio = bio->bi_next)
> + bio_set_dev(bio, bdev);
> blk_steal_bios(&ns->head->requeue_list, req);
> spin_unlock_irqrestore(&ns->head->requeue_lock, flags);
> + bdput(bdev);
>
> blk_mq_end_request(req, 0);
> kblockd_schedule_work(&ns->head->requeue_work);
>
Maybe a WARN_ON(!bdev) after bdget_disk(), but otherwise:
Reviewed-by: Hannes Reinecke <hare@suse.de>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] nvme-multipath: Reset bi_disk to ns head when failover
@ 2021-05-03 13:33 ` Hannes Reinecke
0 siblings, 0 replies; 9+ messages in thread
From: Hannes Reinecke @ 2021-05-03 13:33 UTC (permalink / raw)
To: Daniel Wagner, linux-nvme
Cc: linux-kernel, Keith Busch, Jens Axboe, Christoph Hellwig
On 5/3/21 2:57 PM, Daniel Wagner wrote:
> The path can be stale when we failover. If we don't reset the bdev to
> the ns head and the I/O finally completes in end_io() it will triggers
> a crash. By resetting the to ns head disk so that the submit path can
> map the request to an active path.
>
> Signed-off-by: Daniel Wagner <dwagner@suse.de>
> ---
>
> The patch is against nvme-5.13.
>
> [ 6552.155244] Call Trace:
> [ 6552.155251] bio_endio+0x74/0x120
> [ 6552.155260] nvme_ns_head_submit_bio+0x36f/0x3e0 [nvme_core]
> [ 6552.155266] ? __switch_to_asm+0x34/0x70
> [ 6552.155269] ? __switch_to_asm+0x40/0x70
> [ 6552.155271] submit_bio_noacct+0x175/0x490
> [ 6552.155274] ? __switch_to_asm+0x34/0x70
> [ 6552.155277] ? __switch_to_asm+0x34/0x70
> [ 6552.155284] ? nvme_requeue_work+0x5a/0x70 [nvme_core]
> [ 6552.155290] nvme_requeue_work+0x5a/0x70 [nvme_core]
> [ 6552.155296] process_one_work+0x1f4/0x3e0
> [ 6552.155299] worker_thread+0x2d/0x3e0
> [ 6552.155302] ? process_one_work+0x3e0/0x3e0
> [ 6552.155305] kthread+0x10d/0x130
> [ 6552.155307] ? kthread_park+0xa0/0xa0
> [ 6552.155311] ret_from_fork+0x35/0x40
>
> drivers/nvme/host/multipath.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index 0d0de3433f37..0faf267faa58 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -69,7 +69,9 @@ void nvme_failover_req(struct request *req)
> {
> struct nvme_ns *ns = req->q->queuedata;
> u16 status = nvme_req(req)->status & 0x7ff;
> + struct block_device *bdev;
> unsigned long flags;
> + struct bio *bio;
>
> nvme_mpath_clear_current_path(ns);
>
> @@ -83,9 +85,13 @@ void nvme_failover_req(struct request *req)
> queue_work(nvme_wq, &ns->ctrl->ana_work);
> }
>
> + bdev = bdget_disk(ns->head->disk, 0);
> spin_lock_irqsave(&ns->head->requeue_lock, flags);
> + for (bio = req->bio; bio; bio = bio->bi_next)
> + bio_set_dev(bio, bdev);
> blk_steal_bios(&ns->head->requeue_list, req);
> spin_unlock_irqrestore(&ns->head->requeue_lock, flags);
> + bdput(bdev);
>
> blk_mq_end_request(req, 0);
> kblockd_schedule_work(&ns->head->requeue_work);
>
Maybe a WARN_ON(!bdev) after bdget_disk(), but otherwise:
Reviewed-by: Hannes Reinecke <hare@suse.de>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] nvme-multipath: Reset bi_disk to ns head when failover
2021-05-03 12:57 ` Daniel Wagner
@ 2021-05-03 15:53 ` Christoph Hellwig
-1 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2021-05-03 15:53 UTC (permalink / raw)
To: Daniel Wagner
Cc: linux-nvme, linux-kernel, Hannes Reinecke, Keith Busch,
Jens Axboe, Christoph Hellwig
On Mon, May 03, 2021 at 02:57:41PM +0200, Daniel Wagner wrote:
> The path can be stale when we failover. If we don't reset the bdev to
> the ns head and the I/O finally completes in end_io() it will triggers
> a crash. By resetting the to ns head disk so that the submit path can
> map the request to an active path.
>
> Signed-off-by: Daniel Wagner <dwagner@suse.de>
> ---
>
> The patch is against nvme-5.13.
There is no bi_disk in nvme-5.13, but fortunately that slipped in only
in the subject.
> + bdev = bdget_disk(ns->head->disk, 0);
Please just use ns->head->disk->part0 directly. No need for the
reference and bdget_disk is about to go away.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] nvme-multipath: Reset bi_disk to ns head when failover
@ 2021-05-03 15:53 ` Christoph Hellwig
0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2021-05-03 15:53 UTC (permalink / raw)
To: Daniel Wagner
Cc: linux-nvme, linux-kernel, Hannes Reinecke, Keith Busch,
Jens Axboe, Christoph Hellwig
On Mon, May 03, 2021 at 02:57:41PM +0200, Daniel Wagner wrote:
> The path can be stale when we failover. If we don't reset the bdev to
> the ns head and the I/O finally completes in end_io() it will triggers
> a crash. By resetting the to ns head disk so that the submit path can
> map the request to an active path.
>
> Signed-off-by: Daniel Wagner <dwagner@suse.de>
> ---
>
> The patch is against nvme-5.13.
There is no bi_disk in nvme-5.13, but fortunately that slipped in only
in the subject.
> + bdev = bdget_disk(ns->head->disk, 0);
Please just use ns->head->disk->part0 directly. No need for the
reference and bdget_disk is about to go away.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] nvme-multipath: Reset bi_disk to ns head when failover
2021-05-03 12:57 ` Daniel Wagner
(?)
@ 2021-05-03 16:19 ` kernel test robot
-1 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2021-05-03 16:19 UTC (permalink / raw)
To: Daniel Wagner, linux-nvme
Cc: kbuild-all, linux-kernel, Hannes Reinecke, Keith Busch,
Jens Axboe, Christoph Hellwig, Daniel Wagner
[-- Attachment #1: Type: text/plain, Size: 1559 bytes --]
Hi Daniel,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linux/master]
[also build test ERROR on hch-configfs/for-next linus/master v5.12 next-20210503]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Daniel-Wagner/nvme-multipath-Reset-bi_disk-to-ns-head-when-failover/20210503-210018
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 1fe5501ba1abf2b7e78295df73675423bd6899a0
config: x86_64-randconfig-a006-20210503 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/603164e7d6800851ee5366bfaff31084453284bb
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Daniel-Wagner/nvme-multipath-Reset-bi_disk-to-ns-head-when-failover/20210503-210018
git checkout 603164e7d6800851ee5366bfaff31084453284bb
# save the attached .config to linux build tree
make W=1 W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "bdget_disk" [drivers/nvme/host/nvme-core.ko] undefined!
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29550 bytes --]
[-- Attachment #3: Type: text/plain, Size: 158 bytes --]
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] nvme-multipath: Reset bi_disk to ns head when failover
@ 2021-05-03 16:19 ` kernel test robot
0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2021-05-03 16:19 UTC (permalink / raw)
To: Daniel Wagner, linux-nvme
Cc: kbuild-all, linux-kernel, Hannes Reinecke, Keith Busch,
Jens Axboe, Christoph Hellwig, Daniel Wagner
[-- Attachment #1: Type: text/plain, Size: 1559 bytes --]
Hi Daniel,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linux/master]
[also build test ERROR on hch-configfs/for-next linus/master v5.12 next-20210503]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Daniel-Wagner/nvme-multipath-Reset-bi_disk-to-ns-head-when-failover/20210503-210018
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 1fe5501ba1abf2b7e78295df73675423bd6899a0
config: x86_64-randconfig-a006-20210503 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/603164e7d6800851ee5366bfaff31084453284bb
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Daniel-Wagner/nvme-multipath-Reset-bi_disk-to-ns-head-when-failover/20210503-210018
git checkout 603164e7d6800851ee5366bfaff31084453284bb
# save the attached .config to linux build tree
make W=1 W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "bdget_disk" [drivers/nvme/host/nvme-core.ko] undefined!
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29550 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] nvme-multipath: Reset bi_disk to ns head when failover
@ 2021-05-03 16:19 ` kernel test robot
0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2021-05-03 16:19 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 1593 bytes --]
Hi Daniel,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linux/master]
[also build test ERROR on hch-configfs/for-next linus/master v5.12 next-20210503]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Daniel-Wagner/nvme-multipath-Reset-bi_disk-to-ns-head-when-failover/20210503-210018
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 1fe5501ba1abf2b7e78295df73675423bd6899a0
config: x86_64-randconfig-a006-20210503 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/603164e7d6800851ee5366bfaff31084453284bb
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Daniel-Wagner/nvme-multipath-Reset-bi_disk-to-ns-head-when-failover/20210503-210018
git checkout 603164e7d6800851ee5366bfaff31084453284bb
# save the attached .config to linux build tree
make W=1 W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "bdget_disk" [drivers/nvme/host/nvme-core.ko] undefined!
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 29550 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread