[v2] ndctl: add clear error support for ndctl

Commit Message

Dave Jiang May 1, 2017, 9:02 p.m. UTC

On Mon, May 01, 2017 at 01:25:45PM -0700, Kani, Toshimitsu wrote:
> On Thu, 2017-04-27 at 15:38 -0700, Dave Jiang wrote:
> > Adding ndctl support that will allow clearing of bad blocks for a
> > device. Initial implementation will only support device dax devices.
> > The ndctl takes a device path and parameters of the starting bad
> > block, and the number of bad blocks to clear.
> >
> > Signed-off-by: Dave Jiang <[e-mail]>
> 
> Hi Dave,
> 
> I am seeing a failure to open "badblocks" on my system since its sysfs
> path with "/sys/devcies/platform/ACPI.NFIT" is not valid (not sure how
> I got "ACPI.NFIT" here - I hope I did not mess up my build with test
> cases).
> 
> As you can see below, other open paths use
> "/sys/devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0012:00/" for "nbus0".  If
> you go from "/sys/devices/platform", a valid path seems to be:
> "/sys/devices/platform/ACPI0012:00/firmware_node"
> 
> # strace ndctl clear-error -f /dev/dax0.0 -s 1048576 -l 1
> execve("/usr/bin/ndctl", ["ndctl", "clear-error", "-f", "/dev/dax0.0",
> "-s", "1048576", "-l", "1"], [/* 60 vars */]) = 0
>  :
> open("/sys/devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0012:00/ndbus0/region0/d
> ax0.0/resource", O_RDONLY|O_CLOEXEC) = 4
> read(4, "0x250200000\n", 1024)          = 12
> close(4)                                = 0
> open("/sys/devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0012:00/ndbus0/region0/d
> ax0.0/size", O_RDONLY|O_CLOEXEC) = 4
> read(4, "16909336576\n", 1024)          = 12
> close(4)                                = 0
> getdents(3, /* 0 entries */, 32768)     = 0
> close(3)                                = 0
> open("/sys/devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0012:00/ndbus0/region0/r
> esource", O_RDONLY|O_CLOEXEC) = 3
> read(3, "0x240000000\n", 1024)          = 12
> close(3)                                = 0
> open("/sys/devices/platform/ACPI.NFIT/ndbus0/region0/badblocks",
> O_RDONLY) = -1 ENOENT (No such file or directory)
> write(2, "libndctl: regions_badblocks_init"..., 34libndctl:
> regions_badblocks_init: ) = 34
> write(2, "region badblocks fopen failed\n", 30region badblocks fopen
> failed
> ) = 30
> exit_group(234)                         = ?
> +++ exited with 234 +++
> 
> Thanks,
> -Toshi

Toshi,
Can you please try this and see if that fixes the issue? Thanks!

Comments

Kani, Toshi May 1, 2017, 9:12 p.m. UTC | #1

On Mon, 2017-05-01 at 14:02 -0700, Jiang, Dave wrote:
> On Mon, May 01, 2017 at 01:25:45PM -0700, Kani, Toshimitsu wrote:

> > On Thu, 2017-04-27 at 15:38 -0700, Dave Jiang wrote:

 :
> 

> Toshi,

> Can you please try this and see if that fixes the issue? Thanks!

> 

> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c

> index 7399729..44fc2f3 100644

> --- a/ndctl/lib/libndctl.c

> +++ b/ndctl/lib/libndctl.c

> @@ -1882,10 +1882,8 @@ static int regions_badblocks_init(struct

> ndctl_region *region)

>  		region->badblocks = NULL;

>  	}

>  

> -	if (asprintf(&bb_path,

> "/sys/devices/platform/%s/%s/%s/badblocks",

> -				ndctl_bus_get_provider(bus),

> -				ndctl_bus_get_devname(bus),

> -				ndctl_region_get_devname(region)) <

> 0) {

> +	if (asprintf(&bb_path, "%s/badblocks",

> +				region->region_path) < 0) {

>  		rc = -errno;

>  		err(ctx, "region badblocks path allocation

> failure\n");

>  		return rc;


Yes, this fixes the issue. :-)  Noticed that there is a warning.

  CC       libndctl.lo
libndctl.c: In function 'regions_badblocks_init':
libndctl.c:1875:20: warning: unused variable 'bus' [-Wunused-variable]
  struct ndctl_bus *bus = ndctl_region_get_bus(region);
                    ^
Thanks!!
-Toshi

Dave Jiang May 1, 2017, 9:20 p.m. UTC | #2

On 05/01/2017 02:12 PM, Kani, Toshimitsu wrote:
> On Mon, 2017-05-01 at 14:02 -0700, Jiang, Dave wrote:
>> On Mon, May 01, 2017 at 01:25:45PM -0700, Kani, Toshimitsu wrote:
>>> On Thu, 2017-04-27 at 15:38 -0700, Dave Jiang wrote:
>  :
>>
>> Toshi,
>> Can you please try this and see if that fixes the issue? Thanks!
>>
>> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
>> index 7399729..44fc2f3 100644
>> --- a/ndctl/lib/libndctl.c
>> +++ b/ndctl/lib/libndctl.c
>> @@ -1882,10 +1882,8 @@ static int regions_badblocks_init(struct
>> ndctl_region *region)
>>  region->badblocks = NULL;
>>  }
>>
>> -if (asprintf(&bb_path,
>> "/sys/devices/platform/%s/%s/%s/badblocks",
>> -ndctl_bus_get_provider(bus),
>> -ndctl_bus_get_devname(bus),
>> -ndctl_region_get_devname(region)) <
>> 0) {
>> +if (asprintf(&bb_path, "%s/badblocks",
>> +region->region_path) < 0) {
>>  rc = -errno;
>>  err(ctx, "region badblocks path allocation
>> failure\n");
>>  return rc;
> 
> Yes, this fixes the issue. :-)  Noticed that there is a warning.
> 
>   CC       libndctl.lo
> libndctl.c: In function 'regions_badblocks_init':
> libndctl.c:1875:20: warning: unused variable 'bus' [-Wunused-variable]
>   struct ndctl_bus *bus = ndctl_region_get_bus(region);

Thanks! I'll spin v3 with the fixes.

Patch

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 7399729..44fc2f3 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -1882,10 +1882,8 @@  static int regions_badblocks_init(struct ndctl_region *region)
 		region->badblocks = NULL;
 	}
 
-	if (asprintf(&bb_path, "/sys/devices/platform/%s/%s/%s/badblocks",
-				ndctl_bus_get_provider(bus),
-				ndctl_bus_get_devname(bus),
-				ndctl_region_get_devname(region)) < 0) {
+	if (asprintf(&bb_path, "%s/badblocks",
+				region->region_path) < 0) {
 		rc = -errno;
 		err(ctx, "region badblocks path allocation failure\n");
 		return rc;