* [PATCH] nvme: schedule requeue whenever a LIVE state is entered
@ 2019-03-21 14:10 Hannes Reinecke
2019-03-21 21:26 ` Sagi Grimberg
0 siblings, 1 reply; 8+ messages in thread
From: Hannes Reinecke @ 2019-03-21 14:10 UTC (permalink / raw)
When undergoing state transitions I/O might be requeued, hence
we always have to schedule requeue_work whenever the nvme device
is live, independent on whether the old state was live or not.
Reported-by: Martin George <martin.george at netapp.com>
Signed-off-by: Hannes Reinecke <hare at suse.com>
---
drivers/nvme/host/multipath.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 2839bb70badf..f69c9476d663 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -352,8 +352,6 @@ static void nvme_mpath_set_live(struct nvme_ns *ns)
__nvme_find_path(head, node);
srcu_read_unlock(&head->srcu, srcu_idx);
}
-
- kblockd_schedule_work(&ns->head->requeue_work);
}
static int nvme_parse_ana_log(struct nvme_ctrl *ctrl, void *data,
@@ -412,8 +410,11 @@ static void nvme_update_ns_ana_state(struct nvme_ana_group_desc *desc,
ns->ana_state = desc->state;
clear_bit(NVME_NS_ANA_PENDING, &ns->flags);
- if (nvme_state_is_live(ns->ana_state) && !nvme_state_is_live(old))
- nvme_mpath_set_live(ns);
+ if (nvme_state_is_live(ns->ana_state)) {
+ if (!nvme_state_is_live(old))
+ nvme_mpath_set_live(ns);
+ kblockd_schedule_work(&ns->head->requeue_work);
+ }
mutex_unlock(&ns->head->lock);
}
@@ -587,6 +588,7 @@ void nvme_mpath_add_disk(struct nvme_ns *ns, struct nvme_id_ns *id)
mutex_lock(&ns->head->lock);
ns->ana_state = NVME_ANA_OPTIMIZED;
nvme_mpath_set_live(ns);
+ kblockd_schedule_work(&ns->head->requeue_work);
mutex_unlock(&ns->head->lock);
}
}
--
2.16.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] nvme: schedule requeue whenever a LIVE state is entered
2019-03-21 14:10 [PATCH] nvme: schedule requeue whenever a LIVE state is entered Hannes Reinecke
@ 2019-03-21 21:26 ` Sagi Grimberg
2019-03-22 6:55 ` Hannes Reinecke
0 siblings, 1 reply; 8+ messages in thread
From: Sagi Grimberg @ 2019-03-21 21:26 UTC (permalink / raw)
> When undergoing state transitions I/O might be requeued, hence
> we always have to schedule requeue_work whenever the nvme device
> is live, independent on whether the old state was live or not.
Any reason why not simply update from live to live?
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] nvme: schedule requeue whenever a LIVE state is entered
2019-03-21 21:26 ` Sagi Grimberg
@ 2019-03-22 6:55 ` Hannes Reinecke
2019-03-22 18:57 ` Sagi Grimberg
0 siblings, 1 reply; 8+ messages in thread
From: Hannes Reinecke @ 2019-03-22 6:55 UTC (permalink / raw)
On 3/21/19 10:26 PM, Sagi Grimberg wrote:
>
>> When undergoing state transitions I/O might be requeued, hence
>> we always have to schedule requeue_work whenever the nvme device
>> is live, independent on whether the old state was live or not.
>
> Any reason why not simply update from live to live?
>
?
I somewhat fail to parse the answer.
Care to elaborate?
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare at suse.com +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: F. Imend?rffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG N?rnberg)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] nvme: schedule requeue whenever a LIVE state is entered
2019-03-22 6:55 ` Hannes Reinecke
@ 2019-03-22 18:57 ` Sagi Grimberg
2019-03-22 21:14 ` Hannes Reinecke
0 siblings, 1 reply; 8+ messages in thread
From: Sagi Grimberg @ 2019-03-22 18:57 UTC (permalink / raw)
>>> When undergoing state transitions I/O might be requeued, hence
>>> we always have to schedule requeue_work whenever the nvme device
>>> is live, independent on whether the old state was live or not.
>>
>> Any reason why not simply update from live to live?
>>
> ?
>
> I somewhat fail to parse the answer.
> Care to elaborate?
--
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 2839bb70badf..f0716f6ce41f 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -404,15 +404,12 @@ static inline bool nvme_state_is_live(enum
nvme_ana_state state)
static void nvme_update_ns_ana_state(struct nvme_ana_group_desc *desc,
struct nvme_ns *ns)
{
- enum nvme_ana_state old;
-
mutex_lock(&ns->head->lock);
- old = ns->ana_state;
ns->ana_grpid = le32_to_cpu(desc->grpid);
ns->ana_state = desc->state;
clear_bit(NVME_NS_ANA_PENDING, &ns->flags);
- if (nvme_state_is_live(ns->ana_state) && !nvme_state_is_live(old))
+ if (nvme_state_is_live(ns->ana_state))
nvme_mpath_set_live(ns);
mutex_unlock(&ns->head->lock);
}
--
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] nvme: schedule requeue whenever a LIVE state is entered
2019-03-22 18:57 ` Sagi Grimberg
@ 2019-03-22 21:14 ` Hannes Reinecke
2019-03-25 15:55 ` Sagi Grimberg
0 siblings, 1 reply; 8+ messages in thread
From: Hannes Reinecke @ 2019-03-22 21:14 UTC (permalink / raw)
On 3/22/19 7:57 PM, Sagi Grimberg wrote:
>>>> When undergoing state transitions I/O might be requeued, hence
>>>> we always have to schedule requeue_work whenever the nvme device
>>>> is live, independent on whether the old state was live or not.
>>>
>>> Any reason why not simply update from live to live?
>>>
>> ?
>>
>> I somewhat fail to parse the answer.
>> Care to elaborate?
>
> --
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index 2839bb70badf..f0716f6ce41f 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -404,15 +404,12 @@ static inline bool nvme_state_is_live(enum
> nvme_ana_state state)
> ?static void nvme_update_ns_ana_state(struct nvme_ana_group_desc *desc,
> ??????????????? struct nvme_ns *ns)
> ?{
> -?????? enum nvme_ana_state old;
> -
> ??????? mutex_lock(&ns->head->lock);
> -?????? old = ns->ana_state;
> ??????? ns->ana_grpid = le32_to_cpu(desc->grpid);
> ??????? ns->ana_state = desc->state;
> ??????? clear_bit(NVME_NS_ANA_PENDING, &ns->flags);
>
> -?????? if (nvme_state_is_live(ns->ana_state) && !nvme_state_is_live(old))
> +?????? if (nvme_state_is_live(ns->ana_state))
> ??????????????? nvme_mpath_set_live(ns);
> ??????? mutex_unlock(&ns->head->lock);
> ?}
> --
That's what we tried initially (cf thread "[PATCH] nvme-multipath: relax
ANA state check"), but got rejected by Christoph.
Cheers,
Hannes
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] nvme: schedule requeue whenever a LIVE state is entered
2019-03-22 21:14 ` Hannes Reinecke
@ 2019-03-25 15:55 ` Sagi Grimberg
2019-03-27 8:37 ` Christoph Hellwig
0 siblings, 1 reply; 8+ messages in thread
From: Sagi Grimberg @ 2019-03-25 15:55 UTC (permalink / raw)
> That's what we tried initially (cf thread "[PATCH] nvme-multipath: relax
> ANA state check"), but got rejected by Christoph.
IIRC Christoph was asking why was requests being requeued when moving
from live -> live, and I think you answered that in your patch
description..
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] nvme: schedule requeue whenever a LIVE state is entered
2019-03-25 15:55 ` Sagi Grimberg
@ 2019-03-27 8:37 ` Christoph Hellwig
2019-03-27 8:44 ` Hannes Reinecke
0 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2019-03-27 8:37 UTC (permalink / raw)
On Mon, Mar 25, 2019@08:55:47AM -0700, Sagi Grimberg wrote:
>
>> That's what we tried initially (cf thread "[PATCH] nvme-multipath: relax
>> ANA state check"), but got rejected by Christoph.
>
> IIRC Christoph was asking why was requests being requeued when moving
> from live -> live, and I think you answered that in your patch
> description..
Exactltly. Asking to please explain WTF is happening is not rejecting,
it is asking for more information to actually understand the issue.
So the patch from Georg with a changelog and comment in the code based
on yours seems like what we need.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] nvme: schedule requeue whenever a LIVE state is entered
2019-03-27 8:37 ` Christoph Hellwig
@ 2019-03-27 8:44 ` Hannes Reinecke
0 siblings, 0 replies; 8+ messages in thread
From: Hannes Reinecke @ 2019-03-27 8:44 UTC (permalink / raw)
On 3/27/19 9:37 AM, Christoph Hellwig wrote:
> On Mon, Mar 25, 2019@08:55:47AM -0700, Sagi Grimberg wrote:
>>
>>> That's what we tried initially (cf thread "[PATCH] nvme-multipath: relax
>>> ANA state check"), but got rejected by Christoph.
>>
>> IIRC Christoph was asking why was requests being requeued when moving
>> from live -> live, and I think you answered that in your patch
>> description..
>
> Exactly. Asking to please explain WTF is happening is not rejecting,
> it is asking for more information to actually understand the issue.
>
> So the patch from Georg with a changelog and comment in the code based
> on yours seems like what we need.
>
Okay, will be resending.
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare at suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG N?rnberg)
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-03-27 8:44 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-21 14:10 [PATCH] nvme: schedule requeue whenever a LIVE state is entered Hannes Reinecke
2019-03-21 21:26 ` Sagi Grimberg
2019-03-22 6:55 ` Hannes Reinecke
2019-03-22 18:57 ` Sagi Grimberg
2019-03-22 21:14 ` Hannes Reinecke
2019-03-25 15:55 ` Sagi Grimberg
2019-03-27 8:37 ` Christoph Hellwig
2019-03-27 8:44 ` 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.