diff mbox

[v2,7/9] libnvdimm, pmem: prepare for handling badblocks via nvdimm_read_bytes()

Message ID 20160106223128.2736.40342.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dan Williams Jan. 6, 2016, 10:31 p.m. UTC 
When performing reads through the nvdimm_read_bytes() interface we still
want to consult the badblocks list, but we do not have access to a
gendisk in that path.  Convert the badblocks list to be a generic
attribute of a namespace.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/nvdimm/core.c |   44 +++++++++++++++++++++++++++-----------------
 drivers/nvdimm/nd.h   |    4 ++--
 drivers/nvdimm/pmem.c |   13 +++++++++----
 3 files changed, 38 insertions(+), 23 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Vishal Verma Jan. 9, 2016, 12:46 a.m. UTC | #1 
On Wed, 2016-01-06 at 14:31 -0800, Dan Williams wrote:
> When performing reads through the nvdimm_read_bytes() interface we
> still
> want to consult the badblocks list, but we do not have access to a
> gendisk in that path.  Convert the badblocks list to be a generic
> attribute of a namespace.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/nvdimm/core.c |   44 +++++++++++++++++++++++++++----------
> -------
>  drivers/nvdimm/nd.h   |    4 ++--
>  drivers/nvdimm/pmem.c |   13 +++++++++----
>  3 files changed, 38 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
> index 5541612bb449..63b7386b7299 100644
> --- a/drivers/nvdimm/core.c
> +++ b/drivers/nvdimm/core.c
> @@ -411,32 +411,41 @@ static int __add_badblock_range(struct device
> *dev, struct badblocks **bb,
>  }
>  
>  /**
> - * nvdimm_namespace_add_poison() - Convert a list of poison ranges
> to badblocks
> - * @disk:	the gendisk associated with the namespace where
> badblocks
> - *		will be stored
> + * nvdimm_namespace_badblocks() - Convert a list of poison ranges to
> badblocks
> + * @ndns:	namespace hosting potential badblocks
>   * @offset:	offset at the start of the namespace before
> 'sector 0'
> - * @ndns:	the namespace containing poison ranges
>   *
> - * The poison list generated during NFIT initialization may contain
> multiple,
> - * possibly overlapping ranges in the SPA (System Physical Address)
> space.
> - * Compare each of these ranges to the namespace currently being
> initialized,
> - * and add badblocks to the gendisk for all matching sub-ranges
> + * The poison list generated during NFIT initialization may contain
> + * multiple, possibly overlapping ranges in the SPA (System Physical
> + * Address) space.  Compare each of these ranges to the namespace
> + * currently being initialized, and add badblocks for all matching
> + * sub-ranges
>   *
>   * Return:
> - * 0 - Success
> + * valid badblocks instance on success
> + * ERR_PTR(-ENOENT) on no badblocks present
> + * ERR_PTR(-<error>) on failure
>   */
> -int nvdimm_namespace_add_poison(struct gendisk *disk,
> resource_size_t offset,
> -		struct nd_namespace_common *ndns)
> +struct badblocks *nvdimm_namespace_badblocks(struct
> nd_namespace_common *ndns,
> +		resource_size_t offset)
>  {
> -	struct nd_namespace_io *nsio = to_nd_namespace_io(&ndns-
> >dev);
>  	struct nd_region *nd_region = to_nd_region(ndns-
> >dev.parent);
>  	struct nvdimm_bus *nvdimm_bus;
>  	struct list_head *poison_list;
>  	u64 ns_start, ns_end, ns_size;
> +	struct nd_namespace_io *nsio;
>  	struct badblocks *bb = NULL;
>  	struct nd_poison *pl;
>  	int rc;
>  
> +	/*
> +	 * For now, the addresses retrieved from an nvdimm bus are
> +	 * communicated in terms of system physical address not a
> dimm
> +	 * address (dpa).  This implies pmem only.
> +	 */
> +	if (!is_nd_pmem(&nd_region->dev))
> +		return ERR_PTR(-ENOENT);
> +	nsio = to_nd_namespace_io(&ndns->dev);
>  	ns_size = nvdimm_namespace_capacity(ndns) - offset;
>  	ns_start = nsio->res.start + offset;
>  	ns_end = nsio->res.end;
> @@ -444,7 +453,7 @@ int nvdimm_namespace_add_poison(struct gendisk
> *disk, resource_size_t offset,
>  	nvdimm_bus = to_nvdimm_bus(nd_region->dev.parent);
>  	poison_list = &nvdimm_bus->poison_list;
>  	if (list_empty(poison_list))
> -		return 0;
> +		return ERR_PTR(-ENOENT);
>  
>  	list_for_each_entry(pl, poison_list, list) {
>  		u64 pl_end = pl->start + pl->length - 1;
> @@ -467,7 +476,7 @@ int nvdimm_namespace_add_poison(struct gendisk
> *disk, resource_size_t offset,
>  			rc = __add_badblock_range(&ndns->dev, &bb,
>  					start - ns_start, len);
>  			if (rc)
> -				return rc;
> +				return ERR_PTR(rc);
>  			dev_info(&nvdimm_bus->dev,
>  				"Found a poison range (0x%llx,
> 0x%llx)\n",
>  				start, len);
> @@ -484,16 +493,17 @@ int nvdimm_namespace_add_poison(struct gendisk
> *disk, resource_size_t offset,
>  
>  			rc = __add_badblock_range(&ndns->dev, &bb,
> 0, len);
>  			if (rc)
> -				return rc;
> +				return ERR_PTR(rc);
>  			dev_info(&nvdimm_bus->dev,
>  				"Found a poison range (0x%llx,
> 0x%llx)\n",
>  				pl->start, len);
>  		}
>  	}
>  
> -	return 0;
> +	WARN_ON(!bb);

Just hit this in testing..
I think it is OK to reach this point without having a valid bb.
The poison_list has a region-wide scope, and it is possible that there
are poison locations in the region outside of the namespace currently
being set up. In this case, this namespace's bb should still be null.

> +	return bb;
>  }
> -EXPORT_SYMBOL_GPL(nvdimm_namespace_add_poison);
> +EXPORT_SYMBOL_GPL(nvdimm_namespace_badblocks);
>  
>  static int __add_poison(struct nvdimm_bus *nvdimm_bus, u64 addr, u64
> length)
>  {
> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
> index 198933da83e5..65f1031f2bf9 100644
> --- a/drivers/nvdimm/nd.h
> +++ b/drivers/nvdimm/nd.h
> @@ -268,8 +268,8 @@ int nvdimm_namespace_attach_btt(struct
> nd_namespace_common *ndns);
>  int nvdimm_namespace_detach_btt(struct nd_namespace_common *ndns);
>  const char *nvdimm_namespace_disk_name(struct nd_namespace_common
> *ndns,
>  		char *name);
> -int nvdimm_namespace_add_poison(struct gendisk *disk,
> resource_size_t offset,
> -		struct nd_namespace_common *ndns);
> +struct badblocks *nvdimm_namespace_badblocks(struct
> nd_namespace_common *ndns,
> +		resource_size_t offset);
>  int nd_blk_region_init(struct nd_region *nd_region);
>  void __nd_iostat_start(struct bio *bio, unsigned long *start);
>  static inline bool nd_iostat_start(struct bio *bio, unsigned long
> *start)
> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> index bbe81b7fe031..eb2b1dc14335 100644
> --- a/drivers/nvdimm/pmem.c
> +++ b/drivers/nvdimm/pmem.c
> @@ -192,8 +192,8 @@ static int pmem_attach_disk(struct device *dev,
>  		struct nd_namespace_common *ndns, struct pmem_device
> *pmem)
>  {
>  	int nid = dev_to_node(dev);
> +	struct badblocks *bb;
>  	struct gendisk *disk;
> -	int ret;
>  
>  	pmem->pmem_queue = blk_alloc_queue_node(GFP_KERNEL, nid);
>  	if (!pmem->pmem_queue)
> @@ -222,10 +222,15 @@ static int pmem_attach_disk(struct device *dev,
>  	set_capacity(disk, (pmem->size - pmem->data_offset) / 512);
>  	pmem->pmem_disk = disk;
>  
> -	ret = nvdimm_namespace_add_poison(disk, pmem->data_offset,
> ndns);
> -	if (ret)
> -		return ret;
> +	bb = nvdimm_namespace_badblocks(ndns, pmem->data_offset);
> +	if (IS_ERR(bb)) {
> +		if (PTR_ERR(bb) == -ENOENT)
> +			bb = NULL;
> +		else
> +			return PTR_ERR(bb);
> +	}
>  
> +	disk->bb = bb;
>  	add_disk(disk);
>  	revalidate_disk(disk);
>  
> 
> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
index 5541612bb449..63b7386b7299 100644
--- a/drivers/nvdimm/core.c
+++ b/drivers/nvdimm/core.c
@@ -411,32 +411,41 @@  static int __add_badblock_range(struct device *dev, struct badblocks **bb,
 }
 
 /**
- * nvdimm_namespace_add_poison() - Convert a list of poison ranges to badblocks
- * @disk:	the gendisk associated with the namespace where badblocks
- *		will be stored
+ * nvdimm_namespace_badblocks() - Convert a list of poison ranges to badblocks
+ * @ndns:	namespace hosting potential badblocks
  * @offset:	offset at the start of the namespace before 'sector 0'
- * @ndns:	the namespace containing poison ranges
  *
- * The poison list generated during NFIT initialization may contain multiple,
- * possibly overlapping ranges in the SPA (System Physical Address) space.
- * Compare each of these ranges to the namespace currently being initialized,
- * and add badblocks to the gendisk for all matching sub-ranges
+ * The poison list generated during NFIT initialization may contain
+ * multiple, possibly overlapping ranges in the SPA (System Physical
+ * Address) space.  Compare each of these ranges to the namespace
+ * currently being initialized, and add badblocks for all matching
+ * sub-ranges
  *
  * Return:
- * 0 - Success
+ * valid badblocks instance on success
+ * ERR_PTR(-ENOENT) on no badblocks present
+ * ERR_PTR(-<error>) on failure
  */
-int nvdimm_namespace_add_poison(struct gendisk *disk, resource_size_t offset,
-		struct nd_namespace_common *ndns)
+struct badblocks *nvdimm_namespace_badblocks(struct nd_namespace_common *ndns,
+		resource_size_t offset)
 {
-	struct nd_namespace_io *nsio = to_nd_namespace_io(&ndns->dev);
 	struct nd_region *nd_region = to_nd_region(ndns->dev.parent);
 	struct nvdimm_bus *nvdimm_bus;
 	struct list_head *poison_list;
 	u64 ns_start, ns_end, ns_size;
+	struct nd_namespace_io *nsio;
 	struct badblocks *bb = NULL;
 	struct nd_poison *pl;
 	int rc;
 
+	/*
+	 * For now, the addresses retrieved from an nvdimm bus are
+	 * communicated in terms of system physical address not a dimm
+	 * address (dpa).  This implies pmem only.
+	 */
+	if (!is_nd_pmem(&nd_region->dev))
+		return ERR_PTR(-ENOENT);
+	nsio = to_nd_namespace_io(&ndns->dev);
 	ns_size = nvdimm_namespace_capacity(ndns) - offset;
 	ns_start = nsio->res.start + offset;
 	ns_end = nsio->res.end;
@@ -444,7 +453,7 @@  int nvdimm_namespace_add_poison(struct gendisk *disk, resource_size_t offset,
 	nvdimm_bus = to_nvdimm_bus(nd_region->dev.parent);
 	poison_list = &nvdimm_bus->poison_list;
 	if (list_empty(poison_list))
-		return 0;
+		return ERR_PTR(-ENOENT);
 
 	list_for_each_entry(pl, poison_list, list) {
 		u64 pl_end = pl->start + pl->length - 1;
@@ -467,7 +476,7 @@  int nvdimm_namespace_add_poison(struct gendisk *disk, resource_size_t offset,
 			rc = __add_badblock_range(&ndns->dev, &bb,
 					start - ns_start, len);
 			if (rc)
-				return rc;
+				return ERR_PTR(rc);
 			dev_info(&nvdimm_bus->dev,
 				"Found a poison range (0x%llx, 0x%llx)\n",
 				start, len);
@@ -484,16 +493,17 @@  int nvdimm_namespace_add_poison(struct gendisk *disk, resource_size_t offset,
 
 			rc = __add_badblock_range(&ndns->dev, &bb, 0, len);
 			if (rc)
-				return rc;
+				return ERR_PTR(rc);
 			dev_info(&nvdimm_bus->dev,
 				"Found a poison range (0x%llx, 0x%llx)\n",
 				pl->start, len);
 		}
 	}
 
-	return 0;
+	WARN_ON(!bb);
+	return bb;
 }
-EXPORT_SYMBOL_GPL(nvdimm_namespace_add_poison);
+EXPORT_SYMBOL_GPL(nvdimm_namespace_badblocks);
 
 static int __add_poison(struct nvdimm_bus *nvdimm_bus, u64 addr, u64 length)
 {
diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index 198933da83e5..65f1031f2bf9 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -268,8 +268,8 @@  int nvdimm_namespace_attach_btt(struct nd_namespace_common *ndns);
 int nvdimm_namespace_detach_btt(struct nd_namespace_common *ndns);
 const char *nvdimm_namespace_disk_name(struct nd_namespace_common *ndns,
 		char *name);
-int nvdimm_namespace_add_poison(struct gendisk *disk, resource_size_t offset,
-		struct nd_namespace_common *ndns);
+struct badblocks *nvdimm_namespace_badblocks(struct nd_namespace_common *ndns,
+		resource_size_t offset);
 int nd_blk_region_init(struct nd_region *nd_region);
 void __nd_iostat_start(struct bio *bio, unsigned long *start);
 static inline bool nd_iostat_start(struct bio *bio, unsigned long *start)
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index bbe81b7fe031..eb2b1dc14335 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -192,8 +192,8 @@  static int pmem_attach_disk(struct device *dev,
 		struct nd_namespace_common *ndns, struct pmem_device *pmem)
 {
 	int nid = dev_to_node(dev);
+	struct badblocks *bb;
 	struct gendisk *disk;
-	int ret;
 
 	pmem->pmem_queue = blk_alloc_queue_node(GFP_KERNEL, nid);
 	if (!pmem->pmem_queue)
@@ -222,10 +222,15 @@  static int pmem_attach_disk(struct device *dev,
 	set_capacity(disk, (pmem->size - pmem->data_offset) / 512);
 	pmem->pmem_disk = disk;
 
-	ret = nvdimm_namespace_add_poison(disk, pmem->data_offset, ndns);
-	if (ret)
-		return ret;
+	bb = nvdimm_namespace_badblocks(ndns, pmem->data_offset);
+	if (IS_ERR(bb)) {
+		if (PTR_ERR(bb) == -ENOENT)
+			bb = NULL;
+		else
+			return PTR_ERR(bb);
+	}
 
+	disk->bb = bb;
 	add_disk(disk);
 	revalidate_disk(disk);