linux-kernel - Re: [PATCH] libnvdimm: use consistent naming for request_mem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 29 Nov 2016 01:14:13 +0000
From:   "Verma, Vishal L" <vishal.l.verma@...el.com>
To:     "Williams, Dan J" <dan.j.williams@...el.com>,
        "linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] libnvdimm: use consistent naming for
 request_mem_region()

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@...el.com>
> Signed-off-by: Dan Williams <dan.j.williams@...el.com>
> ---
>  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@...el.com>

> 
> 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;
>  	}
>