Re: [ndctl PATCHv4] ndctl: Use max_available_extent for namespace

["Verma, Vishal L" <vishal.l.verma@intel.com>]

On Thu, 2018-08-02 at 15:49 -0400, Masayoshi Mizuma wrote:
> Hi Keith,
> 
> I tested your patch with the following test script.
> The result is good to me. 
> 
> Please feel free to add:
> 
>    Tested-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
> 
> Test script:
> ---
> #!/bin/bash -Ex
> 
> NDCTL=./ndctl/ndctl
> 
> modprobe nfit_test
> region=$($NDCTL list -b nfit_test.0 -R -t pmem | jq -r .[].dev | head
> -1)
> 
> available_sz=$($NDCTL list -r $region | jq -r .[].available_size)
> size=$(( available_sz/4 ))
> 
> NS1=$($NDCTL create-namespace -r $region -t pmem -s $size | jq -r
> .dev)
> NS2=$($NDCTL create-namespace -r $region -t pmem -s $size | jq -r
> .dev)
> NS3=$($NDCTL create-namespace -r $region -t pmem -s $size | jq -r
> .dev)
> 
> $NDCTL disable-namespace $NS2
> $NDCTL destroy-namespace $NS2
> 
> $NDCTL list -r $region
> 
> $NDCTL create-namespace -r $region -t pmem 
> ret=$?
> 
> $NDCTL disable-region all
> modprobe -r nfit_test
> 
> exit $ret

Hi Masa,

Thanks for the testing, I will add the Tested-by.

If you also wanted to package up this script as a unit test, I'd be
happy to include it. Would be gated on KVER=4.19

Thanks,
	-Vishal

