2:52 a.m.
Seed namespaces are included in "ndctl disable-namespace all". However
since the user never "creates" them it is surprising to see
"disable-namespace" report 1 more namespace relative to the number that
have been created. Catch attempts to disable a zero-sized namespace:
Before:
{
"dev":"namespace1.0",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1"
}
{
"dev":"namespace1.1",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.1"
}
{
"dev":"namespace1.2",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.2"
}
disabled 4 namespaces
After:
{
"dev":"namespace1.0",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1"
}
{
"dev":"namespace1.3",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.3"
}
{
"dev":"namespace1.1",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.1"
}
disabled 3 namespaces
Signed-off-by: Redhairer Li <redhairer.li(a)intel.com>
---
ndctl/lib/libndctl.c | 11 ++++++++---
ndctl/region.c | 4 +++-
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index ee737cb..49f362b 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -4231,6 +4231,7 @@ NDCTL_EXPORT int ndctl_namespace_disable_safe(struct ndctl_namespace
*ndns)
const char *bdev = NULL;
char path[50];
int fd;
+ unsigned long long size = ndctl_namespace_get_size(ndns);
if (pfn && ndctl_pfn_is_enabled(pfn))
bdev = ndctl_pfn_get_block_device(pfn);
@@ -4260,9 +4261,13 @@ NDCTL_EXPORT int ndctl_namespace_disable_safe(struct
ndctl_namespace *ndns)
devname, bdev, strerror(errno));
return -errno;
}
- } else
- ndctl_namespace_disable_invalidate(ndns);
-
+ } else {
+ if (size == 0)
+ /* Don't try to disable idle namespace (no capacity allocated) */
+ return -ENXIO;
+ else
+ ndctl_namespace_disable_invalidate(ndns);
+ }
return 0;
}
diff --git a/ndctl/region.c b/ndctl/region.c
index 7945007..0014bb9 100644
--- a/ndctl/region.c
+++ b/ndctl/region.c
@@ -72,6 +72,7 @@ static int region_action(struct ndctl_region *region, enum device_action
mode)
{
struct ndctl_namespace *ndns;
int rc = 0;
+ unsigned long long size;
switch (mode) {
case ACTION_ENABLE:
@@ -80,7 +81,8 @@ static int region_action(struct ndctl_region *region, enum device_action
mode)
case ACTION_DISABLE:
ndctl_namespace_foreach(region, ndns) {
rc = ndctl_namespace_disable_safe(ndns);
- if (rc)
+ size = ndctl_namespace_get_size(ndns);
+ if (rc && size != 0)
return rc;
}
rc = ndctl_region_disable_invalidate(region);
--
2.20.1.windows.1
1:31 p.m.
New subject: [PATCH 1/1] ndctl/namespace: Fix disable-namespace accounting
relative to seed devices
On Sun, Apr 26, 2020 at 2:53 AM Redhairer Li <redhairer.li(a)intel.com> wrote:
Seed namespaces are included in "ndctl disable-namespace all". However
since the user never "creates" them it is surprising to see
"disable-namespace" report 1 more namespace relative to the number that
have been created. Catch attempts to disable a zero-sized namespace:
Before:
{
"dev":"namespace1.0",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1"
}
{
"dev":"namespace1.1",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.1"
}
{
"dev":"namespace1.2",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.2"
}
disabled 4 namespaces
After:
{
"dev":"namespace1.0",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1"
}
{
"dev":"namespace1.3",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.3"
}
{
"dev":"namespace1.1",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.1"
}
disabled 3 namespaces
Signed-off-by: Redhairer Li <redhairer.li(a)intel.com>
---
ndctl/lib/libndctl.c | 11 ++++++++---
ndctl/region.c | 4 +++-
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index ee737cb..49f362b 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -4231,6 +4231,7 @@ NDCTL_EXPORT int ndctl_namespace_disable_safe(struct
ndctl_namespace *ndns)
const char *bdev = NULL;
char path[50];
int fd;
+ unsigned long long size = ndctl_namespace_get_size(ndns);
if (pfn && ndctl_pfn_is_enabled(pfn))
bdev = ndctl_pfn_get_block_device(pfn);
@@ -4260,9 +4261,13 @@ NDCTL_EXPORT int ndctl_namespace_disable_safe(struct
ndctl_namespace *ndns)
devname, bdev, strerror(errno));
return -errno;
}
- } else
- ndctl_namespace_disable_invalidate(ndns);
-
+ } else {
+ if (size == 0)
+ /* Don't try to disable idle namespace (no capacity
allocated) */
+ return -ENXIO;
+ else
+ ndctl_namespace_disable_invalidate(ndns);
+ }
Maybe make this return 0 in the case when size is zero since by
definition the namespace must already be disabled if it has zero size.
} else if (size)
ndctl_namespace_disable_invalidate(ndns);
return 0;
diff --git a/ndctl/region.c b/ndctl/region.c
index 7945007..0014bb9 100644
--- a/ndctl/region.c
+++ b/ndctl/region.c
@@ -72,6 +72,7 @@ static int region_action(struct ndctl_region *region, enum
device_action mode)
{
struct ndctl_namespace *ndns;
int rc = 0;
+ unsigned long long size;
switch (mode) {
case ACTION_ENABLE:
@@ -80,7 +81,8 @@ static int region_action(struct ndctl_region *region, enum
device_action mode)
case ACTION_DISABLE:
ndctl_namespace_foreach(region, ndns) {
rc = ndctl_namespace_disable_safe(ndns);
- if (rc)
+ size = ndctl_namespace_get_size(ndns);
+ if (rc && size != 0)
return rc;
...then you wouldn't need to have this special case here since
ndctl_namespace_disable_safe() will not fail.
6:09 p.m.
New subject: [PATCH 1/1] ndctl/namespace: Fix disable-namespace accounting
relative to seed devices
If make this return 0 in the case when size is zero, then seed device will be counted in
due to
case ACTION_DISABLE:
rc = ndctl_namespace_disable_safe(ndns);
if (rc == 0)
(*processed)++;
break;
I ever think make it return 1.
It also can return correct number which not count in seed device.
But there will be no error msg when I disable seed device.
It will make user confuse.
Eg.
root@ubuntu-red:~/git/ndctl# ndctl/ndctl disable-namespace namespce0.0
disabled 0 namespace
So I follow enable-namespace to return -ENXIO and it will look like
root@ubuntu-red:~/git/ndctl# ndctl/ndctl disable-namespace namespce0.0
error disabling namespaces: No such device or address
disabled 0 namespaces
But no matter return -ENXIO or 1, it will make test/create.sh fail. This is why I modify
region.c
+ modprobe nfit_test
+ ../ndctl/ndctl disable-region -b nfit_test.0 all
disabled 0 regions
+ ../ndctl/ndctl zero-labels -b nfit_test.0 all
nmem1: regions active, abort label write
nmem3: regions active, abort label write
nmem0: regions active, abort label write
nmem2: regions active, abort label write
zeroed 0 nmem
++ err 28
+++ basename ./create.sh
++ echo test/create.sh: failed at line 28
-----Original Message-----
From: Dan Williams <dan.j.williams(a)intel.com>
Sent: Tuesday, April 28, 2020 4:31 AM
To: Li, Redhairer <redhairer.li(a)intel.com>
Cc: linux-nvdimm <linux-nvdimm(a)lists.01.org>
Subject: Re: [PATCH 1/1] ndctl/namespace: Fix disable-namespace accounting relative to
seed devices
On Sun, Apr 26, 2020 at 2:53 AM Redhairer Li <redhairer.li(a)intel.com> wrote:
Seed namespaces are included in "ndctl disable-namespace all". However
since the user never "creates" them it is surprising to see
"disable-namespace" report 1 more namespace relative to the number
that have been created. Catch attempts to disable a zero-sized namespace:
Before:
{
"dev":"namespace1.0",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1"
}
{
"dev":"namespace1.1",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.1"
}
{
"dev":"namespace1.2",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.2"
}
disabled 4 namespaces
After:
{
"dev":"namespace1.0",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1"
}
{
"dev":"namespace1.3",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.3"
}
{
"dev":"namespace1.1",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.1"
}
disabled 3 namespaces
Signed-off-by: Redhairer Li <redhairer.li(a)intel.com>
---
ndctl/lib/libndctl.c | 11 ++++++++---
ndctl/region.c | 4 +++-
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c index
ee737cb..49f362b 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -4231,6 +4231,7 @@ NDCTL_EXPORT int ndctl_namespace_disable_safe(struct
ndctl_namespace *ndns)
const char *bdev = NULL;
char path[50];
int fd;
+ unsigned long long size = ndctl_namespace_get_size(ndns);
if (pfn && ndctl_pfn_is_enabled(pfn))
bdev = ndctl_pfn_get_block_device(pfn); @@ -4260,9
+4261,13 @@ NDCTL_EXPORT int ndctl_namespace_disable_safe(struct ndctl_namespace *ndns)
devname, bdev, strerror(errno));
return -errno;
}
- } else
- ndctl_namespace_disable_invalidate(ndns);
-
+ } else {
+ if (size == 0)
+ /* Don't try to disable idle namespace (no capacity
allocated) */
+ return -ENXIO;
+ else
+ ndctl_namespace_disable_invalidate(ndns);
+ }
Maybe make this return 0 in the case when size is zero since by definition the namespace
must already be disabled if it has zero size.
} else if (size)
ndctl_namespace_disable_invalidate(ndns);
return 0;
diff --git a/ndctl/region.c b/ndctl/region.c index 7945007..0014bb9
100644
--- a/ndctl/region.c
+++ b/ndctl/region.c
@@ -72,6 +72,7 @@ static int region_action(struct ndctl_region
*region, enum device_action mode) {
struct ndctl_namespace *ndns;
int rc = 0;
+ unsigned long long size;
switch (mode) {
case ACTION_ENABLE:
@@ -80,7 +81,8 @@ static int region_action(struct ndctl_region *region, enum
device_action mode)
case ACTION_DISABLE:
ndctl_namespace_foreach(region, ndns) {
rc = ndctl_namespace_disable_safe(ndns);
- if (rc)
+ size = ndctl_namespace_get_size(ndns);
+ if (rc && size != 0)
return rc;
...then you wouldn't need to have this special case here since
ndctl_namespace_disable_safe() will not fail.
11:07 p.m.
New subject: [PATCH 1/1] ndctl/namespace: Fix disable-namespace accounting
relative to seed devices
Hi Redhairer, sorry for the long delay circling back to this...
On Mon, Apr 27, 2020 at 6:10 PM Li, Redhairer <redhairer.li(a)intel.com> wrote:
If make this return 0 in the case when size is zero, then seed device will be counted in
due to
case ACTION_DISABLE:
rc = ndctl_namespace_disable_safe(ndns);
if (rc == 0)
(*processed)++;
break;
I ever think make it return 1.
I think returning 1 is the right answer, because this isn't a failure
and there are several other places that call
ndctl_namespace_disable_safe() that need to be updated to treat rc < 0
as failure and rc >= 0 as success / no disable necessary.
ndctl/namespace.c:1127: rc = ndctl_namespace_disable_safe(ndns);
ndctl/namespace.c-1128- if (rc)
--
ndctl/namespace.c:1433: rc = ndctl_namespace_disable_safe(ndns);
ndctl/namespace.c-1434- if (rc) {
--
ndctl/namespace.c:1457: rc = ndctl_namespace_disable_safe(ndns);
ndctl/namespace.c-1458- if (rc) {
--
ndctl/namespace.c:1480: rc = ndctl_namespace_disable_safe(ndns);
ndctl/namespace.c-1481- if (rc) {
--
ndctl/namespace.c:2215: rc =
ndctl_namespace_disable_safe(ndns);
ndctl/namespace.c-2216- if (rc == 0)
--
ndctl/region.c:72: rc = ndctl_namespace_disable_safe(ndns);
ndctl/region.c-73- if (rc)
It also can return correct number which not count in seed device.
But there will be no error msg when I disable seed device.
It will make user confuse.
Eg.
root@ubuntu-red:~/git/ndctl# ndctl/ndctl disable-namespace namespce0.0
disabled 0 namespace
Why is that confusing isn't the namespace already disabled?
So I follow enable-namespace to return -ENXIO and it will look like
root@ubuntu-red:~/git/ndctl# ndctl/ndctl disable-namespace namespce0.0
error disabling namespaces: No such device or address
disabled 0 namespaces
But no matter return -ENXIO or 1, it will make test/create.sh fail. This is why I modify
region.c
region.c and several other places need to be updated. I would split
this into 2 patches.
The first patch updates all callers of ndctl_disable_namespace_safe()
to treat rc > 0 as success.
Then the second patch can treat cmd_disable_namespace() to not count
rc > 0 as error, but also don't count it as disabled.
6:50 a.m.
From: Redhairer Li <redhairer.li(a)intel.com>
Seed namespaces are included in "ndctl disable-namespace all". However
since the user never "creates" them it is surprising to see
"disable-namespace" report 1 more namespace relative to the number that
have been created. Catch attempts to disable a zero-sized namespace:
Before:
{
"dev":"namespace1.0",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1"
}
{
"dev":"namespace1.1",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.1"
}
{
"dev":"namespace1.2",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.2"
}
disabled 4 namespaces
After:
{
"dev":"namespace1.0",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1"
}
{
"dev":"namespace1.3",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.3"
}
{
"dev":"namespace1.1",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.1"
}
disabled 3 namespaces
Signed-off-by: Redhairer Li <redhairer.li(a)intel.com>
---
ndctl/lib/libndctl.c | 11 ++++++++---
ndctl/namespace.c | 8 ++++----
ndctl/region.c | 2 +-
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 5546963..b477098 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -4614,6 +4614,7 @@ NDCTL_EXPORT int ndctl_namespace_disable_safe(struct ndctl_namespace
*ndns)
const char *bdev = NULL;
char path[50];
int fd;
+ unsigned long long size = ndctl_namespace_get_size(ndns);
if (pfn && ndctl_pfn_is_enabled(pfn))
bdev = ndctl_pfn_get_block_device(pfn);
@@ -4643,9 +4644,13 @@ NDCTL_EXPORT int ndctl_namespace_disable_safe(struct
ndctl_namespace *ndns)
devname, bdev, strerror(errno));
return -errno;
}
- } else
- ndctl_namespace_disable_invalidate(ndns);
-
+ } else {
+ if (size == 0)
+ /* No disable necessary due to no capacity allocated */
+ return 1;
+ else
+ ndctl_namespace_disable_invalidate(ndns);
+ }
return 0;
}
diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index e734248..77a75b4 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -1110,7 +1110,7 @@ static int namespace_destroy(struct ndctl_region *region,
return -EBUSY;
} else {
rc = ndctl_namespace_disable_safe(ndns);
- if (rc)
+ if (rc < 0)
return rc;
}
@@ -1395,7 +1395,7 @@ static int dax_clear_badblocks(struct ndctl_dax *dax)
return -ENXIO;
rc = ndctl_namespace_disable_safe(ndns);
- if (rc) {
+ if (rc < 0) {
error("%s: unable to disable namespace: %s\n", devname,
strerror(-rc));
return rc;
@@ -1419,7 +1419,7 @@ static int pfn_clear_badblocks(struct ndctl_pfn *pfn)
return -ENXIO;
rc = ndctl_namespace_disable_safe(ndns);
- if (rc) {
+ if (rc < 0) {
error("%s: unable to disable namespace: %s\n", devname,
strerror(-rc));
return rc;
@@ -1442,7 +1442,7 @@ static int raw_clear_badblocks(struct ndctl_namespace *ndns)
return -ENXIO;
rc = ndctl_namespace_disable_safe(ndns);
- if (rc) {
+ if (rc < 0) {
error("%s: unable to disable namespace: %s\n", devname,
strerror(-rc));
return rc;
diff --git a/ndctl/region.c b/ndctl/region.c
index 7945007..df64fc9 100644
--- a/ndctl/region.c
+++ b/ndctl/region.c
@@ -80,7 +80,7 @@ static int region_action(struct ndctl_region *region, enum device_action
mode)
case ACTION_DISABLE:
ndctl_namespace_foreach(region, ndns) {
rc = ndctl_namespace_disable_safe(ndns);
- if (rc)
+ if (rc < 0)
return rc;
}
rc = ndctl_region_disable_invalidate(region);
--
2.27.0.windows.1
9:22 a.m.
New subject: [PATCH 1/1] ndctl/namespace: Fix disable-namespace accounting
relative to seed devices
Hi Dan,
Your comment is make sense.
ndctl_namespace_disable_safe will return 1 if namespace size is 0.
I send a new patch out for review.
But I am not sure what do you mean for 2nd patch.
In cmd_disable_namespace, it already print error if rc<0.
rc = do_xaction_namespace(namespace, ACTION_DISABLE, ctx, &disabled);
if (rc < 0 && !err_count)
fprintf(stderr, "error disabling namespaces: %s\n",
strerror(-rc));
my patch is based on v70 due to latest one will see "FAIL: create.sh" when make
check even not include my change.
-----Original Message-----
From: Dan Williams <dan.j.williams(a)intel.com>
Sent: Wednesday, January 13, 2021 3:08 PM
To: Li, Redhairer <redhairer.li(a)intel.com>
Cc: linux-nvdimm <linux-nvdimm(a)lists.01.org>
Subject: Re: [PATCH 1/1] ndctl/namespace: Fix disable-namespace accounting relative to
seed devices
Hi Redhairer, sorry for the long delay circling back to this...
On Mon, Apr 27, 2020 at 6:10 PM Li, Redhairer <redhairer.li(a)intel.com> wrote:
If make this return 0 in the case when size is zero, then seed device
will be counted in due to case ACTION_DISABLE:
rc = ndctl_namespace_disable_safe(ndns);
if (rc == 0)
(*processed)++;
break;
I ever think make it return 1.
I think returning 1 is the right answer, because this isn't a failure and there are
several other places that call
ndctl_namespace_disable_safe() that need to be updated to treat rc < 0 as failure and
rc >= 0 as success / no disable necessary.
ndctl/namespace.c:1127: rc = ndctl_namespace_disable_safe(ndns);
ndctl/namespace.c-1128- if (rc)
--
ndctl/namespace.c:1433: rc = ndctl_namespace_disable_safe(ndns);
ndctl/namespace.c-1434- if (rc) {
--
ndctl/namespace.c:1457: rc = ndctl_namespace_disable_safe(ndns);
ndctl/namespace.c-1458- if (rc) {
--
ndctl/namespace.c:1480: rc = ndctl_namespace_disable_safe(ndns);
ndctl/namespace.c-1481- if (rc) {
--
ndctl/namespace.c:2215: rc =
ndctl_namespace_disable_safe(ndns);
ndctl/namespace.c-2216- if (rc == 0)
--
ndctl/region.c:72: rc = ndctl_namespace_disable_safe(ndns);
ndctl/region.c-73- if (rc)
It also can return correct number which not count in seed device.
But there will be no error msg when I disable seed device.
It will make user confuse.
Eg.
root@ubuntu-red:~/git/ndctl# ndctl/ndctl disable-namespace namespce0.0
disabled 0 namespace
Why is that confusing isn't the namespace already disabled?
So I follow enable-namespace to return -ENXIO and it will look like
root@ubuntu-red:~/git/ndctl# ndctl/ndctl disable-namespace namespce0.0
error disabling namespaces: No such device or address disabled 0
namespaces
But no matter return -ENXIO or 1, it will make test/create.sh fail.
This is why I modify region.c
region.c and several other places need to be updated. I would split this into 2 patches.
The first patch updates all callers of ndctl_disable_namespace_safe() to treat rc > 0
as success.
Then the second patch can treat cmd_disable_namespace() to not count rc > 0 as error,
but also don't count it as disabled.
12:47 a.m.
New subject: [PATCH 1/1] ndctl/namespace: Fix disable-namespace accounting
relative to seed devices
On Fri, Jan 15, 2021 at 9:22 AM Li, Redhairer <redhairer.li(a)intel.com> wrote:
Hi Dan,
Your comment is make sense.
ndctl_namespace_disable_safe will return 1 if namespace size is 0.
I send a new patch out for review.
It looks ok but it does not apply to the current tip of tree now v71.1
can you resend?
But I am not sure what do you mean for 2nd patch.
In cmd_disable_namespace, it already print error if rc<0.
rc = do_xaction_namespace(namespace, ACTION_DISABLE, ctx, &disabled);
if (rc < 0 && !err_count)
fprintf(stderr, "error disabling namespaces: %s\n",
strerror(-rc));
Hmm, you're right, once you change to the positive error code the
report will just work. Did you give it a try does it fix the
accounting problem with just your first patch?
my patch is based on v70 due to latest one will see "FAIL: create.sh" when make
check even not include my change.
I know of at least one create.sh failure that was fixed by:
Kernel commit:
2dd2a1740ee1 libnvdimm/namespace: Fix reaping of invalidated
block-window-namespace labels
...which is now in v5.11-rc1 and backported to v5.10-rc4. However,
that bug only started triggering after ndctl changed to reconfigure
namespaces in place with commit:
d4bc247faeda ndctl/namespace: Reconfigure in-place
..which was only merged into ndctl in v71.
Another kernel change that may be causing your failures is:
d1c5246e08eb x86/mm: Fix leak of pmd ptlock
...which was merged for v5.11-rc3 and backported to v5.10.7.
Can you run latest kernel and ndctl and see if you still see the
create.sh failure?
6:03 a.m.
From: Redhairer Li <redhairer.li(a)intel.com>
Seed namespaces are included in "ndctl disable-namespace all". However
since the user never "creates" them it is surprising to see
"disable-namespace" report 1 more namespace relative to the number that
have been created. Catch attempts to disable a zero-sized namespace:
Before:
{
"dev":"namespace1.0",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1"
}
{
"dev":"namespace1.1",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.1"
}
{
"dev":"namespace1.2",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.2"
}
disabled 4 namespaces
After:
{
"dev":"namespace1.0",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1"
}
{
"dev":"namespace1.3",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.3"
}
{
"dev":"namespace1.1",
"size":"492.00 MiB (515.90 MB)",
"blockdev":"pmem1.1"
}
disabled 3 namespaces
Signed-off-by: Redhairer Li <redhairer.li(a)intel.com>
---
ndctl/lib/libndctl.c | 10 ++++++++--
ndctl/namespace.c | 8 ++++----
ndctl/region.c | 2 +-
3 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 36fb6fe..2f6d806 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -4602,6 +4602,7 @@ NDCTL_EXPORT int ndctl_namespace_disable_safe(struct ndctl_namespace
*ndns)
const char *bdev = NULL;
char path[50];
int fd;
+ unsigned long long size = ndctl_namespace_get_size(ndns);
if (pfn && ndctl_pfn_is_enabled(pfn))
bdev = ndctl_pfn_get_block_device(pfn);
@@ -4631,8 +4632,13 @@ NDCTL_EXPORT int ndctl_namespace_disable_safe(struct
ndctl_namespace *ndns)
devname, bdev, strerror(errno));
return -errno;
}
- } else
- ndctl_namespace_disable_invalidate(ndns);
+ } else {
+ if (size == 0)
+ /* No disable necessary due to no capacity allocated */
+ return 1;
+ else
+ ndctl_namespace_disable_invalidate(ndns);
+ }
return 0;
}
diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index 0c8df9f..1feb74d 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -1125,7 +1125,7 @@ static int namespace_prep_reconfig(struct ndctl_region *region,
}
rc = ndctl_namespace_disable_safe(ndns);
- if (rc)
+ if (rc < 0)
return rc;
ndctl_namespace_set_enforce_mode(ndns, NDCTL_NS_MODE_RAW);
@@ -1431,7 +1431,7 @@ static int dax_clear_badblocks(struct ndctl_dax *dax)
return -ENXIO;
rc = ndctl_namespace_disable_safe(ndns);
- if (rc) {
+ if (rc < 0) {
error("%s: unable to disable namespace: %s\n", devname,
strerror(-rc));
return rc;
@@ -1455,7 +1455,7 @@ static int pfn_clear_badblocks(struct ndctl_pfn *pfn)
return -ENXIO;
rc = ndctl_namespace_disable_safe(ndns);
- if (rc) {
+ if (rc < 0) {
error("%s: unable to disable namespace: %s\n", devname,
strerror(-rc));
return rc;
@@ -1478,7 +1478,7 @@ static int raw_clear_badblocks(struct ndctl_namespace *ndns)
return -ENXIO;
rc = ndctl_namespace_disable_safe(ndns);
- if (rc) {
+ if (rc < 0) {
error("%s: unable to disable namespace: %s\n", devname,
strerror(-rc));
return rc;
diff --git a/ndctl/region.c b/ndctl/region.c
index 3edb9b3..4552c4a 100644
--- a/ndctl/region.c
+++ b/ndctl/region.c
@@ -70,7 +70,7 @@ static int region_action(struct ndctl_region *region, enum device_action
mode)
case ACTION_DISABLE:
ndctl_namespace_foreach(region, ndns) {
rc = ndctl_namespace_disable_safe(ndns);
- if (rc)
+ if (rc < 0)
return rc;
}
rc = ndctl_region_disable_invalidate(region);
--
2.27.0.windows.1
8:16 a.m.
New subject: [PATCH 1/1] ndctl/namespace: Fix disable-namespace accounting
relative to seed devices
Resend it base on v71.1.
-----Original Message-----
From: Dan Williams <dan.j.williams(a)intel.com>
Sent: Thursday, January 28, 2021 4:47 PM
To: Li, Redhairer <redhairer.li(a)intel.com>
Cc: linux-nvdimm <linux-nvdimm(a)lists.01.org>
Subject: Re: [PATCH 1/1] ndctl/namespace: Fix disable-namespace accounting relative to
seed devices
On Fri, Jan 15, 2021 at 9:22 AM Li, Redhairer <redhairer.li(a)intel.com> wrote:
Hi Dan,
Your comment is make sense.
ndctl_namespace_disable_safe will return 1 if namespace size is 0.
I send a new patch out for review.
It looks ok but it does not apply to the current tip of tree now v71.1 can you resend?
But I am not sure what do you mean for 2nd patch.
In cmd_disable_namespace, it already print error if rc<0.
rc = do_xaction_namespace(namespace, ACTION_DISABLE, ctx, &disabled);
if (rc < 0 && !err_count)
fprintf(stderr, "error disabling namespaces: %s\n",
strerror(-rc));
Hmm, you're right, once you change to the positive error code the report will just
work. Did you give it a try does it fix the accounting problem with just your first
patch?
my patch is based on v70 due to latest one will see "FAIL: create.sh" when make
check even not include my change.
I know of at least one create.sh failure that was fixed by:
Kernel commit:
2dd2a1740ee1 libnvdimm/namespace: Fix reaping of invalidated block-window-namespace
labels
...which is now in v5.11-rc1 and backported to v5.10-rc4. However, that bug only started
triggering after ndctl changed to reconfigure namespaces in place with commit:
d4bc247faeda ndctl/namespace: Reconfigure in-place
..which was only merged into ndctl in v71.
Another kernel change that may be causing your failures is:
d1c5246e08eb x86/mm: Fix leak of pmd ptlock
...which was merged for v5.11-rc3 and backported to v5.10.7.
Can you run latest kernel and ndctl and see if you still see the create.sh failure?
12:44 a.m.
New subject: [PATCH 1/1] ndctl/namespace: Fix disable-namespace accounting
relative to seed devices
After update kernel to 5.10-rc4, all tests will PASS with "make check"
-----Original Message-----
From: Li, Redhairer
Sent: Friday, January 29, 2021 12:16 AM
To: Dan Williams <dan.j.williams(a)intel.com>
Cc: linux-nvdimm <linux-nvdimm(a)lists.01.org>
Subject: RE: [PATCH 1/1] ndctl/namespace: Fix disable-namespace accounting relative to
seed devices
Resend it base on v71.1.
-----Original Message-----
From: Dan Williams <dan.j.williams(a)intel.com>
Sent: Thursday, January 28, 2021 4:47 PM
To: Li, Redhairer <redhairer.li(a)intel.com>
Cc: linux-nvdimm <linux-nvdimm(a)lists.01.org>
Subject: Re: [PATCH 1/1] ndctl/namespace: Fix disable-namespace accounting relative to
seed devices
On Fri, Jan 15, 2021 at 9:22 AM Li, Redhairer <redhairer.li(a)intel.com> wrote:
Hi Dan,
Your comment is make sense.
ndctl_namespace_disable_safe will return 1 if namespace size is 0.
I send a new patch out for review.
It looks ok but it does not apply to the current tip of tree now v71.1 can you resend?
But I am not sure what do you mean for 2nd patch.
In cmd_disable_namespace, it already print error if rc<0.
rc = do_xaction_namespace(namespace, ACTION_DISABLE, ctx, &disabled);
if (rc < 0 && !err_count)
fprintf(stderr, "error disabling namespaces: %s\n",
strerror(-rc));
Hmm, you're right, once you change to the positive error code the report will just
work. Did you give it a try does it fix the accounting problem with just your first
patch?
my patch is based on v70 due to latest one will see "FAIL: create.sh" when make
check even not include my change.
I know of at least one create.sh failure that was fixed by:
Kernel commit:
2dd2a1740ee1 libnvdimm/namespace: Fix reaping of invalidated block-window-namespace
labels
...which is now in v5.11-rc1 and backported to v5.10-rc4. However, that bug only started
triggering after ndctl changed to reconfigure namespaces in place with commit:
d4bc247faeda ndctl/namespace: Reconfigure in-place
..which was only merged into ndctl in v71.
Another kernel change that may be causing your failures is:
d1c5246e08eb x86/mm: Fix leak of pmd ptlock
...which was merged for v5.11-rc3 and backported to v5.10.7.
Can you run latest kernel and ndctl and see if you still see the create.sh failure?
481
days inactive
759
days old
9 comments
4 participants
participants (4)
-
Dan Williams -
Li, Redhairer -
redhairer
-
Redhairer Li