* [PATCH] libnvdimm: call devm_namespace_disable() on error
@ 2020-07-03 11:18 Hannes Reinecke
2020-07-09 5:39 ` Santosh Sivaraj
2020-07-09 5:57 ` Santosh Sivaraj
0 siblings, 2 replies; 4+ messages in thread
From: Hannes Reinecke @ 2020-07-03 11:18 UTC (permalink / raw)
To: Dan Williams; +Cc: linux-nvdimm, Hannes Reinecke
Once devm_namespace_enable() has been called the error path in the
calling function will not call devm_namespace_disable(), leaving the
namespace enabled on error.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/dax/pmem/core.c | 2 +-
drivers/nvdimm/btt.c | 5 ++++-
drivers/nvdimm/claim.c | 8 +++++++-
drivers/nvdimm/pfn_devs.c | 1 +
drivers/nvdimm/pmem.c | 20 ++++++++++----------
5 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/drivers/dax/pmem/core.c b/drivers/dax/pmem/core.c
index 2bedf8414fff..4b26434f0aca 100644
--- a/drivers/dax/pmem/core.c
+++ b/drivers/dax/pmem/core.c
@@ -31,9 +31,9 @@ struct dev_dax *__dax_pmem_probe(struct device *dev, enum dev_dax_subsys subsys)
if (rc)
return ERR_PTR(rc);
rc = nvdimm_setup_pfn(nd_pfn, &pgmap);
+ devm_namespace_disable(dev, ndns);
if (rc)
return ERR_PTR(rc);
- devm_namespace_disable(dev, ndns);
/* reserve the metadata area, device-dax will reserve the data */
pfn_sb = nd_pfn->pfn_sb;
diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index 48e9d169b6f9..bd4747f2c99b 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -1704,13 +1704,16 @@ int nvdimm_namespace_attach_btt(struct nd_namespace_common *ndns)
dev_dbg(&nd_btt->dev, "%s must be at least %ld bytes\n",
dev_name(&ndns->dev),
ARENA_MIN_SIZE + nd_btt->initial_offset);
+ devm_namespace_disable(&nd_btt->dev, ndns);
return -ENXIO;
}
nd_region = to_nd_region(nd_btt->dev.parent);
btt = btt_init(nd_btt, rawsize, nd_btt->lbasize, nd_btt->uuid,
nd_region);
- if (!btt)
+ if (!btt) {
+ devm_namespace_disable(&nd_btt->dev, ndns);
return -ENOMEM;
+ }
nd_btt->btt = btt;
return 0;
diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c
index 45964acba944..15fd1b92d32f 100644
--- a/drivers/nvdimm/claim.c
+++ b/drivers/nvdimm/claim.c
@@ -314,12 +314,18 @@ int devm_nsio_enable(struct device *dev, struct nd_namespace_io *nsio,
}
ndns->rw_bytes = nsio_rw_bytes;
- if (devm_init_badblocks(dev, &nsio->bb))
+ if (devm_init_badblocks(dev, &nsio->bb)) {
+ devm_release_mem_region(dev, res->start, size);
return -ENOMEM;
+ }
nvdimm_badblocks_populate(to_nd_region(ndns->dev.parent), &nsio->bb,
&nsio->res);
nsio->addr = devm_memremap(dev, res->start, size, ARCH_MEMREMAP_PMEM);
+ if (IS_ERR(nsio->addr)) {
+ devm_exit_badblocks(dev, &nsio->bb);
+ devm_release_mem_region(dev, res->start, size);
+ }
return PTR_ERR_OR_ZERO(nsio->addr);
}
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index 34db557dbad1..9faa92662643 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -408,6 +408,7 @@ static int nd_pfn_clear_memmap_errors(struct nd_pfn *nd_pfn)
nsoff += chunk;
}
if (rc) {
+ devm_namespace_disable(&nd_pfn->dev, ndns);
dev_err(&nd_pfn->dev,
"error clearing %x badblocks at %llx\n",
num_bad, first_bad);
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index d25e66fd942d..4f667fe6ef72 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -401,8 +401,10 @@ static int pmem_attach_disk(struct device *dev,
if (is_nd_pfn(dev)) {
nd_pfn = to_nd_pfn(dev);
rc = nvdimm_setup_pfn(nd_pfn, &pmem->pgmap);
- if (rc)
+ if (rc) {
+ devm_namespace_disable(dev, ndns);
return rc;
+ }
}
/* we're attaching a block device, disable raw namespace access */
@@ -549,17 +551,15 @@ static int nd_pmem_probe(struct device *dev)
ret = nd_pfn_probe(dev, ndns);
if (ret == 0)
return -ENXIO;
- else if (ret == -EOPNOTSUPP)
- return ret;
-
- ret = nd_dax_probe(dev, ndns);
- if (ret == 0)
- return -ENXIO;
- else if (ret == -EOPNOTSUPP)
- return ret;
-
+ else if (ret != EOPNOTSUPP) {
+ ret = nd_dax_probe(dev, ndns);
+ if (ret == 0)
+ return -ENXIO;
+ }
/* probe complete, attach handles namespace enabling */
devm_namespace_disable(dev, ndns);
+ if (ret == -EOPNOTSUPP)
+ return ret;
return pmem_attach_disk(dev, ndns);
}
--
2.16.4
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] libnvdimm: call devm_namespace_disable() on error
2020-07-03 11:18 [PATCH] libnvdimm: call devm_namespace_disable() on error Hannes Reinecke
@ 2020-07-09 5:39 ` Santosh Sivaraj
2020-07-13 14:35 ` Hannes Reinecke
2020-07-09 5:57 ` Santosh Sivaraj
1 sibling, 1 reply; 4+ messages in thread
From: Santosh Sivaraj @ 2020-07-09 5:39 UTC (permalink / raw)
To: Hannes Reinecke, Dan Williams; +Cc: linux-nvdimm, Hannes Reinecke
Hi Hannes,
Hannes Reinecke <hare@suse.de> writes:
> Once devm_namespace_enable() has been called the error path in the
> calling function will not call devm_namespace_disable(), leaving the
> namespace enabled on error.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> drivers/dax/pmem/core.c | 2 +-
> drivers/nvdimm/btt.c | 5 ++++-
> drivers/nvdimm/claim.c | 8 +++++++-
> drivers/nvdimm/pfn_devs.c | 1 +
> drivers/nvdimm/pmem.c | 20 ++++++++++----------
> 5 files changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/dax/pmem/core.c b/drivers/dax/pmem/core.c
> index 2bedf8414fff..4b26434f0aca 100644
> --- a/drivers/dax/pmem/core.c
> +++ b/drivers/dax/pmem/core.c
> @@ -31,9 +31,9 @@ struct dev_dax *__dax_pmem_probe(struct device *dev, enum dev_dax_subsys subsys)
> if (rc)
> return ERR_PTR(rc);
> rc = nvdimm_setup_pfn(nd_pfn, &pgmap);
> + devm_namespace_disable(dev, ndns);
> if (rc)
> return ERR_PTR(rc);
> - devm_namespace_disable(dev, ndns);
>
> /* reserve the metadata area, device-dax will reserve the data */
> pfn_sb = nd_pfn->pfn_sb;
> diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
> index 48e9d169b6f9..bd4747f2c99b 100644
> --- a/drivers/nvdimm/btt.c
> +++ b/drivers/nvdimm/btt.c
> @@ -1704,13 +1704,16 @@ int nvdimm_namespace_attach_btt(struct nd_namespace_common *ndns)
> dev_dbg(&nd_btt->dev, "%s must be at least %ld bytes\n",
> dev_name(&ndns->dev),
> ARENA_MIN_SIZE + nd_btt->initial_offset);
> + devm_namespace_disable(&nd_btt->dev, ndns);
> return -ENXIO;
> }
> nd_region = to_nd_region(nd_btt->dev.parent);
> btt = btt_init(nd_btt, rawsize, nd_btt->lbasize, nd_btt->uuid,
> nd_region);
> - if (!btt)
> + if (!btt) {
> + devm_namespace_disable(&nd_btt->dev, ndns);
> return -ENOMEM;
> + }
> nd_btt->btt = btt;
>
> return 0;
> diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c
> index 45964acba944..15fd1b92d32f 100644
> --- a/drivers/nvdimm/claim.c
> +++ b/drivers/nvdimm/claim.c
> @@ -314,12 +314,18 @@ int devm_nsio_enable(struct device *dev, struct nd_namespace_io *nsio,
> }
>
> ndns->rw_bytes = nsio_rw_bytes;
> - if (devm_init_badblocks(dev, &nsio->bb))
> + if (devm_init_badblocks(dev, &nsio->bb)) {
> + devm_release_mem_region(dev, res->start, size);
> return -ENOMEM;
> + }
> nvdimm_badblocks_populate(to_nd_region(ndns->dev.parent), &nsio->bb,
> &nsio->res);
>
> nsio->addr = devm_memremap(dev, res->start, size, ARCH_MEMREMAP_PMEM);
> + if (IS_ERR(nsio->addr)) {
> + devm_exit_badblocks(dev, &nsio->bb);
> + devm_release_mem_region(dev, res->start, size);
> + }
>
> return PTR_ERR_OR_ZERO(nsio->addr);
> }
> diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
> index 34db557dbad1..9faa92662643 100644
> --- a/drivers/nvdimm/pfn_devs.c
> +++ b/drivers/nvdimm/pfn_devs.c
> @@ -408,6 +408,7 @@ static int nd_pfn_clear_memmap_errors(struct nd_pfn *nd_pfn)
> nsoff += chunk;
> }
> if (rc) {
> + devm_namespace_disable(&nd_pfn->dev, ndns);
The disable here seems to be wrong.
This function called from the pmem_attach_disk path, where its expected the
namespace is enabled after the setup_pfn. Also in case of an error, we do
another disable in pmem_attach_disk.
Thanks,
Santosh
> dev_err(&nd_pfn->dev,
> "error clearing %x badblocks at %llx\n",
> num_bad, first_bad);
> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> index d25e66fd942d..4f667fe6ef72 100644
> --- a/drivers/nvdimm/pmem.c
> +++ b/drivers/nvdimm/pmem.c
> @@ -401,8 +401,10 @@ static int pmem_attach_disk(struct device *dev,
> if (is_nd_pfn(dev)) {
> nd_pfn = to_nd_pfn(dev);
> rc = nvdimm_setup_pfn(nd_pfn, &pmem->pgmap);
> - if (rc)
> + if (rc) {
> + devm_namespace_disable(dev, ndns);
> return rc;
> + }
> }
>
> /* we're attaching a block device, disable raw namespace access */
> @@ -549,17 +551,15 @@ static int nd_pmem_probe(struct device *dev)
> ret = nd_pfn_probe(dev, ndns);
> if (ret == 0)
> return -ENXIO;
> - else if (ret == -EOPNOTSUPP)
> - return ret;
> -
> - ret = nd_dax_probe(dev, ndns);
> - if (ret == 0)
> - return -ENXIO;
> - else if (ret == -EOPNOTSUPP)
> - return ret;
> -
> + else if (ret != EOPNOTSUPP) {
> + ret = nd_dax_probe(dev, ndns);
> + if (ret == 0)
> + return -ENXIO;
> + }
> /* probe complete, attach handles namespace enabling */
> devm_namespace_disable(dev, ndns);
> + if (ret == -EOPNOTSUPP)
> + return ret;
>
> return pmem_attach_disk(dev, ndns);
> }
> --
> 2.16.4
> _______________________________________________
> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libnvdimm: call devm_namespace_disable() on error
2020-07-03 11:18 [PATCH] libnvdimm: call devm_namespace_disable() on error Hannes Reinecke
2020-07-09 5:39 ` Santosh Sivaraj
@ 2020-07-09 5:57 ` Santosh Sivaraj
1 sibling, 0 replies; 4+ messages in thread
From: Santosh Sivaraj @ 2020-07-09 5:57 UTC (permalink / raw)
To: Hannes Reinecke, Dan Williams; +Cc: linux-nvdimm, Hannes Reinecke
Hi! Hannes,
Hannes Reinecke <hare@suse.de> writes:
> Once devm_namespace_enable() has been called the error path in the
> calling function will not call devm_namespace_disable(), leaving the
> namespace enabled on error.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> drivers/dax/pmem/core.c | 2 +-
> drivers/nvdimm/btt.c | 5 ++++-
> drivers/nvdimm/claim.c | 8 +++++++-
> drivers/nvdimm/pfn_devs.c | 1 +
> drivers/nvdimm/pmem.c | 20 ++++++++++----------
> 5 files changed, 23 insertions(+), 13 deletions(-)
>
[snip]
> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> index d25e66fd942d..4f667fe6ef72 100644
> --- a/drivers/nvdimm/pmem.c
> +++ b/drivers/nvdimm/pmem.c
> @@ -401,8 +401,10 @@ static int pmem_attach_disk(struct device *dev,
> if (is_nd_pfn(dev)) {
> nd_pfn = to_nd_pfn(dev);
> rc = nvdimm_setup_pfn(nd_pfn, &pmem->pgmap);
> - if (rc)
> + if (rc) {
> + devm_namespace_disable(dev, ndns);
> return rc;
> + }
As mentioned in the previous mail, when rc != 0, disable is called again
here.
> }
>
> /* we're attaching a block device, disable raw namespace access */
> @@ -549,17 +551,15 @@ static int nd_pmem_probe(struct device *dev)
> ret = nd_pfn_probe(dev, ndns);
> if (ret == 0)
> return -ENXIO;
> - else if (ret == -EOPNOTSUPP)
> - return ret;
> -
> - ret = nd_dax_probe(dev, ndns);
> - if (ret == 0)
> - return -ENXIO;
> - else if (ret == -EOPNOTSUPP)
> - return ret;
> -
> + else if (ret != EOPNOTSUPP) {
^ -EOPNOTSUPP
Thanks,
Santosh
> + ret = nd_dax_probe(dev, ndns);
> + if (ret == 0)
> + return -ENXIO;
> + }
> /* probe complete, attach handles namespace enabling */
> devm_namespace_disable(dev, ndns);
> + if (ret == -EOPNOTSUPP)
> + return ret;
>
> return pmem_attach_disk(dev, ndns);
> }
> --
> 2.16.4
> _______________________________________________
> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libnvdimm: call devm_namespace_disable() on error
2020-07-09 5:39 ` Santosh Sivaraj
@ 2020-07-13 14:35 ` Hannes Reinecke
0 siblings, 0 replies; 4+ messages in thread
From: Hannes Reinecke @ 2020-07-13 14:35 UTC (permalink / raw)
To: Santosh Sivaraj, Dan Williams; +Cc: linux-nvdimm
On 7/9/20 7:39 AM, Santosh Sivaraj wrote:
> Hi Hannes,
>
> Hannes Reinecke <hare@suse.de> writes:
>
>> Once devm_namespace_enable() has been called the error path in the
>> calling function will not call devm_namespace_disable(), leaving the
>> namespace enabled on error.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>> drivers/dax/pmem/core.c | 2 +-
>> drivers/nvdimm/btt.c | 5 ++++-
>> drivers/nvdimm/claim.c | 8 +++++++-
>> drivers/nvdimm/pfn_devs.c | 1 +
>> drivers/nvdimm/pmem.c | 20 ++++++++++----------
>> 5 files changed, 23 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/dax/pmem/core.c b/drivers/dax/pmem/core.c
>> index 2bedf8414fff..4b26434f0aca 100644
>> --- a/drivers/dax/pmem/core.c
>> +++ b/drivers/dax/pmem/core.c
>> @@ -31,9 +31,9 @@ struct dev_dax *__dax_pmem_probe(struct device *dev, enum dev_dax_subsys subsys)
>> if (rc)
>> return ERR_PTR(rc);
>> rc = nvdimm_setup_pfn(nd_pfn, &pgmap);
>> + devm_namespace_disable(dev, ndns);
>> if (rc)
>> return ERR_PTR(rc);
>> - devm_namespace_disable(dev, ndns);
>>
>> /* reserve the metadata area, device-dax will reserve the data */
>> pfn_sb = nd_pfn->pfn_sb;
>> diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
>> index 48e9d169b6f9..bd4747f2c99b 100644
>> --- a/drivers/nvdimm/btt.c
>> +++ b/drivers/nvdimm/btt.c
>> @@ -1704,13 +1704,16 @@ int nvdimm_namespace_attach_btt(struct nd_namespace_common *ndns)
>> dev_dbg(&nd_btt->dev, "%s must be at least %ld bytes\n",
>> dev_name(&ndns->dev),
>> ARENA_MIN_SIZE + nd_btt->initial_offset);
>> + devm_namespace_disable(&nd_btt->dev, ndns);
>> return -ENXIO;
>> }
>> nd_region = to_nd_region(nd_btt->dev.parent);
>> btt = btt_init(nd_btt, rawsize, nd_btt->lbasize, nd_btt->uuid,
>> nd_region);
>> - if (!btt)
>> + if (!btt) {
>> + devm_namespace_disable(&nd_btt->dev, ndns);
>> return -ENOMEM;
>> + }
>> nd_btt->btt = btt;
>>
>> return 0;
>> diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c
>> index 45964acba944..15fd1b92d32f 100644
>> --- a/drivers/nvdimm/claim.c
>> +++ b/drivers/nvdimm/claim.c
>> @@ -314,12 +314,18 @@ int devm_nsio_enable(struct device *dev, struct nd_namespace_io *nsio,
>> }
>>
>> ndns->rw_bytes = nsio_rw_bytes;
>> - if (devm_init_badblocks(dev, &nsio->bb))
>> + if (devm_init_badblocks(dev, &nsio->bb)) {
>> + devm_release_mem_region(dev, res->start, size);
>> return -ENOMEM;
>> + }
>> nvdimm_badblocks_populate(to_nd_region(ndns->dev.parent), &nsio->bb,
>> &nsio->res);
>>
>> nsio->addr = devm_memremap(dev, res->start, size, ARCH_MEMREMAP_PMEM);
>> + if (IS_ERR(nsio->addr)) {
>> + devm_exit_badblocks(dev, &nsio->bb);
>> + devm_release_mem_region(dev, res->start, size);
>> + }
>>
>> return PTR_ERR_OR_ZERO(nsio->addr);
>> }
>> diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
>> index 34db557dbad1..9faa92662643 100644
>> --- a/drivers/nvdimm/pfn_devs.c
>> +++ b/drivers/nvdimm/pfn_devs.c
>> @@ -408,6 +408,7 @@ static int nd_pfn_clear_memmap_errors(struct nd_pfn *nd_pfn)
>> nsoff += chunk;
>> }
>> if (rc) {
>> + devm_namespace_disable(&nd_pfn->dev, ndns);
>
> The disable here seems to be wrong.
>
> This function called from the pmem_attach_disk path, where its expected the
> namespace is enabled after the setup_pfn. Also in case of an error, we do
> another disable in pmem_attach_disk.
Oh. Correct. Will be fixing it up.
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-13 14:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-03 11:18 [PATCH] libnvdimm: call devm_namespace_disable() on error Hannes Reinecke
2020-07-09 5:39 ` Santosh Sivaraj
2020-07-13 14:35 ` Hannes Reinecke
2020-07-09 5:57 ` Santosh Sivaraj
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.