nvme: Register for ANA AEN

PATCH New Hannes Reinecke 2 participants 8 messages Jul 26, 2018 NVMe

lore

Share & tools

lore

Hannes Reinecke

author

2018-07-26 10:55 UTC #0 lore raw

PATCH 1/1 nvme: Register for ANA AEN

We need to register for ANA AENs as we rely on them for our ANA implementation. Patch is relative to hch nvme-ana tree.

Signed-off-by Hannes Reinecke <hare at suse.com>

---
 drivers/nvme/host/core.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/nvme/host/core.c +4 -0

index 3b96ace1f86d..99910497acdc 100644
@@ -1042,6 +1042,10 @@ static void nvme_enable_aen(struct nvme_ctrl *ctrl)
 	u32 supported = ctrl->oaes & NVME_AEN_SUPPORTED, result;
 	int status;
 
+#ifdef CONFIG_NVME_MULTIPATH
+	if (ctrl->subsys->cmic & (1 << 3))
+		supported |= NVME_AEN_CFG_ANA_CHANGE;
+#endif
 	status = nvme_set_features(ctrl, NVME_FEAT_ASYNC_EVENT, supported, NULL,
 			0, &result);
 	if (status)

Signature · 1 line

2.12.3

Hannes Reinecke

author

2018-07-26 10:58 UTC #1 lore raw

PATCH 1/1 nvme: Register for ANA AEN

Quoted 14 lines · Hannes Reinecke

We need to register for ANA AENs as we rely on them for our ANA implementation. Patch is relative to hch nvme-ana tree.
Signed-off-by Hannes Reinecke <hare at suse.com>
---
 drivers/nvme/host/core.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/nvme/host/core.c +0 -0
index 3b96ace1f86d..99910497acdc 100644

drivers/nvme/host/core.c static void nvme_enable_aen(struct nvme_ctrl *ctrl) · line 1051

Quoted 10 lines

drivers/nvme/host/core.c +4 -0

@@ -1042,6 +1042,10 @@ static void nvme_enable_aen(struct nvme_ctrl *ctrl)
 	u32 supported = ctrl->oaes & NVME_AEN_SUPPORTED, result;
 	int status;
 
+#ifdef CONFIG_NVME_MULTIPATH
+	if (ctrl->subsys->cmic & (1 << 3))
+		supported |= NVME_AEN_CFG_ANA_CHANGE;
+#endif
 	status = nvme_set_features(ctrl, NVME_FEAT_ASYNC_EVENT, supported, NULL,
 			0, &result);
 	if (status)

Christoph,

this patch fixes the hang I've seen when running my ANA blktest.

Cheers,

Hannes

Signature · 5 lines

Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare at suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: F. Imend?rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG N?rnberg)

Christoph Hellwig

maintainer

2018-07-26 11:59 UTC #2 lore raw

PATCH 1/1 nvme: Register for ANA AEN

Quoted 27 lines · Hannes Reinecke

Quoted 14 lines · Hannes Reinecke
We need to register for ANA AENs as we rely on them for our ANA implementation. Patch is relative to hch nvme-ana tree.
Signed-off-by Hannes Reinecke <hare at suse.com>
---
 drivers/nvme/host/core.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/nvme/host/core.c +0 -0
index 3b96ace1f86d..99910497acdc 100644
drivers/nvme/host/core.c static void nvme_enable_aen(struct nvme_ctrl *ctrl) · line 1051
Quoted 10 lines
drivers/nvme/host/core.c +4 -0
@@ -1042,6 +1042,10 @@ static void nvme_enable_aen(struct nvme_ctrl *ctrl)
 	u32 supported = ctrl->oaes & NVME_AEN_SUPPORTED, result;
 	int status;
 
+#ifdef CONFIG_NVME_MULTIPATH
+	if (ctrl->subsys->cmic & (1 << 3))
+		supported |= NVME_AEN_CFG_ANA_CHANGE;
+#endif
 	status = nvme_set_features(ctrl, NVME_FEAT_ASYNC_EVENT, supported, NULL,
 			0, &result);
 	if (status)
Christoph,

Quoted 1 line

this patch fixes the hang I've seen when running my ANA blktest.

What target do you run against? With the current 4004a TP NVME_AEN_CFG_ANA_CHANGE should be set in OAES, which at least the Linux target always did anyway.

We could work around this bug in the original TP4004, but I'd rather not accumulate hacks like this.

Christoph Hellwig

maintainer

2018-07-26 12:01 UTC #3 lore raw

PATCH 1/1 nvme: Register for ANA AEN

Quoted 3 lines · Christoph Hellwig

What target do you run against? With the current 4004a TP NVME_AEN_CFG_ANA_CHANGE should be set in OAES, which at least the Linux target always did anyway.

Ah, but the patches I sent out are missing this host side fix:

drivers/nvme/host/core.c +1 -1

index e482f223f8fd..f6e52ffe6a6d 100644
@@ -1035,7 +1035,7 @@ int nvme_set_queue_count(struct nvme_ctrl *ctrl, int *count)
 EXPORT_SYMBOL_GPL(nvme_set_queue_count);
 
 #define NVME_AEN_SUPPORTED \
