nvme: ANA support

Hi all,

here's a prototype implementation for ANA support in nvme. The implementation itself is pretty straightforward; only the ANA group linking for NVMe target required some fiddling.

As usual, comments and reviews are welcome.

Hannes Reinecke (5):
nvmet: EUI64 support
nvme: Add ANA base definitions
nvmet: Add ANA base support
block: BLK_STS_NEXUS is a path failure
nvme: ANA base support

 drivers/nvme/host/core.c        | 122 ++++++++++++++-
 drivers/nvme/host/multipath.c   |  12 +-
 drivers/nvme/host/nvme.h        |   3 +
 drivers/nvme/target/admin-cmd.c |  75 ++++++++-
 drivers/nvme/target/configfs.c  | 328 ++++++++++++++++++++++++++++++++++++++++
 drivers/nvme/target/core.c      |  78 +++++++++-
 drivers/nvme/target/discovery.c |  16 ++
 drivers/nvme/target/io-cmd.c    |  10 ++
 drivers/nvme/target/nvmet.h     |  46 ++++++
 include/linux/blk_types.h       |   1 -
 include/linux/nvme.h            |  47 +++++-
 11 files changed, 723 insertions(+), 15 deletions(-)

2.12.3

Allow the setting of an IEEE Extended Unique Identifier (EUI64) for each namespace. As the OUI is per subsystem ensure that the OUI part of the EUI64 value (ie the top 3 bytes) are matching for each namespace.

---
 drivers/nvme/target/admin-cmd.c |  6 ++---
 drivers/nvme/target/configfs.c  | 58 +++++++++++++++++++++++++++++++++++++++++
 drivers/nvme/target/nvmet.h     |  2 ++
 3 files changed, 62 insertions(+), 4 deletions(-)

index 5e0e9fcc0d4d..a53259e1082e 100644
@@ -187,10 +187,7 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
 
 	id->rab = 6;
 
-	/*
-	 * XXX: figure out how we can assign a IEEE OUI, but until then
-	 * the safest is to leave it as zeroes.
-	 */
+	memcpy(id->ieee, &ctrl->subsys->oui, 3);
 
 	/* we support multiple ports and multiples hosts: */
 	id->cmic = (1 << 0) | (1 << 1);

@@ -314,6 +311,7 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
 	id->nmic = (1 << 0);
 
 	memcpy(&id->nguid, &ns->nguid, sizeof(uuid_le));
+	memcpy(&id->eui64, &ns->eui64, sizeof(id->eui64));
 
 	id->lbaf[0].ds = ns->blksize_shift;
 

