diff mbox

[1/3] dev-dax: add support to display badblocks in sysfs for dev-dax

Message ID 148158809239.159590.16102609849408268953.stgit@djiang5-desk3.ch.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dave Jiang Dec. 13, 2016, 12:14 a.m. UTC
Adding support to show badblocks in the pmem region that's provided
by the poison_list. This should show up in
/sys/class/dax/daxN.N/badblocks as read only. Currently we only support
a single resource and we do not support badblocks for seeds. Additional
support code will be implemented to support.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/dax/dax.c  |   27 ++++++++++++++++++++++-----
 drivers/dax/dax.h  |    8 +++++++-
 drivers/dax/pmem.c |   37 ++++++++++++++++++++++++++++++++++++-
 3 files changed, 65 insertions(+), 7 deletions(-)

Comments

Jeff Moyer Dec. 14, 2016, 9:12 p.m. UTC | #1
Dave Jiang <dave.jiang@intel.com> writes:

> Adding support to show badblocks in the pmem region that's provided
> by the poison_list. This should show up in
> /sys/class/dax/daxN.N/badblocks as read only. Currently we only support
> a single resource and we do not support badblocks for seeds. Additional
> support code will be implemented to support.

You didn't wire up a notify event.  Was that intentional?

-Jeff

>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/dax/dax.c  |   27 ++++++++++++++++++++++-----
>  drivers/dax/dax.h  |    8 +++++++-
>  drivers/dax/pmem.c |   37 ++++++++++++++++++++++++++++++++++++-
>  3 files changed, 65 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c
> index 6450c07..2c34cae 100644
> --- a/drivers/dax/dax.c
> +++ b/drivers/dax/dax.c
> @@ -83,6 +83,7 @@ struct dax_dev {
>  	int id;
>  	int num_resources;
>  	struct resource **res;
> +	void *private;
>  };
>  
>  #define for_each_dax_region_resource(dax_region, res) \
> @@ -408,6 +409,20 @@ static struct dax_dev *to_dax_dev(struct device *dev)
>  	return container_of(dev, struct dax_dev, dev);
>  }
>  
> +void *dax_dev_get_private(struct device *dev)
> +{
> +	struct dax_dev *dax_dev = to_dax_dev(dev);
> +
> +	return dax_dev->private;
> +}
> +EXPORT_SYMBOL_GPL(dax_dev_get_private);
> +
> +void dax_dev_set_private(struct dax_dev *dax_dev, void *priv)
> +{
> +	dax_dev->private = priv;
> +}
> +EXPORT_SYMBOL_GPL(dax_dev_set_private);
> +
>  static unsigned long long dax_dev_size(struct dax_dev *dax_dev)
>  {
>  	struct dax_region *dax_region = dax_dev->region;
> @@ -805,7 +820,7 @@ static ssize_t dax_dev_resize(struct dax_region *dax_region,
>  			&& &dax_dev->dev == dax_region->seed) {
>  		struct dax_dev *seed;
>  
> -		seed = devm_create_dax_dev(dax_region, NULL, 0);
> +		seed = devm_create_dax_dev(dax_region, NULL, 0, NULL);
>  		if (IS_ERR(seed))
>  			dev_warn(dax_region->dev,
>  					"failed to create new region seed\n");
> @@ -851,9 +866,10 @@ static struct attribute *dax_device_attributes[] = {
>  	NULL,
>  };
>  
> -static const struct attribute_group dax_device_attribute_group = {
> +struct attribute_group dax_device_attribute_group = {
>  	.attrs = dax_device_attributes,
>  };
> +EXPORT_SYMBOL_GPL(dax_device_attribute_group);
>  
>  static const struct attribute_group *dax_attribute_groups[] = {
>  	&dax_device_attribute_group,
> @@ -1143,7 +1159,8 @@ static void dax_dev_release(struct device *dev)
>  }
>  
>  struct dax_dev *devm_create_dax_dev(struct dax_region *dax_region,
> -		struct resource *res, int count)
> +		struct resource *res, int count,
> +		const struct attribute_group **groups)
>  {
>  	struct device *parent = dax_region->dev;
>  	struct dax_dev *dax_dev;
> @@ -1248,7 +1265,7 @@ struct dax_dev *devm_create_dax_dev(struct dax_region *dax_region,
>  	dev->devt = dev_t;
>  	dev->class = dax_class;
>  	dev->parent = parent;
> -	dev->groups = dax_attribute_groups;
> +	dev->groups = groups ? groups : dax_attribute_groups;
>  	dev->release = dax_dev_release;
>  	dev_set_name(dev, "dax%d.%d", dax_region->id, dax_dev->id);
>  	/* update resource names now that the owner device is named */
> @@ -1268,7 +1285,7 @@ struct dax_dev *devm_create_dax_dev(struct dax_region *dax_region,
>  	if (atomic_inc_return(&dax_region->child_count) == 1) {
>  		struct dax_dev *seed;
>  
> -		seed = devm_create_dax_dev(dax_region, NULL, 0);
> +		seed = devm_create_dax_dev(dax_region, NULL, 0, NULL);
>  		if (IS_ERR(seed))
>  			dev_warn(parent, "failed to create region seed\n");
>  		else
> diff --git a/drivers/dax/dax.h b/drivers/dax/dax.h
> index ddd829a..c23c7ac 100644
> --- a/drivers/dax/dax.h
> +++ b/drivers/dax/dax.h
> @@ -16,10 +16,16 @@ struct device;
>  struct dax_dev;
>  struct resource;
>  struct dax_region;
> +extern struct attribute_group dax_device_attribute_group;
> +
>  void dax_region_put(struct dax_region *dax_region);
>  struct dax_region *alloc_dax_region(struct device *parent,
>  		int region_id, struct resource *res, unsigned int align,
>  		void *addr, unsigned long flags);
>  struct dax_dev *devm_create_dax_dev(struct dax_region *dax_region,
> -		struct resource *res, int count);
> +		struct resource *res, int count,
> +		const struct attribute_group **groups);
> +void *dax_dev_get_private(struct device *dev);
> +void dax_dev_set_private(struct dax_dev *dax_dev, void *priv);
> +
>  #endif /* __DAX_H__ */
> diff --git a/drivers/dax/pmem.c b/drivers/dax/pmem.c
> index 033f49b3..808d3bb 100644
> --- a/drivers/dax/pmem.c
> +++ b/drivers/dax/pmem.c
> @@ -14,6 +14,7 @@
>  #include <linux/memremap.h>
>  #include <linux/module.h>
>  #include <linux/pfn_t.h>
> +#include <linux/badblocks.h>
>  #include "../nvdimm/pfn.h"
>  #include "../nvdimm/nd.h"
>  #include "dax.h"
> @@ -22,6 +23,32 @@ struct dax_pmem {
>  	struct device *dev;
>  	struct percpu_ref ref;
>  	struct completion cmp;
> +	struct badblocks bb;
> +};
> +
> +static ssize_t dax_pmem_badblocks_show(struct device *dev,
> +		struct device_attribute *attr, char *page)
> +{
> +	struct dax_pmem *dax_pmem =
> +		(struct dax_pmem *)dax_dev_get_private(dev);
> +
> +	return badblocks_show(&dax_pmem->bb, page, 0);
> +}
> +static DEVICE_ATTR(badblocks, S_IRUGO, dax_pmem_badblocks_show, NULL);
> +
> +static struct attribute *dax_pmem_badblock_attributes[] = {
> +	&dev_attr_badblocks.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group dax_pmem_badblock_attribute_group = {
> +	.attrs = dax_pmem_badblock_attributes,
> +};
> +
> +static const struct attribute_group *dax_pmem_attribute_groups[] = {
> +	&dax_device_attribute_group,
> +	&dax_pmem_badblock_attribute_group,
> +	NULL,
>  };
>  
>  static struct dax_pmem *to_dax_pmem(struct percpu_ref *ref)
> @@ -130,7 +157,15 @@ static int dax_pmem_probe(struct device *dev)
>  		return -ENOMEM;
>  
>  	/* TODO: support for subdividing a dax region... */
> -	dax_dev = devm_create_dax_dev(dax_region, &res, 1);
> +	dax_dev = devm_create_dax_dev(dax_region, &res, 1,
> +			dax_pmem_attribute_groups);
> +	dax_dev_set_private(dax_dev, dax_pmem);
> +
> +	rc = devm_init_badblocks(dev, &dax_pmem->bb);
> +	if (rc)
> +		return rc;
> +
> +	nvdimm_badblocks_populate(nd_region, &dax_pmem->bb, &res);
>  
>  	/* child dax_dev instances now own the lifetime of the dax_region */
>  	dax_region_put(dax_region);
>
> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm
Dave Jiang Dec. 14, 2016, 10:51 p.m. UTC | #2
On 12/14/2016 02:12 PM, Jeff Moyer wrote:
> Dave Jiang <dave.jiang@intel.com> writes:
> 
>> Adding support to show badblocks in the pmem region that's provided
>> by the poison_list. This should show up in
>> /sys/class/dax/daxN.N/badblocks as read only. Currently we only support
>> a single resource and we do not support badblocks for seeds. Additional
>> support code will be implemented to support.
> 
> You didn't wire up a notify event.  Was that intentional?

No. I totally missed it. Good catch!

> 
> -Jeff
> 
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>>  drivers/dax/dax.c  |   27 ++++++++++++++++++++++-----
>>  drivers/dax/dax.h  |    8 +++++++-
>>  drivers/dax/pmem.c |   37 ++++++++++++++++++++++++++++++++++++-
>>  3 files changed, 65 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c
>> index 6450c07..2c34cae 100644
>> --- a/drivers/dax/dax.c
>> +++ b/drivers/dax/dax.c
>> @@ -83,6 +83,7 @@ struct dax_dev {
>>  	int id;
>>  	int num_resources;
>>  	struct resource **res;
>> +	void *private;
>>  };
>>  
>>  #define for_each_dax_region_resource(dax_region, res) \
>> @@ -408,6 +409,20 @@ static struct dax_dev *to_dax_dev(struct device *dev)
>>  	return container_of(dev, struct dax_dev, dev);
>>  }
>>  
>> +void *dax_dev_get_private(struct device *dev)
>> +{
>> +	struct dax_dev *dax_dev = to_dax_dev(dev);
>> +
>> +	return dax_dev->private;
>> +}
>> +EXPORT_SYMBOL_GPL(dax_dev_get_private);
>> +
>> +void dax_dev_set_private(struct dax_dev *dax_dev, void *priv)
>> +{
>> +	dax_dev->private = priv;
>> +}
>> +EXPORT_SYMBOL_GPL(dax_dev_set_private);
>> +
>>  static unsigned long long dax_dev_size(struct dax_dev *dax_dev)
>>  {
>>  	struct dax_region *dax_region = dax_dev->region;
>> @@ -805,7 +820,7 @@ static ssize_t dax_dev_resize(struct dax_region *dax_region,
>>  			&& &dax_dev->dev == dax_region->seed) {
>>  		struct dax_dev *seed;
>>  
>> -		seed = devm_create_dax_dev(dax_region, NULL, 0);
>> +		seed = devm_create_dax_dev(dax_region, NULL, 0, NULL);
>>  		if (IS_ERR(seed))
>>  			dev_warn(dax_region->dev,
>>  					"failed to create new region seed\n");
>> @@ -851,9 +866,10 @@ static struct attribute *dax_device_attributes[] = {
>>  	NULL,
>>  };
>>  
>> -static const struct attribute_group dax_device_attribute_group = {
>> +struct attribute_group dax_device_attribute_group = {
>>  	.attrs = dax_device_attributes,
>>  };
>> +EXPORT_SYMBOL_GPL(dax_device_attribute_group);
>>  
>>  static const struct attribute_group *dax_attribute_groups[] = {
>>  	&dax_device_attribute_group,
>> @@ -1143,7 +1159,8 @@ static void dax_dev_release(struct device *dev)
>>  }
>>  
>>  struct dax_dev *devm_create_dax_dev(struct dax_region *dax_region,
>> -		struct resource *res, int count)
>> +		struct resource *res, int count,
>> +		const struct attribute_group **groups)
>>  {
>>  	struct device *parent = dax_region->dev;
>>  	struct dax_dev *dax_dev;
>> @@ -1248,7 +1265,7 @@ struct dax_dev *devm_create_dax_dev(struct dax_region *dax_region,
>>  	dev->devt = dev_t;
>>  	dev->class = dax_class;
>>  	dev->parent = parent;
>> -	dev->groups = dax_attribute_groups;
>> +	dev->groups = groups ? groups : dax_attribute_groups;
>>  	dev->release = dax_dev_release;
>>  	dev_set_name(dev, "dax%d.%d", dax_region->id, dax_dev->id);
>>  	/* update resource names now that the owner device is named */
>> @@ -1268,7 +1285,7 @@ struct dax_dev *devm_create_dax_dev(struct dax_region *dax_region,
>>  	if (atomic_inc_return(&dax_region->child_count) == 1) {
>>  		struct dax_dev *seed;
>>  
>> -		seed = devm_create_dax_dev(dax_region, NULL, 0);
>> +		seed = devm_create_dax_dev(dax_region, NULL, 0, NULL);
>>  		if (IS_ERR(seed))
>>  			dev_warn(parent, "failed to create region seed\n");
>>  		else
>> diff --git a/drivers/dax/dax.h b/drivers/dax/dax.h
>> index ddd829a..c23c7ac 100644
>> --- a/drivers/dax/dax.h
>> +++ b/drivers/dax/dax.h
>> @@ -16,10 +16,16 @@ struct device;
>>  struct dax_dev;
>>  struct resource;
>>  struct dax_region;
>> +extern struct attribute_group dax_device_attribute_group;
>> +
>>  void dax_region_put(struct dax_region *dax_region);
>>  struct dax_region *alloc_dax_region(struct device *parent,
>>  		int region_id, struct resource *res, unsigned int align,
>>  		void *addr, unsigned long flags);
>>  struct dax_dev *devm_create_dax_dev(struct dax_region *dax_region,
>> -		struct resource *res, int count);
>> +		struct resource *res, int count,
>> +		const struct attribute_group **groups);
>> +void *dax_dev_get_private(struct device *dev);
>> +void dax_dev_set_private(struct dax_dev *dax_dev, void *priv);
>> +
>>  #endif /* __DAX_H__ */
>> diff --git a/drivers/dax/pmem.c b/drivers/dax/pmem.c
>> index 033f49b3..808d3bb 100644
>> --- a/drivers/dax/pmem.c
>> +++ b/drivers/dax/pmem.c
>> @@ -14,6 +14,7 @@
>>  #include <linux/memremap.h>
>>  #include <linux/module.h>
>>  #include <linux/pfn_t.h>
>> +#include <linux/badblocks.h>
>>  #include "../nvdimm/pfn.h"
>>  #include "../nvdimm/nd.h"
>>  #include "dax.h"
>> @@ -22,6 +23,32 @@ struct dax_pmem {
>>  	struct device *dev;
>>  	struct percpu_ref ref;
>>  	struct completion cmp;
>> +	struct badblocks bb;
>> +};
>> +
>> +static ssize_t dax_pmem_badblocks_show(struct device *dev,
>> +		struct device_attribute *attr, char *page)
>> +{
>> +	struct dax_pmem *dax_pmem =
>> +		(struct dax_pmem *)dax_dev_get_private(dev);
>> +
>> +	return badblocks_show(&dax_pmem->bb, page, 0);
>> +}
>> +static DEVICE_ATTR(badblocks, S_IRUGO, dax_pmem_badblocks_show, NULL);
>> +
>> +static struct attribute *dax_pmem_badblock_attributes[] = {
>> +	&dev_attr_badblocks.attr,
>> +	NULL,
>> +};
>> +
>> +static struct attribute_group dax_pmem_badblock_attribute_group = {
>> +	.attrs = dax_pmem_badblock_attributes,
>> +};
>> +
>> +static const struct attribute_group *dax_pmem_attribute_groups[] = {
>> +	&dax_device_attribute_group,
>> +	&dax_pmem_badblock_attribute_group,
>> +	NULL,
>>  };
>>  
>>  static struct dax_pmem *to_dax_pmem(struct percpu_ref *ref)
>> @@ -130,7 +157,15 @@ static int dax_pmem_probe(struct device *dev)
>>  		return -ENOMEM;
>>  
>>  	/* TODO: support for subdividing a dax region... */
>> -	dax_dev = devm_create_dax_dev(dax_region, &res, 1);
>> +	dax_dev = devm_create_dax_dev(dax_region, &res, 1,
>> +			dax_pmem_attribute_groups);
>> +	dax_dev_set_private(dax_dev, dax_pmem);
>> +
>> +	rc = devm_init_badblocks(dev, &dax_pmem->bb);
>> +	if (rc)
>> +		return rc;
>> +
>> +	nvdimm_badblocks_populate(nd_region, &dax_pmem->bb, &res);
>>  
>>  	/* child dax_dev instances now own the lifetime of the dax_region */
>>  	dax_region_put(dax_region);
>>
>> _______________________________________________
>> Linux-nvdimm mailing list
>> Linux-nvdimm@lists.01.org
>> https://lists.01.org/mailman/listinfo/linux-nvdimm
Elliott, Robert (Servers) Dec. 16, 2016, 1:52 a.m. UTC | #3
> -----Original Message-----
> From: Linux-nvdimm [mailto:linux-nvdimm-bounces@lists.01.org] On
> Behalf Of Dave Jiang
> Sent: Monday, December 12, 2016 6:15 PM
> Subject: [PATCH 1/3] dev-dax: add support to display badblocks in
> sysfs for dev-dax
> 
> Adding support to show badblocks in the pmem region that's provided
> by the poison_list. This should show up in
> /sys/class/dax/daxN.N/badblocks as read only. Currently we only
> support
> a single resource and we do not support badblocks for seeds.
> Additional
> support code will be implemented to support.
> 
...
> diff --git a/drivers/dax/pmem.c b/drivers/dax/pmem.c
...
> +static ssize_t dax_pmem_badblocks_show(struct device *dev,
> +		struct device_attribute *attr, char *page)
> +{
> +	struct dax_pmem *dax_pmem =
> +		(struct dax_pmem *)dax_dev_get_private(dev);
> +
> +	return badblocks_show(&dax_pmem->bb, page, 0);
> +}

Do /dev/dax devices have a "block size" to explain the units used in
the sysfs files?  If so, where is it reported?

This directory has the total size in bytes (here, 8 GiB - 2 MiB):
/sys/class/dax/dax0.0/dev:242:0
/sys/class/dax/dax0.0/size:8587837440
/sys/class/dax/dax0.0/uevent:MAJOR=242
/sys/class/dax/dax0.0/uevent:MINOR=0
/sys/class/dax/dax0.0/uevent:DEVNAME=dax0.0

In contrast, /dev/pmem devices have a full "queue" directory,
including logical_block_size.

/sys/class/block/pmem0/device/devtype:nd_namespace_io
/sys/class/block/pmem0/device/force_raw:0
/sys/class/block/pmem0/device/modalias:nd:t4
/sys/class/block/pmem0/device/mode:raw
/sys/class/block/pmem0/device/nstype:4
/sys/class/block/pmem0/device/numa_node:0
/sys/class/block/pmem0/device/resource:0x880000000
/sys/class/block/pmem0/device/size:8589934592
/sys/class/block/pmem0/device/uevent:DEVTYPE=nd_namespace_io
/sys/class/block/pmem0/device/uevent:DRIVER=nd_pmem
/sys/class/block/pmem0/device/uevent:MODALIAS=nd:t4
/sys/class/block/pmem0/queue/add_random:0
/sys/class/block/pmem0/queue/dax:1
/sys/class/block/pmem0/queue/discard_granularity:0
/sys/class/block/pmem0/queue/discard_max_bytes:0
/sys/class/block/pmem0/queue/discard_max_hw_bytes:0
/sys/class/block/pmem0/queue/discard_zeroes_data:0
/sys/class/block/pmem0/queue/hw_sector_size:512
/sys/class/block/pmem0/queue/io_poll:0
/sys/class/block/pmem0/queue/iostats:0
/sys/class/block/pmem0/queue/logical_block_size:512
/sys/class/block/pmem0/queue/max_hw_sectors_kb:2147483647
/sys/class/block/pmem0/queue/max_integrity_segments:0
/sys/class/block/pmem0/queue/max_sectors_kb:1280
/sys/class/block/pmem0/queue/max_segments:128
/sys/class/block/pmem0/queue/max_segment_size:65536
/sys/class/block/pmem0/queue/minimum_io_size:4096
/sys/class/block/pmem0/queue/nomerges:0
/sys/class/block/pmem0/queue/nr_requests:128
/sys/class/block/pmem0/queue/optimal_io_size:0
/sys/class/block/pmem0/queue/physical_block_size:4096
/sys/class/block/pmem0/queue/read_ahead_kb:128
/sys/class/block/pmem0/queue/rotational:0
/sys/class/block/pmem0/queue/rq_affinity:0
/sys/class/block/pmem0/queue/scheduler:none
/sys/class/block/pmem0/queue/write_cache:write back
/sys/class/block/pmem0/queue/write_same_max_bytes:0


---
Robert Elliott, HPE Persistent Memory
Dan Williams Dec. 16, 2016, 2:43 a.m. UTC | #4
On Thu, Dec 15, 2016 at 5:52 PM, Elliott, Robert (Persistent Memory)
<elliott@hpe.com> wrote:
>> -----Original Message-----
>> From: Linux-nvdimm [mailto:linux-nvdimm-bounces@lists.01.org] On
>> Behalf Of Dave Jiang
>> Sent: Monday, December 12, 2016 6:15 PM
>> Subject: [PATCH 1/3] dev-dax: add support to display badblocks in
>> sysfs for dev-dax
>>
>> Adding support to show badblocks in the pmem region that's provided
>> by the poison_list. This should show up in
>> /sys/class/dax/daxN.N/badblocks as read only. Currently we only
>> support
>> a single resource and we do not support badblocks for seeds.
>> Additional
>> support code will be implemented to support.
>>
> ...
>> diff --git a/drivers/dax/pmem.c b/drivers/dax/pmem.c
> ...
>> +static ssize_t dax_pmem_badblocks_show(struct device *dev,
>> +struct device_attribute *attr, char *page)
>> +{
>> +struct dax_pmem *dax_pmem =
>> +(struct dax_pmem *)dax_dev_get_private(dev);
>> +
>> +return badblocks_show(&dax_pmem->bb, page, 0);
>> +}
>
> Do /dev/dax devices have a "block size" to explain the units used in
> the sysfs files?  If so, where is it reported?

The "badblocks" sysfs interface is always in 512 byte logical units.
If badblocks ever grew support for other block sizes we would add it
as an attribute of the badblocks interface directly and not rely on
traversing other parts of the sysfs topology.
diff mbox

Patch

diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c
index 6450c07..2c34cae 100644
--- a/drivers/dax/dax.c
+++ b/drivers/dax/dax.c
@@ -83,6 +83,7 @@  struct dax_dev {
 	int id;
 	int num_resources;
 	struct resource **res;
+	void *private;
 };
 
 #define for_each_dax_region_resource(dax_region, res) \
@@ -408,6 +409,20 @@  static struct dax_dev *to_dax_dev(struct device *dev)
 	return container_of(dev, struct dax_dev, dev);
 }
 
+void *dax_dev_get_private(struct device *dev)
+{
+	struct dax_dev *dax_dev = to_dax_dev(dev);
+
+	return dax_dev->private;
+}
+EXPORT_SYMBOL_GPL(dax_dev_get_private);
+
+void dax_dev_set_private(struct dax_dev *dax_dev, void *priv)
+{
+	dax_dev->private = priv;
+}
+EXPORT_SYMBOL_GPL(dax_dev_set_private);
+
 static unsigned long long dax_dev_size(struct dax_dev *dax_dev)
 {
 	struct dax_region *dax_region = dax_dev->region;
@@ -805,7 +820,7 @@  static ssize_t dax_dev_resize(struct dax_region *dax_region,
 			&& &dax_dev->dev == dax_region->seed) {
 		struct dax_dev *seed;
 
-		seed = devm_create_dax_dev(dax_region, NULL, 0);
+		seed = devm_create_dax_dev(dax_region, NULL, 0, NULL);
 		if (IS_ERR(seed))
 			dev_warn(dax_region->dev,
 					"failed to create new region seed\n");
@@ -851,9 +866,10 @@  static struct attribute *dax_device_attributes[] = {
 	NULL,
 };
 
-static const struct attribute_group dax_device_attribute_group = {
+struct attribute_group dax_device_attribute_group = {
 	.attrs = dax_device_attributes,
 };
+EXPORT_SYMBOL_GPL(dax_device_attribute_group);
 
 static const struct attribute_group *dax_attribute_groups[] = {
 	&dax_device_attribute_group,
@@ -1143,7 +1159,8 @@  static void dax_dev_release(struct device *dev)
 }
 
 struct dax_dev *devm_create_dax_dev(struct dax_region *dax_region,
-		struct resource *res, int count)
+		struct resource *res, int count,
+		const struct attribute_group **groups)
 {
 	struct device *parent = dax_region->dev;
 	struct dax_dev *dax_dev;
@@ -1248,7 +1265,7 @@  struct dax_dev *devm_create_dax_dev(struct dax_region *dax_region,
 	dev->devt = dev_t;
 	dev->class = dax_class;
 	dev->parent = parent;
-	dev->groups = dax_attribute_groups;
+	dev->groups = groups ? groups : dax_attribute_groups;
 	dev->release = dax_dev_release;
 	dev_set_name(dev, "dax%d.%d", dax_region->id, dax_dev->id);
 	/* update resource names now that the owner device is named */
@@ -1268,7 +1285,7 @@  struct dax_dev *devm_create_dax_dev(struct dax_region *dax_region,
 	if (atomic_inc_return(&dax_region->child_count) == 1) {
 		struct dax_dev *seed;
 
-		seed = devm_create_dax_dev(dax_region, NULL, 0);
+		seed = devm_create_dax_dev(dax_region, NULL, 0, NULL);
 		if (IS_ERR(seed))
 			dev_warn(parent, "failed to create region seed\n");
 		else
diff --git a/drivers/dax/dax.h b/drivers/dax/dax.h
index ddd829a..c23c7ac 100644
--- a/drivers/dax/dax.h
+++ b/drivers/dax/dax.h
@@ -16,10 +16,16 @@  struct device;
 struct dax_dev;
 struct resource;
 struct dax_region;
+extern struct attribute_group dax_device_attribute_group;
+
 void dax_region_put(struct dax_region *dax_region);
 struct dax_region *alloc_dax_region(struct device *parent,
 		int region_id, struct resource *res, unsigned int align,
 		void *addr, unsigned long flags);
 struct dax_dev *devm_create_dax_dev(struct dax_region *dax_region,
-		struct resource *res, int count);
+		struct resource *res, int count,
+		const struct attribute_group **groups);
+void *dax_dev_get_private(struct device *dev);
+void dax_dev_set_private(struct dax_dev *dax_dev, void *priv);
+
 #endif /* __DAX_H__ */
diff --git a/drivers/dax/pmem.c b/drivers/dax/pmem.c
index 033f49b3..808d3bb 100644
--- a/drivers/dax/pmem.c
+++ b/drivers/dax/pmem.c
@@ -14,6 +14,7 @@ 
 #include <linux/memremap.h>
 #include <linux/module.h>
 #include <linux/pfn_t.h>
+#include <linux/badblocks.h>
 #include "../nvdimm/pfn.h"
 #include "../nvdimm/nd.h"
 #include "dax.h"
@@ -22,6 +23,32 @@  struct dax_pmem {
 	struct device *dev;
 	struct percpu_ref ref;
 	struct completion cmp;
+	struct badblocks bb;
+};
+
+static ssize_t dax_pmem_badblocks_show(struct device *dev,
+		struct device_attribute *attr, char *page)
+{
+	struct dax_pmem *dax_pmem =
+		(struct dax_pmem *)dax_dev_get_private(dev);
+
+	return badblocks_show(&dax_pmem->bb, page, 0);
+}
+static DEVICE_ATTR(badblocks, S_IRUGO, dax_pmem_badblocks_show, NULL);
+
+static struct attribute *dax_pmem_badblock_attributes[] = {
+	&dev_attr_badblocks.attr,
+	NULL,
+};
+
+static struct attribute_group dax_pmem_badblock_attribute_group = {
+	.attrs = dax_pmem_badblock_attributes,
+};
+
+static const struct attribute_group *dax_pmem_attribute_groups[] = {
+	&dax_device_attribute_group,
+	&dax_pmem_badblock_attribute_group,
+	NULL,
 };
 
 static struct dax_pmem *to_dax_pmem(struct percpu_ref *ref)
@@ -130,7 +157,15 @@  static int dax_pmem_probe(struct device *dev)
 		return -ENOMEM;
 
 	/* TODO: support for subdividing a dax region... */
-	dax_dev = devm_create_dax_dev(dax_region, &res, 1);
+	dax_dev = devm_create_dax_dev(dax_region, &res, 1,
+			dax_pmem_attribute_groups);
+	dax_dev_set_private(dax_dev, dax_pmem);
+
+	rc = devm_init_badblocks(dev, &dax_pmem->bb);
+	if (rc)
+		return rc;
+
+	nvdimm_badblocks_populate(nd_region, &dax_pmem->bb, &res);
 
 	/* child dax_dev instances now own the lifetime of the dax_region */
 	dax_region_put(dax_region);