[PATCH 0/2] Namespace creation fixups

[PATCH 0/2] Namespace creation fixups

[Keith Busch]

This is a three-part fixup to the warning that occurs when the available
capacity is fragmented. When this occurs, the user may believe they can
create a larger namespace than is actually possible. This was resulting
in the following kernel warning:

  nd_region region0: allocation underrun: 0x0 of 0x1400000000 bytes
  WARNING: CPU: 32 PID: 1975 at drivers/nvdimm/namespace_devs.c:913 size_store+0x879/0x8d0 [libnvdimm]

The kernel side of this determines the maximum size by calculating the
largest contiguous extent that can be allocated. If the requested size
exceeds that, an error is returned early instead of reaching the
alarming kernel warning.

To make it possible for the user to know the maximum size it may
request, a new attribute is exported that shows the largest available
extent.

Finally, separate from this series, ndctl is updated to make use of this
new attribute when creating a namespace.

Keith Busch (2):
  libnvdimm: Use largest contiguous area for namespace size
  libnvdimm: Export max available extent

 drivers/nvdimm/dimm_devs.c      | 29 +++++++++++++++++++++++++++++
 drivers/nvdimm/namespace_devs.c |  2 +-
 drivers/nvdimm/nd-core.h        |  3 +++
 drivers/nvdimm/region_devs.c    | 39 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 72 insertions(+), 1 deletion(-)

-- 
2.14.3

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

[PATCH 1/2] libnvdimm: Use largest contiguous area for namespace size

[Keith Busch]

This patch will find the largest free extent to determine the max size
for a namespace that can be created.

Signed-off-by: Keith Busch <keith.busch@intel.com>
---
 drivers/nvdimm/dimm_devs.c      | 29 +++++++++++++++++++++++++++++
 drivers/nvdimm/namespace_devs.c |  2 +-
 drivers/nvdimm/nd-core.h        |  3 +++
 drivers/nvdimm/region_devs.c    | 23 +++++++++++++++++++++++
 4 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 8d348b22ba45..147613e0f872 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -536,6 +536,35 @@ resource_size_t nd_blk_available_dpa(struct nd_region *nd_region)
 	return info.available;
 }
 
+/**
+ * nd_pmem_largest_contiguous_available_dpa - For the given dimm+region,
+ * 	return the larges contiguous 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);
+	resource_size_t start, end, max = 0;
+	struct resource *res;
+
+	if (!ndd)
+		return 0;
+
+	start = nd_mapping->start;
+	end = start + nd_mapping->size;
+
+	for_each_dpa_resource(ndd, res) {
+		if ((res->start - start) > max)
+			max = res->start - start;
+		start = res->start + resource_size(res);
+	}
+	if (start < end && end - start > max)
+		max = end - start;
+	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 28afdd668905..c00d1d2d48f9 100644
--- a/drivers/nvdimm/namespace_devs.c
+++ b/drivers/nvdimm/namespace_devs.c
@@ -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_contiguous_max(nd_region);
 
 	if (val > available + allocated)
 		return -ENOSPC;
diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
index 79274ead54fb..8e4acd075b0f 100644
--- a/drivers/nvdimm/nd-core.h
+++ b/drivers/nvdimm/nd-core.h
@@ -100,6 +100,9 @@ struct nd_region;
 struct nvdimm_drvdata;
 struct nd_mapping;
 void nd_mapping_free_labels(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_contiguous_max(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..8af483d7ef57 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -357,6 +357,29 @@ static ssize_t set_cookie_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(set_cookie);
 
+resource_size_t nd_region_contiguous_max(struct nd_region *nd_region)
+{
+	resource_size_t available = 0;
+	int i;
+
+	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];
+		struct nvdimm_drvdata *ndd = to_ndd(nd_mapping);
+
+		/* if a dimm is disabled the available capacity is zero */
+		if (!ndd)
+			return 0;
+
+		if (is_memory(&nd_region->dev))
+			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);
+	}
+	return available;
+}
+
 resource_size_t nd_region_available_dpa(struct nd_region *nd_region)
 {
 	resource_size_t blk_max_overlap = 0, available, overlap;
-- 
2.14.3

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

[PATCH] ndctl: Use max_available_extent for creating namespaces

[Keith Busch]

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.

Signed-off-by: Keith Busch <keith.busch@intel.com>
---
 ndctl/lib/libndctl.c   | 30 ++++++++++++++++++++++++++++++
 ndctl/lib/libndctl.sym |  1 +
 ndctl/libndctl.h       |  2 ++
 ndctl/namespace.c      |  2 +-
 4 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 47e005e..a820fb3 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -2025,6 +2025,36 @@ 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;
+	}
+
+	if (sysfs_read_attr(ctx, path, buf) < 0)
+		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 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

