* [PATCH 0/2] nvme-multipath: fixes for non-optimized paths
@ 2020-07-27 16:08 Hannes Reinecke
2020-07-27 16:08 ` [PATCH 1/2] nvme-multipath: fix logic " Hannes Reinecke
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Hannes Reinecke @ 2020-07-27 16:08 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Hannes Reinecke, linux-nvme, Sagi Grimberg, Keith Busch, Martin Wilck
Hi all,
one of our partners informed us that the round-robin path selector
didn't do the right thing when only one optimized path (or, indeed,
none at all) are present.
These patches will fixup nvme_round_robin() to handle the case when
less than two optimized (but several non-optimized) paths are present.
These pathes are a rework of the earlier patches send by Martin Wilck.
As usual, comments and reviews are welcome.
Hannes Reinecke (1):
nvme-multipath: do not fall back to __nvme_find_path() for
non-optimized paths
Martin Wilck (1):
nvme-multipath: fix logic for non-optimized paths
drivers/nvme/host/multipath.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
--
2.16.4
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/2] nvme-multipath: fix logic for non-optimized paths 2020-07-27 16:08 [PATCH 0/2] nvme-multipath: fixes for non-optimized paths Hannes Reinecke @ 2020-07-27 16:08 ` Hannes Reinecke 2020-07-27 19:42 ` Sagi Grimberg 2020-07-27 16:08 ` [PATCH 2/2] nvme-multipath: do not fall back to __nvme_find_path() " Hannes Reinecke 2020-07-28 10:51 ` [PATCH 0/2] nvme-multipath: fixes " Christoph Hellwig 2 siblings, 1 reply; 12+ messages in thread From: Hannes Reinecke @ 2020-07-27 16:08 UTC (permalink / raw) To: Christoph Hellwig Cc: Hannes Reinecke, linux-nvme, Sagi Grimberg, Keith Busch, Martin Wilck From: Martin Wilck <mwilck@suse.com> Handle the special case where we have exactly one optimized path, which we should keep using in this case. Signed off-by: Martin Wilck <mwilck@suse.com> Signed-off-by: Hannes Reinecke <hare@suse.de> --- 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 66509472fe06..fe8f7f123fac 100644 --- a/drivers/nvme/host/multipath.c +++ b/drivers/nvme/host/multipath.c @@ -246,6 +246,12 @@ static struct nvme_ns *nvme_round_robin_path(struct nvme_ns_head *head, fallback = ns; } + /* No optimized path found, re-check the current path */ + if (!nvme_path_is_disabled(old) && + old->ana_state == NVME_ANA_OPTIMIZED) { + found = old; + goto out; + } if (!fallback) return NULL; found = fallback; -- 2.16.4 _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] nvme-multipath: fix logic for non-optimized paths 2020-07-27 16:08 ` [PATCH 1/2] nvme-multipath: fix logic " Hannes Reinecke @ 2020-07-27 19:42 ` Sagi Grimberg 0 siblings, 0 replies; 12+ messages in thread From: Sagi Grimberg @ 2020-07-27 19:42 UTC (permalink / raw) To: Hannes Reinecke, Christoph Hellwig; +Cc: linux-nvme, Martin Wilck, Keith Busch Reviewed-by: Sagi Grimberg <sagi@grimberg.me> _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] nvme-multipath: do not fall back to __nvme_find_path() for non-optimized paths 2020-07-27 16:08 [PATCH 0/2] nvme-multipath: fixes for non-optimized paths Hannes Reinecke 2020-07-27 16:08 ` [PATCH 1/2] nvme-multipath: fix logic " Hannes Reinecke @ 2020-07-27 16:08 ` Hannes Reinecke 2020-07-27 19:46 ` Sagi Grimberg 2020-07-28 10:51 ` [PATCH 0/2] nvme-multipath: fixes " Christoph Hellwig 2 siblings, 1 reply; 12+ messages in thread From: Hannes Reinecke @ 2020-07-27 16:08 UTC (permalink / raw) To: Christoph Hellwig Cc: Hannes Reinecke, linux-nvme, Sagi Grimberg, Keith Busch, Martin Wilck When nvme_round_robin_path() finds a valid namespace we should be using it; falling back to __nvme_find_path() for non-optimized paths will cause the result from nvme_round_robin_path() to be ignored for non-optimized paths. Signed-off-by: Martin Wilck <mwilck@suse.com> Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/nvme/host/multipath.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c index fe8f7f123fac..97d3c19b5684 100644 --- a/drivers/nvme/host/multipath.c +++ b/drivers/nvme/host/multipath.c @@ -272,8 +272,11 @@ inline struct nvme_ns *nvme_find_path(struct nvme_ns_head *head) struct nvme_ns *ns; ns = srcu_dereference(head->current_path[node], &head->srcu); - if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_RR && ns) + if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_RR && ns) { ns = nvme_round_robin_path(head, node, ns); + if (ns) + return ns; + } if (unlikely(!ns || !nvme_path_is_optimized(ns))) ns = __nvme_find_path(head, node); return ns; -- 2.16.4 _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] nvme-multipath: do not fall back to __nvme_find_path() for non-optimized paths 2020-07-27 16:08 ` [PATCH 2/2] nvme-multipath: do not fall back to __nvme_find_path() " Hannes Reinecke @ 2020-07-27 19:46 ` Sagi Grimberg 2020-07-28 6:25 ` Hannes Reinecke 0 siblings, 1 reply; 12+ messages in thread From: Sagi Grimberg @ 2020-07-27 19:46 UTC (permalink / raw) To: Hannes Reinecke, Christoph Hellwig; +Cc: linux-nvme, Martin Wilck, Keith Busch On 7/27/20 9:08 AM, Hannes Reinecke wrote: > When nvme_round_robin_path() finds a valid namespace we should be using it; > falling back to __nvme_find_path() for non-optimized paths will cause the > result from nvme_round_robin_path() to be ignored for non-optimized paths. > > Signed-off-by: Martin Wilck <mwilck@suse.com> > Signed-off-by: Hannes Reinecke <hare@suse.de> > --- > drivers/nvme/host/multipath.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c > index fe8f7f123fac..97d3c19b5684 100644 > --- a/drivers/nvme/host/multipath.c > +++ b/drivers/nvme/host/multipath.c > @@ -272,8 +272,11 @@ inline struct nvme_ns *nvme_find_path(struct nvme_ns_head *head) > struct nvme_ns *ns; > > ns = srcu_dereference(head->current_path[node], &head->srcu); > - if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_RR && ns) > + if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_RR && ns) { > ns = nvme_round_robin_path(head, node, ns); > + if (ns) > + return ns; > + } > if (unlikely(!ns || !nvme_path_is_optimized(ns))) > ns = __nvme_find_path(head, node); Maybe instead of the early return put the following condition on else? _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] nvme-multipath: do not fall back to __nvme_find_path() for non-optimized paths 2020-07-27 19:46 ` Sagi Grimberg @ 2020-07-28 6:25 ` Hannes Reinecke 2020-07-28 6:43 ` Sagi Grimberg 2020-07-28 8:02 ` Christoph Hellwig 0 siblings, 2 replies; 12+ messages in thread From: Hannes Reinecke @ 2020-07-28 6:25 UTC (permalink / raw) To: Sagi Grimberg, Christoph Hellwig; +Cc: linux-nvme, Martin Wilck, Keith Busch On 7/27/20 9:46 PM, Sagi Grimberg wrote: > > > On 7/27/20 9:08 AM, Hannes Reinecke wrote: >> When nvme_round_robin_path() finds a valid namespace we should be >> using it; >> falling back to __nvme_find_path() for non-optimized paths will cause the >> result from nvme_round_robin_path() to be ignored for non-optimized >> paths. >> >> Signed-off-by: Martin Wilck <mwilck@suse.com> >> Signed-off-by: Hannes Reinecke <hare@suse.de> >> --- >> drivers/nvme/host/multipath.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/nvme/host/multipath.c >> b/drivers/nvme/host/multipath.c >> index fe8f7f123fac..97d3c19b5684 100644 >> --- a/drivers/nvme/host/multipath.c >> +++ b/drivers/nvme/host/multipath.c >> @@ -272,8 +272,11 @@ inline struct nvme_ns *nvme_find_path(struct >> nvme_ns_head *head) >> struct nvme_ns *ns; >> ns = srcu_dereference(head->current_path[node], &head->srcu); >> - if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_RR && ns) >> + if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_RR && ns) { >> ns = nvme_round_robin_path(head, node, ns); >> + if (ns) >> + return ns; >> + } >> if (unlikely(!ns || !nvme_path_is_optimized(ns))) >> ns = __nvme_find_path(head, node); > > Maybe instead of the early return put the following condition on else? That depends on whether we want to have a fallback to __nvme_find_path() or not. With your suggestion we would lose that; I'd rather keep it. Cheers, Hannes -- Dr. Hannes Reinecke Teamlead Storage & Networking 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] 12+ messages in thread
* Re: [PATCH 2/2] nvme-multipath: do not fall back to __nvme_find_path() for non-optimized paths 2020-07-28 6:25 ` Hannes Reinecke @ 2020-07-28 6:43 ` Sagi Grimberg 2020-07-28 8:02 ` Christoph Hellwig 1 sibling, 0 replies; 12+ messages in thread From: Sagi Grimberg @ 2020-07-28 6:43 UTC (permalink / raw) To: Hannes Reinecke, Christoph Hellwig; +Cc: linux-nvme, Martin Wilck, Keith Busch >>> When nvme_round_robin_path() finds a valid namespace we should be >>> using it; >>> falling back to __nvme_find_path() for non-optimized paths will cause >>> the >>> result from nvme_round_robin_path() to be ignored for non-optimized >>> paths. >>> >>> Signed-off-by: Martin Wilck <mwilck@suse.com> >>> Signed-off-by: Hannes Reinecke <hare@suse.de> >>> --- >>> drivers/nvme/host/multipath.c | 5 ++++- >>> 1 file changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/nvme/host/multipath.c >>> b/drivers/nvme/host/multipath.c >>> index fe8f7f123fac..97d3c19b5684 100644 >>> --- a/drivers/nvme/host/multipath.c >>> +++ b/drivers/nvme/host/multipath.c >>> @@ -272,8 +272,11 @@ inline struct nvme_ns *nvme_find_path(struct >>> nvme_ns_head *head) >>> struct nvme_ns *ns; >>> ns = srcu_dereference(head->current_path[node], &head->srcu); >>> - if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_RR && ns) >>> + if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_RR && ns) { >>> ns = nvme_round_robin_path(head, node, ns); >>> + if (ns) >>> + return ns; >>> + } >>> if (unlikely(!ns || !nvme_path_is_optimized(ns))) >>> ns = __nvme_find_path(head, node); >> >> Maybe instead of the early return put the following condition on else? > > That depends on whether we want to have a fallback to __nvme_find_path() > or not. With your suggestion we would lose that; I'd rather keep it. OK... Reviewed-by: Sagi Grimberg <sagi@grimberg.me> _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] nvme-multipath: do not fall back to __nvme_find_path() for non-optimized paths 2020-07-28 6:25 ` Hannes Reinecke 2020-07-28 6:43 ` Sagi Grimberg @ 2020-07-28 8:02 ` Christoph Hellwig 2020-07-28 8:14 ` Hannes Reinecke 1 sibling, 1 reply; 12+ messages in thread From: Christoph Hellwig @ 2020-07-28 8:02 UTC (permalink / raw) To: Hannes Reinecke Cc: Martin Wilck, linux-nvme, Sagi Grimberg, Keith Busch, Christoph Hellwig On Tue, Jul 28, 2020 at 08:25:18AM +0200, Hannes Reinecke wrote: >> Maybe instead of the early return put the following condition on else? > > That depends on whether we want to have a fallback to __nvme_find_path() or > not. With your suggestion we would lose that; I'd rather keep it. Why would we want to fall back? nvme_round_robin_path only returns NULL in case there is a single disabled path, in which case __nvme_find_path won't find anything either. I'd go for something like this: --- From 4f578364a3d15898c7d715315a3371b2f71db416 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke <hare@suse.de> Date: Mon, 27 Jul 2020 18:08:03 +0200 Subject: nvme-multipath: do not fall back to __nvme_find_path() for non-optimized paths When nvme_round_robin_path() finds a valid namespace we should be using it; falling back to __nvme_find_path() for non-optimized paths will cause the result from nvme_round_robin_path() to be ignored for non-optimized paths. Signed-off-by: Martin Wilck <mwilck@suse.com> Signed-off-by: Hannes Reinecke <hare@suse.de> Reviewed-by: Sagi Grimberg <sagi@grimberg.me> Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/nvme/host/multipath.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c index 93c70e1591de8f..3ded54d2c9c6ad 100644 --- a/drivers/nvme/host/multipath.c +++ b/drivers/nvme/host/multipath.c @@ -281,10 +281,13 @@ inline struct nvme_ns *nvme_find_path(struct nvme_ns_head *head) struct nvme_ns *ns; ns = srcu_dereference(head->current_path[node], &head->srcu); - if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_RR && ns) - ns = nvme_round_robin_path(head, node, ns); - if (unlikely(!ns || !nvme_path_is_optimized(ns))) - ns = __nvme_find_path(head, node); + if (unlikely(!ns)) + return __nvme_find_path(head, node); + + if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_RR) + return nvme_round_robin_path(head, node, ns); + if (unlikely(!nvme_path_is_optimized(ns))) + return __nvme_find_path(head, node); return ns; } -- 2.27.0 _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] nvme-multipath: do not fall back to __nvme_find_path() for non-optimized paths 2020-07-28 8:02 ` Christoph Hellwig @ 2020-07-28 8:14 ` Hannes Reinecke 2020-07-28 8:16 ` Christoph Hellwig 0 siblings, 1 reply; 12+ messages in thread From: Hannes Reinecke @ 2020-07-28 8:14 UTC (permalink / raw) To: Christoph Hellwig; +Cc: linux-nvme, Sagi Grimberg, Keith Busch, Martin Wilck On 7/28/20 10:02 AM, Christoph Hellwig wrote: > On Tue, Jul 28, 2020 at 08:25:18AM +0200, Hannes Reinecke wrote: >>> Maybe instead of the early return put the following condition on else? >> >> That depends on whether we want to have a fallback to __nvme_find_path() or >> not. With your suggestion we would lose that; I'd rather keep it. > > Why would we want to fall back? nvme_round_robin_path only > returns NULL in case there is a single disabled path, in which > case __nvme_find_path won't find anything either. I'd go for something > like this: > > --- > From 4f578364a3d15898c7d715315a3371b2f71db416 Mon Sep 17 00:00:00 2001 > From: Hannes Reinecke <hare@suse.de> > Date: Mon, 27 Jul 2020 18:08:03 +0200 > Subject: nvme-multipath: do not fall back to __nvme_find_path() for > non-optimized paths > > When nvme_round_robin_path() finds a valid namespace we should be using it; > falling back to __nvme_find_path() for non-optimized paths will cause the > result from nvme_round_robin_path() to be ignored for non-optimized paths. > > Signed-off-by: Martin Wilck <mwilck@suse.com> > Signed-off-by: Hannes Reinecke <hare@suse.de> > Reviewed-by: Sagi Grimberg <sagi@grimberg.me> > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > drivers/nvme/host/multipath.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c > index 93c70e1591de8f..3ded54d2c9c6ad 100644 > --- a/drivers/nvme/host/multipath.c > +++ b/drivers/nvme/host/multipath.c > @@ -281,10 +281,13 @@ inline struct nvme_ns *nvme_find_path(struct nvme_ns_head *head) > struct nvme_ns *ns; > > ns = srcu_dereference(head->current_path[node], &head->srcu); > - if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_RR && ns) > - ns = nvme_round_robin_path(head, node, ns); > - if (unlikely(!ns || !nvme_path_is_optimized(ns))) > - ns = __nvme_find_path(head, node); > + if (unlikely(!ns)) > + return __nvme_find_path(head, node); > + > + if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_RR) > + return nvme_round_robin_path(head, node, ns); > + if (unlikely(!nvme_path_is_optimized(ns))) > + return __nvme_find_path(head, node); > return ns; > } > > Yes, that would work, too. Should I resend? Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.de +49 911 74053 688 SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), GF: 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] 12+ messages in thread
* Re: [PATCH 2/2] nvme-multipath: do not fall back to __nvme_find_path() for non-optimized paths 2020-07-28 8:14 ` Hannes Reinecke @ 2020-07-28 8:16 ` Christoph Hellwig 2020-07-28 8:18 ` Hannes Reinecke 0 siblings, 1 reply; 12+ messages in thread From: Christoph Hellwig @ 2020-07-28 8:16 UTC (permalink / raw) To: Hannes Reinecke Cc: Martin Wilck, linux-nvme, Christoph Hellwig, Keith Busch, Sagi Grimberg On Tue, Jul 28, 2020 at 10:14:14AM +0200, Hannes Reinecke wrote: > On 7/28/20 10:02 AM, Christoph Hellwig wrote: > > On Tue, Jul 28, 2020 at 08:25:18AM +0200, Hannes Reinecke wrote: > >>> Maybe instead of the early return put the following condition on else? > >> > >> That depends on whether we want to have a fallback to __nvme_find_path() or > >> not. With your suggestion we would lose that; I'd rather keep it. > > > > Why would we want to fall back? nvme_round_robin_path only > > returns NULL in case there is a single disabled path, in which > > case __nvme_find_path won't find anything either. I'd go for something > > like this: > > > > --- > > From 4f578364a3d15898c7d715315a3371b2f71db416 Mon Sep 17 00:00:00 2001 > > From: Hannes Reinecke <hare@suse.de> > > Date: Mon, 27 Jul 2020 18:08:03 +0200 > > Subject: nvme-multipath: do not fall back to __nvme_find_path() for > > non-optimized paths > > > > When nvme_round_robin_path() finds a valid namespace we should be using it; > > falling back to __nvme_find_path() for non-optimized paths will cause the > > result from nvme_round_robin_path() to be ignored for non-optimized paths. > > > > Signed-off-by: Martin Wilck <mwilck@suse.com> > > Signed-off-by: Hannes Reinecke <hare@suse.de> > > Reviewed-by: Sagi Grimberg <sagi@grimberg.me> > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > --- > > drivers/nvme/host/multipath.c | 11 +++++++---- > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c > > index 93c70e1591de8f..3ded54d2c9c6ad 100644 > > --- a/drivers/nvme/host/multipath.c > > +++ b/drivers/nvme/host/multipath.c > > @@ -281,10 +281,13 @@ inline struct nvme_ns *nvme_find_path(struct nvme_ns_head *head) > > struct nvme_ns *ns; > > > > ns = srcu_dereference(head->current_path[node], &head->srcu); > > - if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_RR && ns) > > - ns = nvme_round_robin_path(head, node, ns); > > - if (unlikely(!ns || !nvme_path_is_optimized(ns))) > > - ns = __nvme_find_path(head, node); > > + if (unlikely(!ns)) > > + return __nvme_find_path(head, node); > > + > > + if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_RR) > > + return nvme_round_robin_path(head, node, ns); > > + if (unlikely(!nvme_path_is_optimized(ns))) > > + return __nvme_find_path(head, node); > > return ns; > > } > > > > > > Yes, that would work, too. > > Should I resend? No need, I can apply it with the modification. But if you have a RR test setup it would be great to run it through that again, just in case. _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] nvme-multipath: do not fall back to __nvme_find_path() for non-optimized paths 2020-07-28 8:16 ` Christoph Hellwig @ 2020-07-28 8:18 ` Hannes Reinecke 0 siblings, 0 replies; 12+ messages in thread From: Hannes Reinecke @ 2020-07-28 8:18 UTC (permalink / raw) To: Christoph Hellwig; +Cc: linux-nvme, Sagi Grimberg, Keith Busch, Martin Wilck On 7/28/20 10:16 AM, Christoph Hellwig wrote: > On Tue, Jul 28, 2020 at 10:14:14AM +0200, Hannes Reinecke wrote: >> On 7/28/20 10:02 AM, Christoph Hellwig wrote: >>> On Tue, Jul 28, 2020 at 08:25:18AM +0200, Hannes Reinecke wrote: >>>>> Maybe instead of the early return put the following condition on else? >>>> >>>> That depends on whether we want to have a fallback to __nvme_find_path() or >>>> not. With your suggestion we would lose that; I'd rather keep it. >>> >>> Why would we want to fall back? nvme_round_robin_path only >>> returns NULL in case there is a single disabled path, in which >>> case __nvme_find_path won't find anything either. I'd go for something >>> like this: >>> >>> --- >>> From 4f578364a3d15898c7d715315a3371b2f71db416 Mon Sep 17 00:00:00 2001 >>> From: Hannes Reinecke <hare@suse.de> >>> Date: Mon, 27 Jul 2020 18:08:03 +0200 >>> Subject: nvme-multipath: do not fall back to __nvme_find_path() for >>> non-optimized paths >>> >>> When nvme_round_robin_path() finds a valid namespace we should be using it; >>> falling back to __nvme_find_path() for non-optimized paths will cause the >>> result from nvme_round_robin_path() to be ignored for non-optimized paths. >>> >>> Signed-off-by: Martin Wilck <mwilck@suse.com> >>> Signed-off-by: Hannes Reinecke <hare@suse.de> >>> Reviewed-by: Sagi Grimberg <sagi@grimberg.me> >>> Signed-off-by: Christoph Hellwig <hch@lst.de> >>> --- >>> drivers/nvme/host/multipath.c | 11 +++++++---- >>> 1 file changed, 7 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c >>> index 93c70e1591de8f..3ded54d2c9c6ad 100644 >>> --- a/drivers/nvme/host/multipath.c >>> +++ b/drivers/nvme/host/multipath.c >>> @@ -281,10 +281,13 @@ inline struct nvme_ns *nvme_find_path(struct nvme_ns_head *head) >>> struct nvme_ns *ns; >>> >>> ns = srcu_dereference(head->current_path[node], &head->srcu); >>> - if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_RR && ns) >>> - ns = nvme_round_robin_path(head, node, ns); >>> - if (unlikely(!ns || !nvme_path_is_optimized(ns))) >>> - ns = __nvme_find_path(head, node); >>> + if (unlikely(!ns)) >>> + return __nvme_find_path(head, node); >>> + >>> + if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_RR) >>> + return nvme_round_robin_path(head, node, ns); >>> + if (unlikely(!nvme_path_is_optimized(ns))) >>> + return __nvme_find_path(head, node); >>> return ns; >>> } >>> >>> >> >> Yes, that would work, too. >> >> Should I resend? > > No need, I can apply it with the modification. But if you have a > RR test setup it would be great to run it through that again, just in > case. > I'm at it. Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.de +49 911 74053 688 SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), GF: 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] 12+ messages in thread
* Re: [PATCH 0/2] nvme-multipath: fixes for non-optimized paths 2020-07-27 16:08 [PATCH 0/2] nvme-multipath: fixes for non-optimized paths Hannes Reinecke 2020-07-27 16:08 ` [PATCH 1/2] nvme-multipath: fix logic " Hannes Reinecke 2020-07-27 16:08 ` [PATCH 2/2] nvme-multipath: do not fall back to __nvme_find_path() " Hannes Reinecke @ 2020-07-28 10:51 ` Christoph Hellwig 2 siblings, 0 replies; 12+ messages in thread From: Christoph Hellwig @ 2020-07-28 10:51 UTC (permalink / raw) To: Hannes Reinecke Cc: Martin Wilck, linux-nvme, Christoph Hellwig, Keith Busch, Sagi Grimberg Applied to nvme-5.9 with the tweak discussed earlier. _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-07-28 10:51 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-27 16:08 [PATCH 0/2] nvme-multipath: fixes for non-optimized paths Hannes Reinecke 2020-07-27 16:08 ` [PATCH 1/2] nvme-multipath: fix logic " Hannes Reinecke 2020-07-27 19:42 ` Sagi Grimberg 2020-07-27 16:08 ` [PATCH 2/2] nvme-multipath: do not fall back to __nvme_find_path() " Hannes Reinecke 2020-07-27 19:46 ` Sagi Grimberg 2020-07-28 6:25 ` Hannes Reinecke 2020-07-28 6:43 ` Sagi Grimberg 2020-07-28 8:02 ` Christoph Hellwig 2020-07-28 8:14 ` Hannes Reinecke 2020-07-28 8:16 ` Christoph Hellwig 2020-07-28 8:18 ` Hannes Reinecke 2020-07-28 10:51 ` [PATCH 0/2] nvme-multipath: fixes " Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).