nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [ndctl PATCHv2] ndctl: Use max_available_extent for namespaces
@ 2018-06-26 17:00 Keith Busch
  2018-06-26 17:07 ` Verma, Vishal L
  0 siblings, 1 reply; 2+ messages in thread
From: Keith Busch @ 2018-06-26 17:00 UTC (permalink / raw)
  To: linux-nvdimm, Dan Williams, Dave Jiang, Ross Zwisler, Vishal Verma
  Cc: Yasunori Goto

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);
+
+	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);
-- 
2.14.3

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [ndctl PATCHv2] ndctl: Use max_available_extent for namespaces
  2018-06-26 17:00 [ndctl PATCHv2] ndctl: Use max_available_extent for namespaces Keith Busch
@ 2018-06-26 17:07 ` Verma, Vishal L
  0 siblings, 0 replies; 2+ messages in thread
From: Verma, Vishal L @ 2018-06-26 17:07 UTC (permalink / raw)
  To: Williams, Dan J, ross.zwisler, Busch, Keith, linux-nvdimm, Jiang, Dave
  Cc: y-goto

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);
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-06-26 17:07 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-26 17:00 [ndctl PATCHv2] ndctl: Use max_available_extent for namespaces Keith Busch
2018-06-26 17:07 ` Verma, Vishal L

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).