[PATCH 2/2] libnvdimm: Export max available extent

[Keith Busch]

The 'available_size' attribute showing the combined total of all
unallocated space isn't always useful to know how large of a namespace
a user may be able to allocate if the region is fragmented. This patch
will export the largest extent of contiguous unallocated space that may
be allocated to create a new namespace.

Signed-off-by: Keith Busch <keith.busch@intel.com>
---
 drivers/nvdimm/region_devs.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index 8af483d7ef57..82aaf7d6488d 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -433,6 +433,21 @@ static ssize_t available_size_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(available_size);
 
+static ssize_t max_available_extent_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct nd_region *nd_region = to_nd_region(dev);
+	unsigned long long available = 0;
+
+	nvdimm_bus_lock(dev);
+	wait_nvdimm_bus_probe_idle(dev);
+	available = nd_region_contiguous_max(nd_region);
+	nvdimm_bus_unlock(dev);
+
+	return sprintf(buf, "%llu\n", available);
+}
+static DEVICE_ATTR_RO(max_available_extent);
+
 static ssize_t init_namespaces_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
@@ -584,6 +599,7 @@ static struct attribute *nd_region_attributes[] = {
 	&dev_attr_read_only.attr,
 	&dev_attr_set_cookie.attr,
 	&dev_attr_available_size.attr,
+	&dev_attr_max_available_extent.attr,
 	&dev_attr_namespace_seed.attr,
 	&dev_attr_init_namespaces.attr,
 	&dev_attr_badblocks.attr,
-- 
2.14.3

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

Re: [PATCH] ndctl: Use max_available_extent for creating namespaces

[Verma, Vishal L]

On Tue, 2018-06-26 at 09:37 -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.
> 
> Signed-off-by: Keith Busch <keith.busch@intel.com>
> ---
>  ndctl/lib/libndctl.c   | 30 ++++++++++++++++++++++++++++++
>  ndctl/lib/libndctl.sym |  1 +
>  ndctl/libndctl.h       |  2 ++
>  ndctl/namespace.c      |  2 +-
>  4 files changed, 34 insertions(+), 1 deletion(-)

Hi Keith,

The patch looks good, but just a couple of 'meta' comments.
1. We typically send ndctl patches separately from kernel patches (i.e. not
thraded together).
2. for ndctl patches, an 'ndctl PATCH' prefix is recommended. You can set a
repo local config parameter for doing this automatically on git format-
patch.
	git config format.subjectprefix "ndctl PATCH"

I'm thinking the kernel changes will be queued for 4.19, which means the
ndctl changes will go into v62.

Thanks,
	-Vishal

> 
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index 47e005e..a820fb3 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -2025,6 +2025,36 @@ 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;
> +	}
> +
> +	if (sysfs_read_attr(ctx, path, buf) < 0)
> +		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 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

Re: [PATCH] ndctl: Use max_available_extent for creating namespaces

[Verma, Vishal L]