-	(NVME_AEN_CFG_NS_ATTR | NVME_AEN_CFG_FW_ACT)
+	(NVME_AEN_CFG_NS_ATTR | NVME_AEN_CFG_FW_ACT | NVME_AEN_CFG_ANA_CHANGE)
 
 static void nvme_enable_aen(struct nvme_ctrl *ctrl)
 {

Hannes Reinecke

author

2018-07-26 12:02 UTC #4 lore raw

PATCH 1/1 nvme: Register for ANA AEN

Quoted 13 lines · Christoph Hellwig

Quoted 3 lines · Christoph Hellwig
What target do you run against? With the current 4004a TP NVME_AEN_CFG_ANA_CHANGE should be set in OAES, which at least the Linux target always did anyway.
Ah, but the patches I sent out are missing this host side fix:
drivers/nvme/host/core.c +0 -0
index e482f223f8fd..f6e52ffe6a6d 100644

drivers/nvme/host/core.c int nvme_set_queue_count(struct nvme_ctrl *ctrl, int *count) · line 1041

Quoted 8 lines

drivers/nvme/host/core.c +1 -1

@@ -1035,7 +1035,7 @@ int nvme_set_queue_count(struct nvme_ctrl *ctrl, int *count)
 EXPORT_SYMBOL_GPL(nvme_set_queue_count);
 
 #define NVME_AEN_SUPPORTED \
-	(NVME_AEN_CFG_NS_ATTR | NVME_AEN_CFG_FW_ACT)
+	(NVME_AEN_CFG_NS_ATTR | NVME_AEN_CFG_FW_ACT | NVME_AEN_CFG_ANA_CHANGE)
 
 static void nvme_enable_aen(struct nvme_ctrl *ctrl)
 {

Ah. Maybe we should fix it up for the final submission...

Cheers,

Hannes

Signature · 5 lines

Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare at suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: F. Imend?rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG N?rnberg)

Christoph Hellwig

maintainer

2018-07-26 12:07 UTC #5 lore raw

PATCH 1/1 nvme: Register for ANA AEN

Quoted 15 lines · Hannes Reinecke

Quoted 14 lines · Christoph Hellwig
Quoted 3 lines · Christoph Hellwig
What target do you run against? With the current 4004a TP NVME_AEN_CFG_ANA_CHANGE should be set in OAES, which at least the Linux target always did anyway.
Ah, but the patches I sent out are missing this host side fix:
drivers/nvme/host/core.c +0 -0
index e482f223f8fd..f6e52ffe6a6d 100644
@@ -1035,7 +1035,7 @@ int nvme_set_queue_count(struct nvme_ctrl *ctrl, int *count)
 EXPORT_SYMBOL_GPL(nvme_set_queue_count);
 

Quoted 8 lines

drivers/nvme/host/core.c int nvme_set_queue_count(struct nvme_ctrl *ctrl, int *count) · line 1041
Quoted 7 lines
drivers/nvme/host/core.c +1 -1
 #define NVME_AEN_SUPPORTED \
-	(NVME_AEN_CFG_NS_ATTR | NVME_AEN_CFG_FW_ACT)
+	(NVME_AEN_CFG_NS_ATTR | NVME_AEN_CFG_FW_ACT | NVME_AEN_CFG_ANA_CHANGE)
 
 static void nvme_enable_aen(struct nvme_ctrl *ctrl)
 {
Ah. Maybe we should fix it up for the final submission...

I'll send out another round later today.

Hannes Reinecke

author

2018-07-26 13:37 UTC #6 lore raw

PATCH 1/1 nvme: Register for ANA AEN

Quoted 20 lines · Christoph Hellwig

Quoted 19 lines · Hannes Reinecke
Quoted 18 lines · Christoph Hellwig
Quoted 3 lines · Christoph Hellwig
What target do you run against? With the current 4004a TP NVME_AEN_CFG_ANA_CHANGE should be set in OAES, which at least the Linux target always did anyway.
Ah, but the patches I sent out are missing this host side fix:
drivers/nvme/host/core.c +1 -1
index e482f223f8fd..f6e52ffe6a6d 100644
@@ -1035,7 +1035,7 @@ int nvme_set_queue_count(struct nvme_ctrl *ctrl, int *count)
 EXPORT_SYMBOL_GPL(nvme_set_queue_count);
 
 #define NVME_AEN_SUPPORTED \
-	(NVME_AEN_CFG_NS_ATTR | NVME_AEN_CFG_FW_ACT)
+	(NVME_AEN_CFG_NS_ATTR | NVME_AEN_CFG_FW_ACT | NVME_AEN_CFG_ANA_CHANGE)
 

Quoted 7 lines

Quoted 4 lines
drivers/nvme/host/core.c int nvme_set_queue_count(struct nvme_ctrl *ctrl, int *count) · line 1041
Quoted 3 lines
drivers/nvme/host/core.c +0 -0
 static void nvme_enable_aen(struct nvme_ctrl *ctrl)
 {
Ah. Maybe we should fix it up for the final submission...
I'll send out another round later today.

Would it make sense to read back the features after we've tried to enable them?
It might be that the target doesn't enable ANA AENs (or doesn't support them), at which point we have to disable ANA on the host as we rely on ANA AENs for correct operations ...

Cheers,

Hannes

Signature · 5 lines

Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare at suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: F. Imend?rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG N?rnberg)

Christoph Hellwig

maintainer

2018-07-26 15:12 UTC #7 lore raw

PATCH 1/1 nvme: Register for ANA AEN

Quoted 7 lines · Hannes Reinecke

Quoted 2 lines
I'll send out another round later today.
Would it make sense to read back the features after we've tried to enable them?
It might be that the target doesn't enable ANA AENs (or doesn't support them), at which point we have to disable ANA on the host as we rely on ANA AENs for correct operations ...

If it doesn't support the AEN it won't be in OAES..