diff mbox

[ndctl,PATCHv2] ndctl: Use max_available_extent for namespaces

Message ID 20180626170027.6942-1-keith.busch@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Keith Busch June 26, 2018, 5 p.m. UTC
The available_size attribute returns all the unused regions, but a
namespace has to use contiguous free regions. This patch uses the
attribute returning the largest capacity that can be created for
determining if the namespace can be created.

If this is used on a kernel that predates the new attribute, ndctl will
fall back to the previous behavior using available_size. While that may
still fail, it is more likely to succeed than returning a hard-coded
value. 

Signed-off-by: Keith Busch <keith.busch@intel.com>
---
v1 -> v2:

  Appropriate subject-prefex for ndctl.

  Added the fallback to the previous behavior if the attribute is not
  found, and noted this behavior in the changelog.

 ndctl/lib/libndctl.c   | 31 +++++++++++++++++++++++++++++++
 ndctl/lib/libndctl.sym |  1 +
 ndctl/libndctl.h       |  2 ++
 ndctl/namespace.c      |  2 +-
 4 files changed, 35 insertions(+), 1 deletion(-)

Comments

Verma, Vishal L June 26, 2018, 5:07 p.m. UTC | #1
On Tue, 2018-06-26 at 11:00 -0600, Keith Busch wrote:
> The available_size attribute returns all the unused regions, but a
> namespace has to use contiguous free regions. This patch uses the
> attribute returning the largest capacity that can be created for
> determining if the namespace can be created.
> 
> If this is used on a kernel that predates the new attribute, ndctl will
> fall back to the previous behavior using available_size. While that may
> still fail, it is more likely to succeed than returning a hard-coded
> value. 
> 
> Signed-off-by: Keith Busch <keith.busch@intel.com>
> ---
> v1 -> v2:
> 
>   Appropriate subject-prefex for ndctl.
> 
>   Added the fallback to the previous behavior if the attribute is not
>   found, and noted this behavior in the changelog.
> 
>  ndctl/lib/libndctl.c   | 31 +++++++++++++++++++++++++++++++
>  ndctl/lib/libndctl.sym |  1 +
>  ndctl/libndctl.h       |  2 ++
>  ndctl/namespace.c      |  2 +-
>  4 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index 47e005e..8ff1fa6 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -2025,6 +2025,37 @@ 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)
> +		return ndctl_region_get_available_size(region);

Instead of silently falling back to the legacy behavior, perhaps we should
print a dbg() warning here so that the user can be prompted to update their
kernel.

> +
> +	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 c1228e5..22fd026 100644
> --- a/ndctl/lib/libndctl.sym
> +++ b/ndctl/lib/libndctl.sym
> @@ -123,6 +123,7 @@ global:
>  	ndctl_region_get_mappings;
>  	ndctl_region_get_size;
>  	ndctl_region_get_available_size;
> +	ndctl_region_get_max_available_extent;
>  	ndctl_region_get_type;
>  	ndctl_region_get_namespace_seed;
>  	ndctl_region_get_btt_seed;
> diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
> index be997ac..624115d 100644
> --- a/ndctl/libndctl.h
> +++ b/ndctl/libndctl.h
> @@ -338,6 +338,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/namespace.c b/ndctl/namespace.c
> index fe86d82..4a562a2 100644
> --- a/ndctl/namespace.c
> +++ b/ndctl/namespace.c
> @@ -764,7 +764,7 @@ 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 || p.size > available) {
>  		debug("%s: insufficient capacity size: %llx avail:
> %llx\n",
>  			devname, p.size, available);
diff mbox

Patch

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 47e005e..8ff1fa6 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -2025,6 +2025,37 @@  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)
+		return ndctl_region_get_available_size(region);
+
+	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 c1228e5..22fd026 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -123,6 +123,7 @@  global:
 	ndctl_region_get_mappings;
 	ndctl_region_get_size;
 	ndctl_region_get_available_size;
+	ndctl_region_get_max_available_extent;
 	ndctl_region_get_type;
 	ndctl_region_get_namespace_seed;
 	ndctl_region_get_btt_seed;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index be997ac..624115d 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -338,6 +338,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/namespace.c b/ndctl/namespace.c
index fe86d82..4a562a2 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -764,7 +764,7 @@  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 || p.size > available) {
 		debug("%s: insufficient capacity size: %llx avail: %llx\n",
 			devname, p.size, available);