Hi Hannes,

Hannes Reinecke <[email protected]> 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 <[email protected]>
> ---
>  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 -- [email protected]
> To unsubscribe send an email to [email protected]
_______________________________________________
Linux-nvdimm mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to