On Tue, 2018-07-24 at 15:07 -0600, Keith Busch wrote:
> This patch will find the max contiguous area to determine the largest
> pmem namespace size that can be created. If the requested size exceeds
> the largest available, ENOSPC error will be returned.
>
> This fixes the allocation underrun error and wrong error return code
> that have otherwise been observed as the following kernel warning:
>
> WARNING: CPU: <CPU> PID: <PID> at drivers/nvdimm/namespace_devs.c:913 size_store
>
> Fixes: a1f3e4d6a0c3 ("libnvdimm, region: update nd_region_available_dpa() for multi-pmem support")
> Cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Keith Busch <keith.busch@xxxxxxxxx>
> ---
> v3 -> v4:
>
> Actually constrain the reserved pmem to the region under consideration
> rather than the mapping's dimm. This is done by directly calling
> __reserve_free_pmem with the region's device instead of walking the
> parent devices children. Thanks to Vishal Verma for reporting how
> to trigger the incorrect reportings.
>
> Fixed a possible NULL deref, from Gustavo A. R. Silva.
Looks good to me. Feel free to add:
Reviewed-by: Vishal Verma <vishal.l.verma@xxxxxxxxx>
>
> drivers/nvdimm/dimm_devs.c | 31 +++++++++++++++++++++++++++++++
> drivers/nvdimm/namespace_devs.c | 6 +++---
> drivers/nvdimm/nd-core.h | 8 ++++++++
> drivers/nvdimm/region_devs.c | 24 ++++++++++++++++++++++++
> 4 files changed, 66 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
> index 8d348b22ba45..863cabc35215 100644
> --- a/drivers/nvdimm/dimm_devs.c
> +++ b/drivers/nvdimm/dimm_devs.c
> @@ -536,6 +536,37 @@ resource_size_t nd_blk_available_dpa(struct nd_region *nd_region)
> return info.available;
> }
>
> +/**
> + * nd_pmem_max_contiguous_dpa - For the given dimm+region, return the max
> + * contiguous unallocated dpa range.
> + * @nd_region: constrain available space check to this reference region
> + * @nd_mapping: container of dpa-resource-root + labels
> + */
> +resource_size_t nd_pmem_max_contiguous_dpa(struct nd_region *nd_region,
> + struct nd_mapping *nd_mapping)
> +{
> + struct nvdimm_drvdata *ndd = to_ndd(nd_mapping);
> + struct nvdimm_bus *nvdimm_bus;
> + resource_size_t max = 0;
> + struct resource *res;
> +
> + /* if a dimm is disabled the available capacity is zero */
> + if (!ndd)
> + return 0;
> +
> + nvdimm_bus = walk_to_nvdimm_bus(ndd->dev);
> + if (__reserve_free_pmem(&nd_region->dev, nd_mapping->nvdimm))
> + return 0;
> + for_each_dpa_resource(ndd, res) {
> + if (strcmp(res->name, "pmem-reserve") != 0)
> + continue;
> + if (resource_size(res) > max)
> + max = resource_size(res);
> + }
> + release_free_pmem(nvdimm_bus, nd_mapping);
> + return max;
> +}
> +
> /**
> * nd_pmem_available_dpa - for the given dimm+region account unallocated dpa
> * @nd_mapping: container of dpa-resource-root + labels
> diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
> index cb322f2bc605..4a4266250c28 100644
> --- a/drivers/nvdimm/namespace_devs.c
> +++ b/drivers/nvdimm/namespace_devs.c
> @@ -799,7 +799,7 @@ static int merge_dpa(struct nd_region *nd_region,
> return 0;
> }
>
> -static int __reserve_free_pmem(struct device *dev, void *data)
> +int __reserve_free_pmem(struct device *dev, void *data)
> {
> struct nvdimm *nvdimm = data;
> struct nd_region *nd_region;
> @@ -836,7 +836,7 @@ static int __reserve_free_pmem(struct device *dev, void *data)
> return 0;
> }
>
> -static void release_free_pmem(struct nvdimm_bus *nvdimm_bus,
> +void release_free_pmem(struct nvdimm_bus *nvdimm_bus,
> struct nd_mapping *nd_mapping)
> {
> struct nvdimm_drvdata *ndd = to_ndd(nd_mapping);
> @@ -1032,7 +1032,7 @@ static ssize_t __size_store(struct device *dev, unsigned long long val)
>
> allocated += nvdimm_allocated_dpa(ndd, &label_id);
> }
> - available = nd_region_available_dpa(nd_region);
> + available = nd_region_allocatable_dpa(nd_region);
>
> if (val > available + allocated)
> return -ENOSPC;
> diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
> index 79274ead54fb..ac68072fb8cd 100644
> --- a/drivers/nvdimm/nd-core.h
> +++ b/drivers/nvdimm/nd-core.h
> @@ -100,6 +100,14 @@ struct nd_region;
> struct nvdimm_drvdata;
> struct nd_mapping;
> void nd_mapping_free_labels(struct nd_mapping *nd_mapping);
> +
> +int __reserve_free_pmem(struct device *dev, void *data);
> +void release_free_pmem(struct nvdimm_bus *nvdimm_bus,
> + struct nd_mapping *nd_mapping);
> +
> +resource_size_t nd_pmem_max_contiguous_dpa(struct nd_region *nd_region,
> + struct nd_mapping *nd_mapping);
> +resource_size_t nd_region_allocatable_dpa(struct nd_region *nd_region);
> resource_size_t nd_pmem_available_dpa(struct nd_region *nd_region,
> struct nd_mapping *nd_mapping, resource_size_t *overlap);
> resource_size_t nd_blk_available_dpa(struct nd_region *nd_region);
> diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
> index ec3543b83330..c30d5af02cc2 100644
> --- a/drivers/nvdimm/region_devs.c
> +++ b/drivers/nvdimm/region_devs.c
> @@ -389,6 +389,30 @@ resource_size_t nd_region_available_dpa(struct nd_region *nd_region)
> return available;
> }
>
> +resource_size_t nd_region_allocatable_dpa(struct nd_region *nd_region)
> +{
> + resource_size_t available = 0;
> + int i;
> +
> + if (is_memory(&nd_region->dev))
> + available = PHYS_ADDR_MAX;
> +
> + WARN_ON(!is_nvdimm_bus_locked(&nd_region->dev));
> + for (i = 0; i < nd_region->ndr_mappings; i++) {
> + struct nd_mapping *nd_mapping = &nd_region->mapping[i];
> +
> + if (is_memory(&nd_region->dev))
> + available = min(available,
> + nd_pmem_max_contiguous_dpa(nd_region,
> + nd_mapping));
> + else if (is_nd_blk(&nd_region->dev))
> + available += nd_blk_available_dpa(nd_region);
> + }
> + if (is_memory(&nd_region->dev))
> + return available * nd_region->ndr_mappings;
> + return available;
> +}
> +
> static ssize_t available_size_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
