Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 7 Mar 2017 21:47:55 +0000 (UTC)
From:      Warner Losh <imp@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r314884 - head/sys/dev/nvme
Message-ID:  <201703072147.v27Lltmi074554@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: imp
Date: Tue Mar  7 21:47:54 2017
New Revision: 314884
URL: https://svnweb.freebsd.org/changeset/base/314884

Log:
  Make multi-namespace nvme drives more robust.
  
  Fix assumptions about name spaces in NVME driver. First, it assumes
  cdata.nn is the number of configured devices. However, it is the
  number of supported name spaces. Second, it assumes that there will
  never be more than 16 name spaces supported, but a certain drive I'm
  testing reports 1024. It assumes that name spaces are a tightly packed
  namespace, but the standard seems to indicate otherwise. Finally, it
  assumes that an error would be generated when quearying an
  unconfigured namespace. Instead, it succeeds but the identify data is
  all zeros.
  
  Fix these by limiting the number of name spaces we probe to 16. Remove
  aborting when we find one in error. When the size of the name space is
  zero, ignore it.
  
  This is admittedly a bandaide. The long term fix will be to
  participate in the enumeration and name space change protocols
  definfed in the NVNe standard.
  
  Sponsored by: Netflix

Modified:
  head/sys/dev/nvme/nvme.c
  head/sys/dev/nvme/nvme_ctrlr.c
  head/sys/dev/nvme/nvme_ns.c

Modified: head/sys/dev/nvme/nvme.c
==============================================================================
--- head/sys/dev/nvme/nvme.c	Tue Mar  7 21:44:05 2017	(r314883)
+++ head/sys/dev/nvme/nvme.c	Tue Mar  7 21:47:54 2017	(r314884)
@@ -327,8 +327,10 @@ nvme_notify(struct nvme_consumer *cons,
 		 */
 		return;
 	}
-	for (ns_idx = 0; ns_idx < ctrlr->cdata.nn; ns_idx++) {
+	for (ns_idx = 0; ns_idx < min(ctrlr->cdata.nn, NVME_MAX_NAMESPACES); ns_idx++) {
 		ns = &ctrlr->ns[ns_idx];
+		if (ns->data.nsze == 0)
+			continue;
 		if (cons->ns_fn != NULL)
 			ns->cons_cookie[cons->id] =
 			    (*cons->ns_fn)(ns, ctrlr_cookie);

Modified: head/sys/dev/nvme/nvme_ctrlr.c
==============================================================================
--- head/sys/dev/nvme/nvme_ctrlr.c	Tue Mar  7 21:44:05 2017	(r314883)
+++ head/sys/dev/nvme/nvme_ctrlr.c	Tue Mar  7 21:47:54 2017	(r314884)
@@ -458,13 +458,11 @@ static int
 nvme_ctrlr_construct_namespaces(struct nvme_controller *ctrlr)
 {
 	struct nvme_namespace	*ns;
-	int			i, status;
+	int			i;
 
-	for (i = 0; i < ctrlr->cdata.nn; i++) {
+	for (i = 0; i < min(ctrlr->cdata.nn, NVME_MAX_NAMESPACES); i++) {
 		ns = &ctrlr->ns[i];
-		status = nvme_ns_construct(ns, i+1, ctrlr);
-		if (status != 0)
-			return (status);
+		nvme_ns_construct(ns, i+1, ctrlr);
 	}
 
 	return (0);

Modified: head/sys/dev/nvme/nvme_ns.c
==============================================================================
--- head/sys/dev/nvme/nvme_ns.c	Tue Mar  7 21:44:05 2017	(r314883)
+++ head/sys/dev/nvme/nvme_ns.c	Tue Mar  7 21:47:54 2017	(r314884)
@@ -512,13 +512,22 @@ nvme_ns_construct(struct nvme_namespace 
 	}
 
 	/*
+	 * If the size of is zero, chances are this isn't a valid
+	 * namespace (eg one that's not been configured yet). The
+	 * standard says the entire id will be zeros, so this is a
+	 * cheap way to test for that.
+	 */
+	if (ns->data.nsze == 0)
+		return (ENXIO);
+
+	/*
 	 * Note: format is a 0-based value, so > is appropriate here,
 	 *  not >=.
 	 */
 	if (ns->data.flbas.format > ns->data.nlbaf) {
 		printf("lba format %d exceeds number supported (%d)\n",
 		    ns->data.flbas.format, ns->data.nlbaf+1);
-		return (1);
+		return (ENXIO);
 	}
 
 	if (ctrlr->cdata.oncs.dsm)

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201703072147.v27Lltmi074554>