Re: [PATCH ndctl v1 0/8] daxctl: Add device align and range mapping allocation

[Joao Martins <joao.m.martins@oracle.com>]

On 12/16/20 7:13 PM, Dan Williams wrote:
> On Wed, Dec 16, 2020 at 3:41 AM Joao Martins <joao.m.martins@oracle.com> wrote:
>>
>> On 7/16/20 7:46 PM, Joao Martins wrote:
>>> Hey,
>>>
>>> This series builds on top of this one[0] and does the following improvements
>>> to the Soft-Reserved subdivision:
>>>
>>>  1) Support for {create,reconfigure}-device for selecting @align (hugepage size).
>>>  Here we add a '-a|--align 4K|2M|1G' option to the existing commands;
>>>
>>>  2) Listing improvements for device alignment and mappings;
>>>  Note: Perhaps it is better to hide the mappings by default, and only
>>>        print with -v|--verbose. This would align with ndctl, as the mappings
>>>        info can be quite large.
>>>
>>>  3) Allow creating devices from selecting ranges. This allows to keep the
>>>    same GPA->HPA mapping as before we kexec the hypervisor with running guests:
>>>
>>>    daxctl list -d dax0.1 > /var/log/dax0.1.json
>>>    kexec -d -l bzImage
>>>    systemctl kexec
>>>    daxctl create -u --restore /var/log/dax0.1.json
>>>
>>>    The JSON was what I though it would be easier for an user, given that it is
>>>    the data format daxctl outputs. Alternatives could be adding multiple:
>>>       --mapping <pgoff>:<start>-<end>
>>>
>>>    But that could end up in a gigantic line and a little more
>>>    unmanageable I think.
>>>
>>> This series requires this series[0] on top of Dan's patches[1]:
>>>
>>>  [0] https://lore.kernel.org/linux-nvdimm/20200716172913.19658-1-joao.m.martins@oracle.com/
>>>  [1] https://lore.kernel.org/linux-nvdimm/159457116473.754248.7879464730875147365.stgit@dwillia2-desk3.amr.corp.intel.com/
>>>
>>> The only TODO here is docs and improving tests to validate mappings, and test
>>> the restore path.
>>>
>>> Suggestions/comments are welcome.
>>>
>> There's a couple of issues in this series regarding daxctl-reconfigure options and
>> breakage of ndctl with kernels (<5.10) that do not supply a device @align upon testing
>> with NVDIMMs. Plus it is missing daxctl-create.sh unit test for @align.
> 
> What's the breakage with older kernels, is it the kernel regressing
> old daxctl, or is it new daxctl being incompatible with old kernels?
> If it's the latter, it needs a fixup, if it's the former it needs a
> kernel compat change.

It's the latter i.e. new daxctl being incompatible with old kernels, because of a change
in the first patch. Essentially a wrong assumption of device align being always available
in add_dax_dev().

The fixup would be this snip below to the first patch. But I will respin the first four
patches today or my morning tomorrow, with a test.

diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index 14bf48dd00bf..b01cc916eb6e 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -498,10 +498,12 @@ static void *add_dax_dev(void *parent, int id, const char *daxdev_base)
                goto err_read;
        dev->size = strtoull(buf, NULL, 0);

+       /* Device align attribute is only available in v5.10 or up */
        sprintf(path, "%s/align", daxdev_base);
-       if (sysfs_read_attr(ctx, path, buf) < 0)
-               goto err_read;
-       dev->align = strtoull(buf, NULL, 0);
+       if (!sysfs_read_attr(ctx, path, buf))
+               dev->align = strtoull(buf, NULL, 0);
+       else
+               dev->align = 0;

        dev->dev_path = strdup(daxdev_base);
        if (!dev->dev_path)
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org