On Tue, 2018-06-26 at 10:29 -0600, Keith Busch wrote:
> On Tue, Jun 26, 2018 at 09:19:28AM -0700, Verma, Vishal L wrote:
> > On Tue, 2018-06-26 at 09:37 -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.
> > > 
> > > Signed-off-by: Keith Busch <keith.busch@intel.com>
> > > ---
> > >  ndctl/lib/libndctl.c   | 30 ++++++++++++++++++++++++++++++
> > >  ndctl/lib/libndctl.sym |  1 +
> > >  ndctl/libndctl.h       |  2 ++
> > >  ndctl/namespace.c      |  2 +-
> > >  4 files changed, 34 insertions(+), 1 deletion(-)
> > 
> > Hi Keith,
> > 
> > The patch looks good, but just a couple of 'meta' comments.
> > 1. We typically send ndctl patches separately from kernel patches (i.e.
> > not
> > thraded together).
> > 2. for ndctl patches, an 'ndctl PATCH' prefix is recommended. You can
> > set a
> > repo local config parameter for doing this automatically on git format-
> > patch.
> > 	git config format.subjectprefix "ndctl PATCH"
> > 
> > I'm thinking the kernel changes will be queued for 4.19, which means
> > the
> > ndctl changes will go into v62.
> 
> Thanks for the info. I'll make those changes for next time.
> 
> I think I may need to send a v2 for this. Should we have this fall back
> to
> the available_size for the older kernels where the max_available_extents
> attribute is not provided? I actually had that in my repo and used a
> slightly older patch here, but I'm not sure if its okay to strongly
> couple an ndctl release to a kernel version.

I was thinking that too. Typically we don't guarantee ndctl to work with
old kernels, but this does seem like a bit of an invasive change.

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

Re: [PATCH] ndctl: Use max_available_extent for creating namespaces

[Keith Busch]

On Tue, Jun 26, 2018 at 09:19:28AM -0700, Verma, Vishal L wrote:
> On Tue, 2018-06-26 at 09:37 -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.
> > 
> > Signed-off-by: Keith Busch <keith.busch@intel.com>
> > ---
> >  ndctl/lib/libndctl.c   | 30 ++++++++++++++++++++++++++++++
> >  ndctl/lib/libndctl.sym |  1 +
> >  ndctl/libndctl.h       |  2 ++
> >  ndctl/namespace.c      |  2 +-
> >  4 files changed, 34 insertions(+), 1 deletion(-)
> 
> Hi Keith,
> 
> The patch looks good, but just a couple of 'meta' comments.
> 1. We typically send ndctl patches separately from kernel patches (i.e. not
> thraded together).
> 2. for ndctl patches, an 'ndctl PATCH' prefix is recommended. You can set a
> repo local config parameter for doing this automatically on git format-
> patch.
> 	git config format.subjectprefix "ndctl PATCH"
> 
> I'm thinking the kernel changes will be queued for 4.19, which means the
> ndctl changes will go into v62.

Thanks for the info. I'll make those changes for next time.

I think I may need to send a v2 for this. Should we have this fall back to
the available_size for the older kernels where the max_available_extents
attribute is not provided? I actually had that in my repo and used a
slightly older patch here, but I'm not sure if its okay to strongly
couple an ndctl release to a kernel version.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH] ndctl: Use max_available_extent for creating namespaces

[Dan Williams]

On Tue, Jun 26, 2018 at 9:27 AM, Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
> On Tue, 2018-06-26 at 10:29 -0600, Keith Busch wrote:
>> On Tue, Jun 26, 2018 at 09:19:28AM -0700, Verma, Vishal L wrote:
>> > On Tue, 2018-06-26 at 09:37 -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.
>> > >
>> > > Signed-off-by: Keith Busch <keith.busch@intel.com>
>> > > ---
>> > >  ndctl/lib/libndctl.c   | 30 ++++++++++++++++++++++++++++++
>> > >  ndctl/lib/libndctl.sym |  1 +
>> > >  ndctl/libndctl.h       |  2 ++
>> > >  ndctl/namespace.c      |  2 +-
>> > >  4 files changed, 34 insertions(+), 1 deletion(-)
>> >
>> > Hi Keith,
>> >
>> > The patch looks good, but just a couple of 'meta' comments.
>> > 1. We typically send ndctl patches separately from kernel patches (i.e.
>> > not
>> > thraded together).
>> > 2. for ndctl patches, an 'ndctl PATCH' prefix is recommended. You can
>> > set a
>> > repo local config parameter for doing this automatically on git format-
>> > patch.
>> >     git config format.subjectprefix "ndctl PATCH"
>> >
>> > I'm thinking the kernel changes will be queued for 4.19, which means
>> > the
>> > ndctl changes will go into v62.
>>
>> Thanks for the info. I'll make those changes for next time.
>>
>> I think I may need to send a v2 for this. Should we have this fall back
>> to
>> the available_size for the older kernels where the max_available_extents
>> attribute is not provided? I actually had that in my repo and used a
>> slightly older patch here, but I'm not sure if its okay to strongly
>> couple an ndctl release to a kernel version.
>
> I was thinking that too. Typically we don't guarantee ndctl to work with
> old kernels, but this does seem like a bit of an invasive change.
>
> Dan, thoughts?