index 15fd84ab21f8..8cd42ed37314 100644
@@ -47,6 +47,7 @@ struct nvmet_ns {
 	u32			blksize_shift;
 	loff_t			size;
 	u8			nguid[16];
+	u64			eui64;
 	uuid_t			uuid;
 
 	bool			enabled;

@@ -154,6 +155,7 @@ struct nvmet_subsys {
 
 	u64			ver;
 	u64			serial;
+	u32			oui;
 	char			*subsysnqn;
 
 	struct config_group	group;

2.12.3

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>

Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

I don't think we should support the EUI64 value. It doesn't have a lot of uniqueness, and it requires and IEEE OID to be properly implemented. Please use the UUID identifier always, unless you actually doe havean IEEE OID and really want to use it, in which case you should use the NGUID.

It's not so much about usability, it's about choice. Some users might want to set an EUI64; companies (like ours) have a valid OUI, so generating an EUI64 is pretty simple.

And the patch itself is pretty simple, so where's the harm?

Cheers,

Hannes

Please always use the NGUID then. Using a 64-bit unique identifier is a bad idea. And even when using NGUIDs you have to be extremely careful to make sure they are never reused.

...

Add definitions from the ANA TP errata.

---
 include/linux/nvme.h | 47 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 44 insertions(+), 3 deletions(-)

index 4112e2bd747f..40c2d08ee761 100644
@@ -242,7 +242,11 @@ struct nvme_id_ctrl {
 	__le32			sanicap;
 	__le32			hmminds;
 	__le16			hmmaxd;
-	__u8			rsvd338[174];
+	__u8			anatt;
+	__u8			anacap;
+	__le32			anagrpmax;
+	__le32			nanagrpid;
+	__u8			rsvd338[164];
 	__u8			sqes;
 	__u8			cqes;
 	__le16			maxcmd;

@@ -258,7 +262,8 @@ struct nvme_id_ctrl {
 	__le16			acwu;
 	__u8			rsvd534[2];
 	__le32			sgls;
-	__u8			rsvd540[228];
+	__le32			mnn;
+	__u8			rsvd540[224];
 	char			subnqn[256];
 	__u8			rsvd1024[768];
 	__le32			ioccsz;

@@ -312,7 +317,8 @@ struct nvme_id_ns {
 	__le16			nabspf;
 	__le16			noiob;
 	__u8			nvmcap[16];
-	__u8			rsvd64[40];
+	__u8			rsvd64[36];
+	__le32			anagrpid;
 	__u8			nguid[16];
 	__u8			eui64[8];
 	struct nvme_lbaf	lbaf[16];

@@ -440,6 +446,7 @@ enum {
 	NVME_AER_VS			= 7,
 	NVME_AER_NOTICE_NS_CHANGED	= 0x0002,
 	NVME_AER_NOTICE_FW_ACT_STARTING = 0x0102,
+	NVME_AER_NOTICE_ANA_CHANGE	= 0x0302,
 };
 
 struct nvme_lba_range_type {

@@ -748,11 +755,17 @@ enum {
 	NVME_LOG_SMART		= 0x02,
 	NVME_LOG_FW_SLOT	= 0x03,
 	NVME_LOG_CMD_EFFECTS	= 0x05,
+	NVME_LOG_ANA		= 0x0c,
 	NVME_LOG_DISC		= 0x70,
 	NVME_LOG_RESERVATION	= 0x80,
 	NVME_FWACT_REPL		= (0 << 3),
 	NVME_FWACT_REPL_ACTV	= (1 << 3),
 	NVME_FWACT_ACTV		= (2 << 3),
+	NVME_ANA_STATE_OPTIMIZED = 0x01,
+	NVME_ANA_STATE_NONOPTIMIZED = 0x02,
+	NVME_ANA_STATE_INACCESSIBLE = 0x03,
+	NVME_ANA_STATE_PERSISTENT_LOSS = 0x04,
+	NVME_ANA_STATE_CHANGE_STATE = 0x05,
 };
 
 struct nvme_identify {

@@ -917,6 +930,23 @@ struct nvmf_common_command {
 	__u8	ts[24];
 };
 
+/* Asymmetric Namespace Access log page entry */
+struct nvmf_ana_group_descriptor {
+	__le32 groupid;
+	__le32 nsid_num;
+	__le64 chgcnt;
+	__u8 ana_state;
+	__u8 resv1[7];
+	__le32 nsid[0];
+};
+
+struct nvmf_ana_rsp_page_header {
+	__le64 chgcnt;
+	__le16 grpid_num;
+	__le16 resv[3];
+	struct nvmf_ana_group_descriptor desc[0];
+};
+
 /*
  * The legal cntlid range a NVMe Target will provide.
  * Note that cntlid of value 0 is considered illegal in the fabrics world.

@@ -1135,6 +1165,8 @@ enum {
 	NVME_SC_NS_NOT_ATTACHED		= 0x11a,
 	NVME_SC_THIN_PROV_NOT_SUPP	= 0x11b,
 	NVME_SC_CTRL_LIST_INVALID	= 0x11c,
+	NVME_SC_GRP_ID_INVALID		= 0x123,
+	NVME_SC_ANA_ATTACH_FAILED	= 0x124,
 
 	/*
 	 * I/O Command Set Specific - NVM commands:

@@ -1168,6 +1200,15 @@ enum {
 	NVME_SC_ACCESS_DENIED		= 0x286,
 	NVME_SC_UNWRITTEN_BLOCK		= 0x287,
 
+	/* Path-related Errors */
+	NVME_SC_INTERNAL_PATH_ERROR	= 0x300,
+	NVME_SC_ANA_PERSISTENT_LOSS	= 0x301,
+	NVME_SC_ANA_INACCESSIBLE	= 0x302,
+	NVME_SC_ANA_TRANSITION		= 0x303,
+	NVME_SC_CTRL_PATHING_ERROR	= 0x360,
+	NVME_SC_HOST_PATHING_ERROR	= 0x370,
+	NVME_SC_HOST_ABORTED		= 0x371,
+
 	NVME_SC_DNR			= 0x4000,
 };
 

2.12.3

Agreed:

95:92 - ANAGRPID
103:96 - Reserved

Note that this was changed at the last minute. There are some older drafts of the NVM_TP_4004 out there. You want to be sure to use the ratified version from:

http://nvmexpress.org/resources/specifications/

NVM Express 1.3 Ratified TPs

/John

?On 5/4/18, 12:16 PM, "Keith Busch" <[email protected]> wrote:

    On Fri, May 04, 2018@01:28:42PM +0200, Hannes Reinecke wrote:
    > @@ -312,7 +317,8 @@ struct nvme_id_ns {
    >  	__le16			nabspf;
    >  	__le16			noiob;
    >  	__u8			nvmcap[16];
    > -	__u8			rsvd64[40];
    > +	__u8			rsvd64[36];
    > +	__le32			anagrpid;
    >  	__u8			nguid[16];
    >  	__u8			eui64[8];
    >  	struct nvme_lbaf	lbaf[16];
    
    Is this the right place for the new field? Your patch has the group ID
    at byte 100. TP4004 has the ANA Group Identifier starting at byte 92,
    followed by 8 reserved bytes, then the NGUID.

And here are several of the other fields around about that same area:

...
47:46 - NOIOB
63:48 - NVMCAP
91:64 - Reserved
95:92 - ANAGRPID
98:96 - Reserved
99 - NSATTR
101:100 - NVMSETID
103:102 - ENDGID
119:104 - NGUID
...

Fred

-----Original Message-----
From: Meneghini, John
Sent: Friday, May 04, 2018 1:04 PM
To: Keith Busch <keith.busch at linux.intel.com>; Hannes Reinecke <hare at suse.de>
Cc: Christoph Hellwig <hch at lst.de>; Jens Axboe <axboe at kernel.dk>; Hannes Reinecke <hare at suse.com>; Sagi Grimberg <sagi at grimberg.me>; linux-nvme at lists.infradead.org; Johannes Thumshirn <jth at kernel.org>; Knight, Frederick <Frederick.Knight at netapp.com>; Meneghini, John <John.Meneghini at netapp.com>
Subject: Re: [PATCH 2/5] nvme: Add ANA base definitions

Agreed:

95:92 - ANAGRPID
103:96 - Reserved

Note that this was changed at the last minute. There are some older drafts of the NVM_TP_4004 out there. You want to be sure to use the ratified version from:

http://nvmexpress.org/resources/specifications/

NVM Express 1.3 Ratified TPs

/John

?On 5/4/18, 12:16 PM, "Keith Busch" <[email protected]> wrote:

    On Fri, May 04, 2018@01:28:42PM +0200, Hannes Reinecke wrote:
    > @@ -312,7 +317,8 @@ struct nvme_id_ns {
    >  	__le16			nabspf;
    >  	__le16			noiob;
    >  	__u8			nvmcap[16];
    > -	__u8			rsvd64[40];
    > +	__u8			rsvd64[36];
    > +	__le32			anagrpid;
    >  	__u8			nguid[16];
    >  	__u8			eui64[8];
    >  	struct nvme_lbaf	lbaf[16];
    
    Is this the right place for the new field? Your patch has the group ID
    at byte 100. TP4004 has the ANA Group Identifier starting at byte 92,
    followed by 8 reserved bytes, then the NGUID.

Ah. Hence. Must've looked at an older copy then.

Will be fixing it up.

Cheers,

Hannes

Add ANA support to the nvme target. The ANA configuration is optional, and doesn't interfere with existing configurations.
Each subsystem has distinct ANA groups; if ANA groups are created it is required to link the ANA groups into the individual ports. Linking the entire subsystem will be refused if ANA groups are specified.
Also this implementation has a limit of one single ANA state per groups, irrespective of the path. So when distinct ANA states are required one needs to create different ANA groups.

---
 drivers/nvme/target/admin-cmd.c |  69 +++++++++-
 drivers/nvme/target/configfs.c  | 272 +++++++++++++++++++++++++++++++++++++++-
 drivers/nvme/target/core.c      |  78 +++++++++++-
 drivers/nvme/target/discovery.c |  16 +++
 drivers/nvme/target/io-cmd.c    |  10 ++
 drivers/nvme/target/nvmet.h     |  44 +++++++
 6 files changed, 483 insertions(+), 6 deletions(-)

index a53259e1082e..7e47527963e4 100644
@@ -102,6 +102,35 @@ static u16 nvmet_get_smart_log(struct nvmet_req *req,
 	return status;
 }
 
+static u16 nvmet_get_ana_log(struct nvmet_req *req,
+			     struct nvmf_ana_rsp_page_header *alog)
+{
+	struct nvmf_ana_group_descriptor *desc;
+	struct nvmet_ns *ns;
+	struct nvmet_ctrl *ctrl;
+	int nsid = 0;
+
+	ctrl = req->sq->ctrl;
+
+	put_unaligned_le64(ctrl->subsys->change_count, &alog->chgcnt);
+	/* We only have one ANA group per controller for now */
+	put_unaligned_le64(1, &alog->grpid_num);
+	desc = &alog->desc[0];
+
+	put_unaligned_le32(ctrl->ag->grpid, &desc->groupid);
+	put_unaligned_le64(ctrl->ag->change_count, &desc->chgcnt);
+	desc->ana_state = ctrl->ag->state;
+	rcu_read_lock();
+	list_for_each_entry_rcu(ns, &ctrl->subsys->namespaces, dev_link) {
+		put_unaligned_le32(ns->nsid, &desc->nsid[nsid]);
+		nsid++;
+	}
+	rcu_read_unlock();
+	put_unaligned_le32(nsid, &desc->nsid_num);
+
+	return NVME_SC_SUCCESS;
+}
+
 static void nvmet_execute_get_log_page(struct nvmet_req *req)
 {
 	struct nvme_smart_log *smart_log;

@@ -149,6 +178,11 @@ static void nvmet_execute_get_log_page(struct nvmet_req *req)
 		 * still claim to fully implement this mandatory log page.
 		 */
 		break;
+	case NVME_LOG_ANA:
+		status = nvmet_get_ana_log(req, buf);
+		if (status)
+			goto err;
+		break;
 	default:
 		BUG();
 	}

@@ -165,6 +199,8 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
 {
 	struct nvmet_ctrl *ctrl = req->sq->ctrl;
 	struct nvme_id_ctrl *id;
+	struct nvmet_ag *ag;
+	u32 num_ag = 0;
 	u16 status = 0;
 	const char model[] = "Linux";
 

@@ -189,9 +225,26 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
 
 	memcpy(id->ieee, &ctrl->subsys->oui, 3);
 
-	/* we support multiple ports and multiples hosts: */
+	rcu_read_lock();
+	list_for_each_entry_rcu(ag, &ctrl->subsys->ana_groups, entry)
+		num_ag++;
+	rcu_read_unlock();
+
+	/* we support multiple ports and multiple hosts */
 	id->cmic = (1 << 0) | (1 << 1);
 
+	/* ANA support */
+	if (num_ag > 0) {
+		id->cmic |= (1 << 3);
+
+		/* All ANA states are supported */
+		id->anacap = (1 << 0) | (1 << 1) | (1 << 2) | \
+			(1 << 3) | (1 << 4);
+		id->anatt = ctrl->subsys->ana_tt;
+		id->anagrpmax = cpu_to_le32((u32)-1);
+		id->nanagrpid = cpu_to_le32(num_ag);
+	}
+
 	/* no limit on data transfer sizes for now */
 	id->mdts = 0;
 	id->cntlid = cpu_to_le16(ctrl->cntlid);

@@ -277,6 +330,7 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
 	struct nvmet_ns *ns;
 	struct nvme_id_ns *id;
 	u16 status = 0;
+	u8 ag_state = NVME_ANA_STATE_OPTIMIZED;
 
 	ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid);
 	if (!ns) {

@@ -290,12 +344,24 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
 		goto out_put_ns;
 	}
 
+	if (req->sq->ctrl->ag) {
+		id->anagrpid = cpu_to_le32(req->sq->ctrl->ag->grpid);
+		ag_state = req->sq->ctrl->ag->state;
+	}
+
 	/*
 	 * nuse = ncap = nsze isn't always true, but we have no way to find
 	 * that out from the underlying device.
 	 */
 	id->ncap = id->nuse = id->nsze =
 		cpu_to_le64(ns->size >> ns->blksize_shift);
+	/*
+	 * nuse and nsze should be zero for inaccessible or
+	 * persistent loss ANA state.
+	 */
+	if ((ag_state == NVME_ANA_STATE_INACCESSIBLE) ||
+	    (ag_state == NVME_ANA_STATE_PERSISTENT_LOSS))
+		id->nuse = id->nsze = 0;
 
 	/*
 	 * We just provide a single LBA format that matches what the

@@ -560,6 +626,7 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req)
 		case NVME_LOG_ERROR:
 		case NVME_LOG_SMART:
 		case NVME_LOG_FW_SLOT:
+		case NVME_LOG_ANA:
 			req->execute = nvmet_execute_get_log_page;
 			return 0;
 		}

index 231e04e0a496..7caa46691702 100644
@@ -90,6 +90,7 @@ static void nvmet_execute_get_disc_log_page(struct nvmet_req *req)
 	size_t alloc_len = max(data_len, sizeof(*hdr));
 	int residual_len = data_len - sizeof(*hdr);
 	struct nvmet_subsys_link *p;
+	struct nvmet_ag_link *a;
 	struct nvmet_port *r;
 	u32 numrec = 0;
 	u16 status = 0;

@@ -106,6 +107,21 @@ static void nvmet_execute_get_disc_log_page(struct nvmet_req *req)
 	}
 
 	down_read(&nvmet_config_sem);
+	list_for_each_entry(a, &req->port->ags, entry) {
+		if (!nvmet_host_allowed(req, a->ag->subsys, ctrl->hostnqn))
+			continue;
+		if (residual_len >= entry_size) {
+			char traddr[NVMF_TRADDR_SIZE];
+
+			nvmet_set_disc_traddr(req, req->port, traddr);
+			nvmet_format_discovery_entry(hdr, req->port,
+					a->ag->subsys->subsysnqn, traddr,
+					NVME_NQN_NVME, numrec);
+			residual_len -= entry_size;
+		}
+		numrec++;
+	}
+
 	list_for_each_entry(p, &req->port->subsystems, entry) {
 		if (!nvmet_host_allowed(req, p->subsys, ctrl->hostnqn))
 			continue;

index cd2344179673..c25d459bb787 100644
@@ -204,6 +204,16 @@ u16 nvmet_parse_io_cmd(struct nvmet_req *req)
 	if (unlikely(!req->ns))
 		return NVME_SC_INVALID_NS | NVME_SC_DNR;
 
+	if (req->sq->ctrl->ag) {
+		struct nvmet_ag *ag = req->sq->ctrl->ag;
+
+		if (work_pending(&ag->state_change_work.work))
+			return NVME_SC_ANA_TRANSITION | NVME_SC_DNR;
+		if (ag->state == NVME_ANA_STATE_INACCESSIBLE)
+			return NVME_SC_ANA_INACCESSIBLE | NVME_SC_DNR;
+		if (ag->state == NVME_ANA_STATE_PERSISTENT_LOSS)
+			return NVME_SC_ANA_PERSISTENT_LOSS | NVME_SC_DNR;
+	}
 	switch (cmd->common.opcode) {
 	case nvme_cmd_read:
 	case nvme_cmd_write:

2.12.3

Hi Hannes,

A block error of BLK_STS_NEXUS is equivalent to a path failure, so blk_path_error() should reflect this.

---
 include/linux/blk_types.h | 1 -
 1 file changed, 1 deletion(-)

index 17b18b91ebac..3c6aeba732f1 100644
@@ -80,7 +80,6 @@ static inline bool blk_path_error(blk_status_t error)
 	case BLK_STS_NOTSUPP:
 	case BLK_STS_NOSPC:
 	case BLK_STS_TARGET:
-	case BLK_STS_NEXUS:
 	case BLK_STS_MEDIUM:
 	case BLK_STS_PROTECTION:
 		return false;

2.12.3

BLK_STS_NEXUS is the replacement of -EBADE, which in the old world order was labelled 'critical nexus"' and is claim to be used for reservation conflicts, in which case we should not rety on another path.

Add ANA support to the nvme host. If ANA is supported the state and the group id are displayed in new sysfs attributes 'ana_state' and 'ana_group'.

---
 drivers/nvme/host/core.c      | 123 +++++++++++++++++++++++++++++++++++++++++-
 drivers/nvme/host/multipath.c |  12 ++++-
 drivers/nvme/host/nvme.h      |   3 ++
 3 files changed, 136 insertions(+), 2 deletions(-)

index 62262fac7a5d..14dff8e96899 100644
@@ -99,6 +99,7 @@ static struct class *nvme_subsys_class;
 
 static void nvme_ns_remove(struct nvme_ns *ns);
 static int nvme_revalidate_disk(struct gendisk *disk);
+static void nvme_get_ana_log(struct nvme_ctrl *ctrl, struct nvme_ns *ns);
 
 int nvme_reset_ctrl(struct nvme_ctrl *ctrl)
 {

@@ -1488,6 +1489,9 @@ static int nvme_revalidate_disk(struct gendisk *disk)
 		goto out;
 	}
 
+	if (ctrl->subsys->cmic & (1 << 3))
+		nvme_get_ana_log(ctrl, ns);
+
 	__nvme_revalidate_disk(disk, id);
 	nvme_report_ns_ids(ctrl, ns->head->ns_id, id, &ids);
 	if (!nvme_ns_ids_equal(&ns->head->ids, &ids)) {

@@ -2276,6 +2280,61 @@ static int nvme_get_effects_log(struct nvme_ctrl *ctrl)
 	return ret;
 }
 
+static void nvme_get_ana_log(struct nvme_ctrl *ctrl, struct nvme_ns *ns)
+{
+	int i, j;
+	struct nvmf_ana_rsp_page_header *ana_log;
+	size_t ana_log_size = 4096;
+
+	ana_log = kzalloc(ana_log_size, GFP_KERNEL);
+	if (!ana_log)
+		return;
+
+	if (nvme_get_log(ctrl, NVME_LOG_ANA, ana_log, ana_log_size))
+		dev_warn(ctrl->device,
+			 "Get ANA log error\n");
+	for (i = 0; i < ana_log->grpid_num; i++) {
+		struct nvmf_ana_group_descriptor *desc =
+			&ana_log->desc[i];
+		for (j = 0; j < desc->nsid_num; j++) {
+			if (desc->nsid[j] == ns->head->ns_id) {
+				ns->ana_state = desc->ana_state;
+				ns->ana_group = desc->groupid;
+			}
+		}
+	}
+	kfree(ana_log);
+}
+
+static void nvme_get_full_ana_log(struct nvme_ctrl *ctrl)
+{
+	int i, j;
+	struct nvme_ns *ns;
+	struct nvmf_ana_rsp_page_header *ana_log;
+	size_t ana_log_size = 4096;
+
+	ana_log = kzalloc(ana_log_size, GFP_KERNEL);
+	if (!ana_log)
+		return;
+
+	if (nvme_get_log(ctrl, NVME_LOG_ANA, ana_log, ana_log_size))
+		dev_warn(ctrl->device,
+			 "Get ANA log error\n");
+	list_for_each_entry(ns, &ctrl->namespaces, list) {
+		for (i = 0; i < ana_log->grpid_num; i++) {
+			struct nvmf_ana_group_descriptor *desc =
+				&ana_log->desc[i];
+			for (j = 0; j < desc->nsid_num; j++) {
+				if (desc->nsid[j] == ns->head->ns_id) {
+					ns->ana_state = desc->ana_state;
+					ns->ana_group = desc->groupid;
+				}
+			}
+		}
+	}
+	kfree(ana_log);
+}
+
 /*
  * Initialize the cached copies of the Identify data and various controller
  * register in our nvme_ctrl structure.  This should be called as soon as

@@ -2631,12 +2690,45 @@ static ssize_t nsid_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(nsid);
 
+static ssize_t ana_state_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct nvme_ns *ns = nvme_get_ns_from_dev(dev);
+
+	switch (ns->ana_state & 0x0f) {
+	case NVME_ANA_STATE_OPTIMIZED:
+		return sprintf(buf, "optimized\n");
+	case NVME_ANA_STATE_NONOPTIMIZED:
+		return sprintf(buf, "non-optimized\n");
+	case NVME_ANA_STATE_INACCESSIBLE:
+		return sprintf(buf, "inaccessible\n");
+	case NVME_ANA_STATE_PERSISTENT_LOSS:
+		return sprintf(buf, "persistent-loss\n");
+	case NVME_ANA_STATE_CHANGE_STATE:
+		return sprintf(buf, "change-state\n");
+	default:
+		return sprintf(buf, "<reserved>\n");
+	}
+}
+static DEVICE_ATTR_RO(ana_state);
+
+static ssize_t ana_group_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct nvme_ns *ns = dev_to_disk(dev)->private_data;
+	return sprintf(buf, "%d\n", ns->ana_group);
+
+}
+static DEVICE_ATTR_RO(ana_group);
+
 static struct attribute *nvme_ns_id_attrs[] = {
 	&dev_attr_wwid.attr,
 	&dev_attr_uuid.attr,
 	&dev_attr_nguid.attr,
 	&dev_attr_eui.attr,
 	&dev_attr_nsid.attr,
+	&dev_attr_ana_state.attr,
+	&dev_attr_ana_group.attr,
 	NULL,
 };
 

@@ -2645,6 +2737,7 @@ static umode_t nvme_ns_id_attrs_are_visible(struct kobject *kobj,
 {
 	struct device *dev = container_of(kobj, struct device, kobj);
 	struct nvme_ns_ids *ids = &dev_to_ns_head(dev)->ids;
+	struct nvme_ns *ns = nvme_get_ns_from_dev(dev);
 
 	if (a == &dev_attr_uuid.attr) {
 		if (uuid_is_null(&ids->uuid) &&

@@ -2659,6 +2752,12 @@ static umode_t nvme_ns_id_attrs_are_visible(struct kobject *kobj,
 		if (!memchr_inv(ids->eui64, 0, sizeof(ids->eui64)))
 			return 0;
 	}
+	if ((a == &dev_attr_ana_state.attr) ||
+	    (a == &dev_attr_ana_group.attr)) {
+		if (!ns->ctrl || !ns->ctrl->subsys ||
+		    !(ns->ctrl->subsys->cmic & (1 << 3)))
+			return 0;
+	}
 	return a->mode;
 }
 

@@ -2984,6 +3083,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid)
 	blk_queue_flag_set(QUEUE_FLAG_NONROT, ns->queue);
 	ns->queue->queuedata = ns;
 	ns->ctrl = ctrl;
+	ns->ana_state = NVME_ANA_STATE_OPTIMIZED;
 
 	kref_init(&ns->kref);
 	ns->lba_shift = 9; /* set to a default value for 512 until disk is validated */

@@ -3001,7 +3101,9 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid)
 	if (nvme_init_ns_head(ns, nsid, id))
 		goto out_free_id;
 	nvme_setup_streams_ns(ctrl, ns);
-	
+
+	if (ctrl->subsys->cmic & (1 << 3))
+		nvme_get_ana_log(ctrl, ns);
 #ifdef CONFIG_NVME_MULTIPATH
 	/*
 	 * If multipathing is enabled we need to always use the subsystem

@@ -3343,6 +3445,19 @@ static void nvme_fw_act_work(struct work_struct *work)
 	nvme_get_fw_slot_info(ctrl);
 }
 
+static void nvme_ana_change_work(struct work_struct *work)
+{
+	struct nvme_ctrl *ctrl = container_of(work,
+				struct nvme_ctrl, ana_change_work);
+
+	if (ctrl->state != NVME_CTRL_LIVE)
+		return;
+
+	down_read(&ctrl->namespaces_rwsem);
+	nvme_get_full_ana_log(ctrl);
+	up_read(&ctrl->namespaces_rwsem);
+}
+
 void nvme_complete_async_event(struct nvme_ctrl *ctrl, __le16 status,
 		union nvme_result *res)
 {

@@ -3370,6 +3485,10 @@ void nvme_complete_async_event(struct nvme_ctrl *ctrl, __le16 status,
 	case NVME_AER_NOTICE_FW_ACT_STARTING:
 		queue_work(nvme_wq, &ctrl->fw_act_work);
 		break;
+	case NVME_AER_NOTICE_ANA_CHANGE:
+		dev_info(ctrl->device, "ANA state change\n");
+		queue_work(nvme_wq, &ctrl->ana_change_work);
+		break;
 	default:
 		dev_warn(ctrl->device, "async event result %08x\n", result);
 	}

@@ -3383,6 +3502,7 @@ void nvme_stop_ctrl(struct nvme_ctrl *ctrl)
 	flush_work(&ctrl->async_event_work);
 	flush_work(&ctrl->scan_work);
 	cancel_work_sync(&ctrl->fw_act_work);
+	cancel_work_sync(&ctrl->ana_change_work);
 	if (ctrl->ops->stop_ctrl)
 		ctrl->ops->stop_ctrl(ctrl);
 }

@@ -3449,6 +3569,7 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
 	INIT_WORK(&ctrl->scan_work, nvme_scan_work);
 	INIT_WORK(&ctrl->async_event_work, nvme_async_event_work);
 	INIT_WORK(&ctrl->fw_act_work, nvme_fw_act_work);
+	INIT_WORK(&ctrl->ana_change_work, nvme_ana_change_work);
 	INIT_WORK(&ctrl->delete_work, nvme_delete_ctrl_work);
 
 	ret = ida_simple_get(&nvme_instance_ida, 0, 0, GFP_KERNEL);

index 956e0b8e9c4d..077b56f1739a 100644
@@ -56,8 +56,18 @@ static struct nvme_ns *__nvme_find_path(struct nvme_ns_head *head)
 {
 	struct nvme_ns *ns;
 
+	/* First round: select from all ANA optimized paths */
 	list_for_each_entry_rcu(ns, &head->list, siblings) {
-		if (ns->ctrl->state == NVME_CTRL_LIVE) {
+		if (ns->ctrl->state == NVME_CTRL_LIVE &&
+		    ns->ana_state == NVME_ANA_STATE_OPTIMIZED) {
+			rcu_assign_pointer(head->current_path, ns);
+			return ns;
+		}
+	}
+	/* Second round: select from all ANA non-optimized paths */
+	list_for_each_entry_rcu(ns, &head->list, siblings) {
+		if (ns->ctrl->state == NVME_CTRL_LIVE &&
+		    ns->ana_state == NVME_ANA_STATE_NONOPTIMIZED) {
 			rcu_assign_pointer(head->current_path, ns);
 			return ns;
 		}

index 061fecfd44f5..75182a5cc166 100644
@@ -187,6 +187,7 @@ struct nvme_ctrl {
 	struct delayed_work ka_work;
 	struct nvme_command ka_cmd;
 	struct work_struct fw_act_work;
+	struct work_struct ana_change_work;
 
 	/* Power saving configuration */
 	u64 ps_max_latency_us;

@@ -289,6 +290,8 @@ struct nvme_ns {
 	u32 sws;
 	bool ext;
 	u8 pi_type;
+	u8 ana_state;
+	u32 ana_group;
 	unsigned long flags;
 #define NVME_NS_REMOVING 0
 #define NVME_NS_DEAD     1

2.12.3

Good idea. Will be doing it.

Yes, should be doing so.

Actually, it should be sufficient to just unset the current path; the algorithm will find the next best path then.
But yes, true, we should be doing it.
(I had the Optimized/Inaccessible scenario in mind here; there the new path will be selected automatically as we'd be getting an error from the Inaccessible path. But for Optimized/non-optimized the new path won't be chosen automatically as the original path remains valid.)

Thanks for the hints.

Cheers,

Hannes

...

...

Hi,

I work for NetApp and leading efforts to add ANA support for our
NVMe/FC target. So me and my team are excited with this patch.So thanks for working on this!
We had some spec comments, but John M, Fred K and others have already raised it. We have few more comments looking at the overall solution. Please consider addressing the following:

1) Ana log size
+ size_t ana_log_size = 4096;

This is hard coded. Don?t you expect to have a larger log page? Looks like the number of ANA groups will be restricted to around 113 assuming one namespace identifier per ana group.
Can we have a larger ana_log_size? Maybe it should be MDTS?

2) Sanity check after reading ana log page
There should be a sanity check to make sure the log page is read completely. A check to see if all the ana groups are read and all the num nsids(for the last group) are read otherwise states on few namespace paths will be stale.

3) ANATT not used.
Didn't see anatt honored. Is there a plan to do that in future? Looks like you retry forever if there is any path related error. Let's say there are two paths ANA Optimized and other path ANA Inaccessible and then the ANA optimized path is transitioning to ANA Inaccessible.
If my understanding is correct, this is what happens:
* Linux gets back ANA Transition.
* The current controller is reset.
* IO will be re-queued or failover to other path regardless of the other path being still Inaccessible (or even Persistent loss).
* nvme_find_path will return null so the IO is re-queued again.
* This state continues till the target completes an AER(?) Can it go forever?

4) Controller reset on path related errors
Looks like any path related errors will result in controller reset. This looks bad as path change for a single namespace will result in controller reset. Didn?t see any special handling for path related errors. Should that happen as controller reset is a bigger hammer for these errors?

5) Path related error handling
Again on the same lines as above. All path related errors fall in one category as of now (BLK_STS_IOERR). Handling of persistent loss or ANA inaccessible or ANA transition should be according to the spec.

6) Detecting host ANA support.
Didn?t see good way for target to know if host is ANA complaint or not in the spec itself. One way we can negotiate ANA support is through set features(0Bh). That helps targets to know whether its ok to return ANA path related errors or complete AERs with ANA change notice. Is there a plan to implement set features (0Bh) to negotiate asynchronous event configuration?

Regards,
~Sriram

Yeah, 'tis a bit of a cheeky implementation.
I'll see to make it configurable.

Well; this basically is the same issue we had with ALUA on the SCSI side. What _are_ the correct actions if the ANA log page couldn't be read properly?
Sure, one could to a retry, but even that will be exhausted after some iterations. What then?
Fail the controller?
-> probably not the best of options; after all, ANA support is optional. Set the paths to failed?
-> Again, probably not the best of ideas as the path itself might be operational.
So for now it's probably best to just leave the states as they are and wait for the situation to resolve itself.

True. What we _could_ to is to move the paths to a tentative 'non-optimized' state, and see if the I/O succeeds. If it fails we should be getting an appropriate status code, and should be able to set the path status accordingly.
We will not get a notification for the optimized path, but then something went wrong on the array, so we don't know the correct status.

Maybe this would be the best solution:

1.) If the ANA log fails set all paths to an invalid ANA state (eg 0), thus allowing I/O to continue. If the I/O comes back with an ANA status code like 'inaccessible' or 'persistent loss' (or even 'transitioning', use that to update the status. Then schedule an ANA log read at a later time (eg 1 seconds later or somesuch).
2.) If the paths are in 'transitioning' retry I/O until ANATT. If we get an AEN signalling a state change or ANATT expired submit an ANA log page read to update the ANA state. If that fails, continue at 1.

The entire idea here is that the controller does _not_ need to be reset; yes, we've returned with an status, but this actually is expected, and shouldn't cause the controller to be reset.
Obviously we need to update the ANA state on all paths before attempting a failover; otherwise we're running against a not-yet-updated path in eg inaccessible and would not be able to issue I/O.

For 'normal' path state change errors like 'ANA inaccessible' we shouldn't reset the controller; but we'll need to run some tests here how the system actually behaves.

Yeah, that's actually wrong. I've got a patch reclassifying ANA errors as path errors, which hopefully resolves this.

Well. Yes, obviously we should be registering for ANA AENs; might be that the current code doesn't do it as the linux target implementation is somewhat lazy in that respect.

Cheers,

Hannes

After revisiting the ANA Base protocol this indeed is a good question. The spec explicitly has:

If the RGO bit is cleared to ?0? in Command Dword 10, then the LPOL field in Command Dword 12 and the LPOU field in Command Dword 13 of the Get Log Page command should be cleared to zero.
Note: Clearing the RGO bit to ?0? and setting either the LPOL field or the LPOU field to a non-zero value may return a log page that the host is unable to parse.

Which I would interpret that we _have_ to use a buffer of the correct size to retrieve the entire information.

Curiously, we should be able to use the LPOL and LPOU setting when the RGO bit is set, ie when we're just enumerating ANA groups.

A quick calculation there reveals that the max size for the ANA log page with the RGO bit set is about 2M (65535 descriptors * 32 bytes per descriptor + 16 byte header), which probably is a bit unwieldy to use directly.

So this makes it a bit awkward to handle; we first have to get the ANA log with the RGO bit set and a default size, then (possibly) re-issue the ANA log with the RGO bit set and the correct size, then calculate the actual size of the ANA log, and _then_ retrieve the full ANA log.

My, you don't really believe in making things easy for the programmer ...

Fred: if you ever plan for a revisited ANA spec, _please_ change this. Make the currently reserved field in the ANA log page header a max nsid count (ie store the maximum number of NSIDs per ANA group here). With that we have a max length of the ANA log page, and would have to retrieve the log page only twice, not three times.

Cheers,

Hannes

My comments inline with tag "[Sriram]".

Regards,
~Sriram

-----Original Message-----
From: Hannes Reinecke <[email protected]>
Sent: Friday, May 11, 2018 1:20 PM
To: Sriram Popuri <sgpopuri at gmail.com>
Cc: emilne at redhat.com; linux-nvme at lists.infradead.org; Popuri, Sriram <Sriram.Popuri at netapp.com>; Knight, Frederick <Frederick.Knight at netapp.com>; Meneghini, John <John.Meneghini at netapp.com>
Subject: Re: [PATCH 5/5] nvme: ANA base support

After revisiting the ANA Base protocol this indeed is a good question. The spec explicitly has:

If the RGO bit is cleared to ?0? in Command Dword 10, then the LPOL field in Command Dword 12 and the LPOU field in Command Dword 13 of the Get Log Page command should be cleared to zero.
Note: Clearing the RGO bit to ?0? and setting either the LPOL field or the LPOU field to a non-zero value may return a log page that the host is unable to parse.

Which I would interpret that we _have_ to use a buffer of the correct size to retrieve the entire information.

[Sriram] That?s precisely the reason I suggested MDTS. Target would only give you back the size in MDTS and you can't read beyond MDTS since the spec doesn't allow you to read at some offset.

Curiously, we should be able to use the LPOL and LPOU setting when the RGO bit is set, ie when we're just enumerating ANA groups.

[Sriram] I second that and I did suggest this to Fred sometime back.

A quick calculation there reveals that the max size for the ANA log page with the RGO bit set is about 2M (65535 descriptors * 32 bytes per descriptor + 16 byte header), which probably is a bit unwieldy to use directly.

So this makes it a bit awkward to handle; we first have to get the ANA log with the RGO bit set and a default size, then (possibly) re-issue the ANA log with the RGO bit set and the correct size, then calculate the actual size of the ANA log, and _then_ retrieve the full ANA log.

[Sriram] Unfortunately you can't get the correct size with the method above because the spec says if RGO=1, num_nsid field is 0. So you can't figure out the full size of log page.

My, you don't really believe in making things easy for the programmer ...

Fred: if you ever plan for a revisited ANA spec, _please_ change this.
Make the currently reserved field in the ANA log page header a max nsid count (ie store the maximum number of NSIDs per ANA group here). With that we have a max length of the ANA log page, and would have to retrieve the log page only twice, not three times.

Cheers,

Hannes

Small corrections inline.

Regards,
~Sriram
-----Original Message-----
From: Linux-nvme <[email protected]> On Behalf Of Popuri, Sriram
Sent: Friday, May 11, 2018 1:52 PM
To: Hannes Reinecke <hare at suse.de>; Sriram Popuri <sgpopuri at gmail.com>
Cc: Knight, Frederick <Frederick.Knight at netapp.com>; linux-nvme at lists.infradead.org; emilne at redhat.com; Meneghini, John <John.Meneghini at netapp.com>
Subject: RE: [PATCH 5/5] nvme: ANA base support

My comments inline with tag "[Sriram]".

Regards,
~Sriram

-----Original Message-----
From: Hannes Reinecke <[email protected]>
Sent: Friday, May 11, 2018 1:20 PM
To: Sriram Popuri <sgpopuri at gmail.com>
Cc: emilne at redhat.com; linux-nvme at lists.infradead.org; Popuri, Sriram <Sriram.Popuri at netapp.com>; Knight, Frederick <Frederick.Knight at netapp.com>; Meneghini, John <John.Meneghini at netapp.com>
Subject: Re: [PATCH 5/5] nvme: ANA base support

After revisiting the ANA Base protocol this indeed is a good question. The spec explicitly has:

If the RGO bit is cleared to ?0? in Command Dword 10, then the LPOL field in Command Dword 12 and the LPOU field in Command Dword 13 of the Get Log Page command should be cleared to zero.
Note: Clearing the RGO bit to ?0? and setting either the LPOL field or the LPOU field to a non-zero value may return a log page that the host is unable to parse.

Which I would interpret that we _have_ to use a buffer of the correct size to retrieve the entire information.

[Sriram] That?s precisely the reason I suggested MDTS. Target would only give you back at max the size in MDTS and you can't read beyond MDTS if the log size is larger since the spec doesn't allow you to read at some offset. So the targets have to restrict the config to be able to adjust the log page within MDTS (bad, but that?s how it is today).

Curiously, we should be able to use the LPOL and LPOU setting when the RGO bit is set, ie when we're just enumerating ANA groups.

A quick calculation there reveals that the max size for the ANA log page with the RGO bit set is about 2M (65535 descriptors * 32 bytes per descriptor + 16 byte header), which probably is a bit unwieldy to use directly.

So this makes it a bit awkward to handle; we first have to get the ANA log with the RGO bit set and a default size, then (possibly) re-issue the ANA log with the RGO bit set and the correct size, then calculate the actual size of the ANA log, and _then_ retrieve the full ANA log.

[Sriram] Unfortunately you can't get the correct size with the method above because the spec says if RGO=1, num_nsid field is 0. So you can't figure out the full size of log page.

My, you don't really believe in making things easy for the programmer ...

Fred: if you ever plan for a revisited ANA spec, _please_ change this.
Make the currently reserved field in the ANA log page header a max nsid count (ie store the maximum number of NSIDs per ANA group here). With that we have a max length of the ANA log page, and would have to retrieve the log page only twice, not three times.

[Sriram] I second that and I did suggest this to Fred sometime back.

Cheers,

Hannes
_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

FK> INLINE

-----Original Message-----
From: Popuri, Sriram
Sent: Friday, May 11, 2018 4:36 AM
To: Popuri, Sriram <Sriram.Popuri at netapp.com>; Hannes Reinecke <hare at suse.de>; Sriram Popuri <sgpopuri at gmail.com>
Cc: Knight, Frederick <Frederick.Knight at netapp.com>; linux-nvme at lists.infradead.org; emilne at redhat.com; Meneghini, John <John.Meneghini at netapp.com>
Subject: RE: [PATCH 5/5] nvme: ANA base support

Small corrections inline.

Regards,
~Sriram
-----Original Message-----
From: Linux-nvme <[email protected]> On Behalf Of Popuri, Sriram
Sent: Friday, May 11, 2018 1:52 PM
To: Hannes Reinecke <hare at suse.de>; Sriram Popuri <sgpopuri at gmail.com>
Cc: Knight, Frederick <Frederick.Knight at netapp.com>; linux-nvme at lists.infradead.org; emilne at redhat.com; Meneghini, John <John.Meneghini at netapp.com>
Subject: RE: [PATCH 5/5] nvme: ANA base support

My comments inline with tag "[Sriram]".

Regards,
~Sriram

-----Original Message-----
From: Hannes Reinecke <[email protected]>
Sent: Friday, May 11, 2018 1:20 PM
To: Sriram Popuri <sgpopuri at gmail.com>
Cc: emilne at redhat.com; linux-nvme at lists.infradead.org; Popuri, Sriram <Sriram.Popuri at netapp.com>; Knight, Frederick <Frederick.Knight at netapp.com>; Meneghini, John <John.Meneghini at netapp.com>
Subject: Re: [PATCH 5/5] nvme: ANA base support

[] FK> All spec comments that were raised were addressed. There were no outstanding comments at the time the spec was approved (or now for that matter).

After revisiting the ANA Base protocol this indeed is a good question. The spec explicitly has:

If the RGO bit is cleared to ?0? in Command Dword 10, then the LPOL field in Command Dword 12 and the LPOU field in Command Dword 13 of the Get Log Page command should be cleared to zero.
Note: Clearing the RGO bit to ?0? and setting either the LPOL field or the LPOU field to a non-zero value may return a log page that the host is unable to parse.

Which I would interpret that we _have_ to use a buffer of the correct size to retrieve the entire information.

[] FK> NO, you don't "have_to". LPOL and LPOU are fine to use when RGO=1 (if you are getting a fixed definition data structure, then you a priory know where things are, and therefore, what offset to use to get to where you want to get). If however, RGO=0, then the definition of the data structure is variable (the contents of offset X+n is dependent on the contents of the n bytes that come before X). If RGO=0, the number of NSID values in each group can change (at any time), and therefore, change where things are - and therefore change the offset value you need to use - to get where you think you should be getting. BUT, if you want to risk it, or if you've invented a unique new algorithm to find stuff in that variable structure - go for it - it is just a "should".

[Sriram] That?s precisely the reason I suggested MDTS. Target would only give you back at max the size in MDTS and you can't read beyond MDTS if the log size is larger since the spec doesn't allow you to read at some offset. So the targets have to restrict the config to be able to adjust the log page within MDTS (bad, but that?s how it is today).

[] FK> Wrong. With RGO=1, it is fine to use the offset fields to work your way through a long log page.

Curiously, we should be able to use the LPOL and LPOU setting when the RGO bit is set, ie when we're just enumerating ANA groups.

[] FK> That is the way it was designed.

A quick calculation there reveals that the max size for the ANA log page with the RGO bit set is about 2M (65535 descriptors * 32 bytes per descriptor + 16 byte header), which probably is a bit unwieldy to use directly.

[] FK> You missed the MNAN field in that calculation. The buffer only needs to be large enough to hold the maximum number of NSID values, not up to the maximum NSID value - you don't need to reserve space for a whole bunch of values that are never used.

So this makes it a bit awkward to handle; we first have to get the ANA log with the RGO bit set and a default size, then (possibly) re-issue the ANA log with the RGO bit set and the correct size, then calculate the actual size of the ANA log, and _then_ retrieve the full ANA log.

[] FK> Again, you missed the MNAN field - just use that, and do it once.

[Sriram] Unfortunately you can't get the correct size with the method above because the spec says if RGO=1, num_nsid field is 0. So you can't figure out the full size of log page.

My, you don't really believe in making things easy for the programmer ...

[] FK> This was agreed to by all the people on the call (which included some host programmers).

Fred: if you ever plan for a revisited ANA spec, _please_ change this.
Make the currently reserved field in the ANA log page header a max nsid count (ie store the maximum number of NSIDs per ANA group here). With that we have a max length of the ANA log page, and would have to retrieve the log page only twice, not three times.

[] FK> You already have that with the MNAN field.

[Sriram] I second that and I did suggest this to Fred sometime back.

[] FK> Which is why we added the MNAN field.

Cheers,

Hannes
_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Passing a ns to the function reading the ANA log doesn't make any sense. The ANA log page is global to the controller, so we should only read it when rescanning the controller, or when the AER triggers.

Yes, already fixed up for the next round.

Okay, will do.

True; the next round will have an error handling here.

I'll be moving the function to get the ANA log page to a workqueue, so we'll get an implicit serialisation from there.
And obviously we need to get the namespace mutex when traversing the list of namspaces.

Ok.

Okay; no problem with that.

Hmm. Not sure if it that really simplifies things, but I'll have a look.

Yeah, that's particularly mean. I've added some code the nvmet, though, so we should be able to test this one.

Cheers,

Hannes