> ---
> 
> Thanks,
> Masa
> 
> On 07/24/2018 06:31 PM, Keith Busch wrote:
> > The available_size attribute returns all the unused regions, but a
> > memory namespace has to use contiguous free regions. This patch
> > uses the
> > max_available_extent attribute, which reports the largest capacity
> > that
> > can be created when determining what size to allocate.
> > 
> > If the attribute is available, this patch will also report the max
> > available extent in the region list output.
> > 
> > Signed-off-by: Keith Busch <keith.busch@intel.com>
> > ---
> > v3 -> v4:
> > 
> >   Moved the symbol to the appropriate version section of the
> > symbols
> >   script.
> > 
> >   Report the max available extent in the region list if the
> > attribute
> >   exists.
> > 
> >  ndctl/lib/libndctl.c   | 33 +++++++++++++++++++++++++++++++++
> >  ndctl/lib/libndctl.sym |  1 +
> >  ndctl/libndctl.h       |  2 ++
> >  ndctl/list.c           |  9 +++++++++
> >  ndctl/namespace.c      |  4 +++-
> >  5 files changed, 48 insertions(+), 1 deletion(-)
> > 
> > diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> > index 969e4aa..e911ea2 100644
> > --- a/ndctl/lib/libndctl.c
> > +++ b/ndctl/lib/libndctl.c
> > @@ -2107,6 +2107,39 @@ NDCTL_EXPORT unsigned long long
> > ndctl_region_get_available_size(
> >  	return strtoull(buf, NULL, 0);
> >  }
> >  
> > +NDCTL_EXPORT unsigned long long
> > ndctl_region_get_max_available_extent(
> > +		struct ndctl_region *region)
> > +{
> > +	unsigned int nstype = ndctl_region_get_nstype(region);
> > +	struct ndctl_ctx *ctx = ndctl_region_get_ctx(region);
> > +	char *path = region->region_buf;
> > +	int len = region->buf_len;
> > +	char buf[SYSFS_ATTR_SIZE];
> > +
> > +	switch (nstype) {
> > +	case ND_DEVICE_NAMESPACE_PMEM:
> > +	case ND_DEVICE_NAMESPACE_BLK:
> > +		break;
> > +	default:
> > +		return 0;
> > +	}
> > +
> > +	if (snprintf(path, len,
> > +		     "%s/max_available_extent", region-
> > >region_path) >= len) {
> > +		err(ctx, "%s: buffer too small!\n",
> > +				ndctl_region_get_devname(region));
> > +		return ULLONG_MAX;
> > +	}
> > +
> > +	/* fall back to legacy behavior if max extents is not
> > exported */
> > +	if (sysfs_read_attr(ctx, path, buf) < 0) {
> > +		dbg(ctx, "max extents attribute not exported on
> > older kernels\n");
> > +		return ULLONG_MAX;
> > +	}
> > +
> > +	return strtoull(buf, NULL, 0);
> > +}
> > +
> >  NDCTL_EXPORT unsigned int ndctl_region_get_range_index(struct
> > ndctl_region *region)
> >  {
> >  	return region->range_index;
> > diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
> > index 9b36960..2bc312d 100644
> > --- a/ndctl/lib/libndctl.sym
> > +++ b/ndctl/lib/libndctl.sym
> > @@ -375,4 +375,5 @@ global:
> >  	ndctl_dimm_get_flags;
> >  	ndctl_dimm_get_event_flags;
> >  	ndctl_dimm_is_flag_supported;
> > +	ndctl_region_get_max_available_extent;
> >  } LIBNDCTL_16;
> > diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
> > index 6a6bb0d..a146502 100644
> > --- a/ndctl/libndctl.h
> > +++ b/ndctl/libndctl.h
> > @@ -350,6 +350,8 @@ unsigned int
> > ndctl_region_get_interleave_ways(struct ndctl_region *region);
> >  unsigned int ndctl_region_get_mappings(struct ndctl_region
> > *region);
> >  unsigned long long ndctl_region_get_size(struct ndctl_region
> > *region);
> >  unsigned long long ndctl_region_get_available_size(struct
> > ndctl_region *region);
> > +unsigned long long ndctl_region_get_max_available_extent(
> > +		struct ndctl_region *region);
> >  unsigned int ndctl_region_get_range_index(struct ndctl_region
> > *region);
> >  unsigned int ndctl_region_get_type(struct ndctl_region *region);
> >  struct ndctl_namespace *ndctl_region_get_namespace_seed(
> > diff --git a/ndctl/list.c b/ndctl/list.c
> > index a06edc9..82e88bb 100644
> > --- a/ndctl/list.c
> > +++ b/ndctl/list.c
> > @@ -72,6 +72,7 @@ static struct json_object *region_to_json(struct
> > ndctl_region *region,
> >  	struct ndctl_interleave_set *iset;
> >  	struct ndctl_mapping *mapping;
> >  	unsigned int bb_count = 0;
> > +	unsigned long long extent;
> >  	enum ndctl_persistence_domain pd;
> >  	int numa;
> >  
> > @@ -94,6 +95,14 @@ static struct json_object *region_to_json(struct
> > ndctl_region *region,
> >  		goto err;
> >  	json_object_object_add(jregion, "available_size", jobj);
> >  
> > +	extent = ndctl_region_get_max_available_extent(region);
> > +	if (extent != ULLONG_MAX) {
> > +		jobj = util_json_object_size(extent, flags);
> > +		if (!jobj)
> > +			goto err;
> > +		json_object_object_add(jregion,
> > "max_available_extent", jobj);
> > +	}
> > +
> >  	switch (ndctl_region_get_type(region)) {
> >  	case ND_DEVICE_REGION_PMEM:
> >  		jobj = json_object_new_string("pmem");
> > diff --git a/ndctl/namespace.c b/ndctl/namespace.c
> > index fe86d82..cfe0559 100644
> > --- a/ndctl/namespace.c
> > +++ b/ndctl/namespace.c
> > @@ -764,7 +764,9 @@ static int namespace_create(struct ndctl_region
> > *region)
> >  		return -EAGAIN;
> >  	}
> >  
> > -	available = ndctl_region_get_available_size(region);
> > +	available = ndctl_region_get_max_available_extent(region);
> > +	if (available == ULLONG_MAX)
> > +		available =
> > ndctl_region_get_available_size(region);
> >  	if (!available || p.size > available) {
> >  		debug("%s: insufficient capacity size: %llx avail:
> > %llx\n",
> >  			devname, p.size, available);
> > 
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm