Re: [PATCH] libnvdimm: use consistent naming for request_mem_region()

[Verma, Vishal L]

On Mon, 2016-11-28 at 11:25 -0800, Dan Williams wrote:
> Here is an example /proc/iomem listing for a system with 2
> namespaces,
> one in "sector" mode and one in "memory" mode:
> 
> Â 1fc000000-2fbffffff : Persistent Memory (legacy)
> ÂÂÂÂ1fc000000-2fbffffff : namespace1.0
> Â 340000000-34fffffff : Persistent Memory
> ÂÂÂÂ340000000-34fffffff : btt0.1
> 
> Here is the corresponding ndctl listing:
> 
> Â # ndctl list
> Â [
> ÂÂÂÂ{
> ÂÂÂÂÂÂ"dev":"namespace1.0",
> ÂÂÂÂÂÂ"mode":"memory",
> ÂÂÂÂÂÂ"size":4294967296,
> ÂÂÂÂÂÂ"blockdev":"pmem1"
> ÂÂÂÂ},
> ÂÂÂÂ{
> ÂÂÂÂÂÂ"dev":"namespace0.0",
> ÂÂÂÂÂÂ"mode":"sector",
> ÂÂÂÂÂÂ"size":267091968,
> ÂÂÂÂÂÂ"uuid":"f7594f86-badb-4592-875f-ded577da2eaf",
> ÂÂÂÂÂÂ"sector_size":4096,
> ÂÂÂÂÂÂ"blockdev":"pmem0s"
> ÂÂÂÂ}
> Â ]
> 
> Notice that the ndctl listing is purely in terms of namespace
> devices,
> while the iomem listing leaks the internal "btt0.1" implementation
> detail. Given that ndctl requires the namespace device name to change
> the mode, for example:
> 
> Â # ndctl create-namespace --reconfig=namespace0.0 --mode=raw --force
> 
> ...use the namespace name in the iomem listing to keep the claiming
> device name consistent across different mode settings.
> 
> Cc: Vishal Verma <vishal.l.verma@xxxxxxxxx>
> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> ---
> Âdrivers/dax/pmem.cÂÂÂÂÂ|ÂÂÂÂ3 ++-
> Âdrivers/nvdimm/claim.c |ÂÂÂÂ2 +-
> Âdrivers/nvdimm/pmem.cÂÂ|ÂÂÂÂ2 +-
> Â3 files changed, 4 insertions(+), 3 deletions(-)

Looks good!

Reveiwed-by: Vishal Verma <vishal.l.verma@xxxxxxxxx>

> 
> diff --git a/drivers/dax/pmem.c b/drivers/dax/pmem.c
> index 9630d8837ba9..3ff84784249a 100644
> --- a/drivers/dax/pmem.c
> +++ b/drivers/dax/pmem.c
> @@ -87,7 +87,8 @@ static int dax_pmem_probe(struct device *dev)
> Â	pfn_sb = nd_pfn->pfn_sb;
> Â
> Â	if (!devm_request_mem_region(dev, nsio->res.start,
> -				resource_size(&nsio->res),
> dev_name(dev))) {
> +				resource_size(&nsio->res),
> +				dev_name(&ndns->dev))) {
> Â		dev_warn(dev, "could not reserve region %pR\n",
> &nsio->res);
> Â		return -EBUSY;
> Â	}
> diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c
> index 8d66fbb779ed..4638b9ea5229 100644
> --- a/drivers/nvdimm/claim.c
> +++ b/drivers/nvdimm/claim.c
> @@ -275,7 +275,7 @@ int devm_nsio_enable(struct device *dev, struct
> nd_namespace_io *nsio)
> Â
> Â	nsio->size = resource_size(res);
> Â	if (!devm_request_mem_region(dev, res->start,
> resource_size(res),
> -				dev_name(dev))) {
> +				dev_name(&ndns->dev))) {
> Â		dev_warn(dev, "could not reserve region %pR\n",
> res);
> Â		return -EBUSY;
> Â	}
> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> index 42b3a8217073..34f16a17c07b 100644
> --- a/drivers/nvdimm/pmem.c
> +++ b/drivers/nvdimm/pmem.c
> @@ -266,7 +266,7 @@ static int pmem_attach_disk(struct device *dev,
> Â		dev_warn(dev, "unable to guarantee persistence of
> writes\n");
> Â
> Â	if (!devm_request_mem_region(dev, res->start,
> resource_size(res),
> -				dev_name(dev))) {
> +				dev_name(&ndns->dev))) {
> Â		dev_warn(dev, "could not reserve region %pR\n",
> res);
> Â		return -EBUSY;
> Â	}
>