It should fall back if the attribute is not available. Our *tests*
aren't guaranteed to pass on older kernels, but ndctl proper should
try it's best to accommodate old kernels.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH] ndctl: Use max_available_extent for creating namespaces

[Verma, Vishal L]

On Tue, 2018-06-26 at 09:32 -0700, Dan Williams wrote:
> On Tue, Jun 26, 2018 at 9:27 AM, Verma, Vishal L
> <vishal.l.verma@intel.com> wrote:
> > On Tue, 2018-06-26 at 10:29 -0600, Keith Busch wrote:
> > > On Tue, Jun 26, 2018 at 09:19:28AM -0700, Verma, Vishal L wrote:
> > > > On Tue, 2018-06-26 at 09:37 -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.
> > > > > 
> > > > > Signed-off-by: Keith Busch <keith.busch@intel.com>
> > > > > ---
> > > > >  ndctl/lib/libndctl.c   | 30 ++++++++++++++++++++++++++++++
> > > > >  ndctl/lib/libndctl.sym |  1 +
> > > > >  ndctl/libndctl.h       |  2 ++
> > > > >  ndctl/namespace.c      |  2 +-
> > > > >  4 files changed, 34 insertions(+), 1 deletion(-)
> > > > 
> > > > Hi Keith,
> > > > 
> > > > The patch looks good, but just a couple of 'meta' comments.
> > > > 1. We typically send ndctl patches separately from kernel patches
> > > > (i.e.
> > > > not
> > > > thraded together).
> > > > 2. for ndctl patches, an 'ndctl PATCH' prefix is recommended. You
> > > > can
> > > > set a
> > > > repo local config parameter for doing this automatically on git
> > > > format-
> > > > patch.
> > > >     git config format.subjectprefix "ndctl PATCH"
> > > > 
> > > > I'm thinking the kernel changes will be queued for 4.19, which
> > > > means
> > > > the
> > > > ndctl changes will go into v62.
> > > 
> > > Thanks for the info. I'll make those changes for next time.
> > > 
> > > I think I may need to send a v2 for this. Should we have this fall
> > > back
> > > to
> > > the available_size for the older kernels where the
> > > max_available_extents
> > > attribute is not provided? I actually had that in my repo and used a
> > > slightly older patch here, but I'm not sure if its okay to strongly
> > > couple an ndctl release to a kernel version.
> > 
> > I was thinking that too. Typically we don't guarantee ndctl to work
> > with
> > old kernels, but this does seem like a bit of an invasive change.
> > 
> > Dan, thoughts?
> 
> It should fall back if the attribute is not available. Our *tests*
> aren't guaranteed to pass on older kernels, but ndctl proper should
> try it's best to accommodate old kernels.

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

Re: [PATCH 1/2] libnvdimm: Use largest contiguous area for namespace size

[Dan Williams]

On Tue, Jun 26, 2018 at 8:37 AM, Keith Busch <keith.busch@intel.com> wrote:
> This patch will find the largest free extent to determine the max size
> for a namespace that can be created.
>

This also fixes the "underrun" WARN_ON, right? Can you resend with
that detail in the changelog and add the Fixes tags with a cc: stable?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH 1/2] libnvdimm: Use largest contiguous area for namespace size

[Keith Busch]

On Thu, Jul 05, 2018 at 10:57:56AM -0700, Dan Williams wrote:
> On Tue, Jun 26, 2018 at 8:37 AM, Keith Busch <keith.busch@intel.com> wrote:
> > This patch will find the largest free extent to determine the max size
> > for a namespace that can be created.
> >
> 
> This also fixes the "underrun" WARN_ON, right? Can you resend with
> that detail in the changelog and add the Fixes tags with a cc: stable?

Sure thing, will resend shortly.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm