Re: [PATCH] nvdimm/of_pmem: Provide a unique name for bus provider

[Dan Williams]

On Tue, Aug 6, 2019 at 9:17 PM Aneesh Kumar K.V
<aneesh.ku...@linux.ibm.com> wrote:
>
> On 8/7/19 9:43 AM, Dan Williams wrote:
> > On Tue, Aug 6, 2019 at 9:00 PM Aneesh Kumar K.V
> > <aneesh.ku...@linux.ibm.com> wrote:
> >>
> >> ndctl utility requires the ndbus to have unique names. If not while
> >> enumerating the bus in userspace it drops bus with similar names.
> >> This results in us not listing devices beneath the bus.
> >
> > It does?
> >
> >>
> >> Signed-off-by: Aneesh Kumar K.V <aneesh.ku...@linux.ibm.com>
> >> ---
> >>   drivers/nvdimm/of_pmem.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/nvdimm/of_pmem.c b/drivers/nvdimm/of_pmem.c
> >> index a0c8dcfa0bf9..97187d6c0bdb 100644
> >> --- a/drivers/nvdimm/of_pmem.c
> >> +++ b/drivers/nvdimm/of_pmem.c
> >> @@ -42,7 +42,7 @@ static int of_pmem_region_probe(struct platform_device 
> >> *pdev)
> >>                  return -ENOMEM;
> >>
> >>          priv->bus_desc.attr_groups = bus_attr_groups;
> >> -       priv->bus_desc.provider_name = "of_pmem";
> >> +       priv->bus_desc.provider_name = kstrdup(pdev->name, GFP_KERNEL);
> >
> > This looks ok to me to address support for older ndctl binaries, but
> > I'd like to also fix the ndctl bug that makes non-unique provider
> > names fail.
> >
>
> 0462269ab121d323a016874ebdd42217f2911ee7 (ndctl: provide a method to
> invalidate the bus list)
>
> This hunk does the filtering.
>
> @@ -928,6 +929,14 @@ static int add_bus(void *parent, int id, const char
> *ctl_base)
>                 goto err_read;
>         bus->buf_len = strlen(bus->bus_path) + 50;
>
> +       ndctl_bus_foreach(ctx, bus_dup)
> +               if (strcmp(ndctl_bus_get_provider(bus_dup),
> +                                       ndctl_bus_get_provider(bus)) == 0) {
> +                       free_bus(bus, NULL);
> +                       free(path);
> +                       return 1;
> +               }
> +

Yup, that's broken, does this incremental fix work?

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 4d9cc7e29c6b..6596f94edef8 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -889,7 +889,9 @@ static void *add_bus(void *parent, int id, const
char *ctl_base)

        ndctl_bus_foreach(ctx, bus_dup)
                if (strcmp(ndctl_bus_get_provider(bus_dup),
-                                       ndctl_bus_get_provider(bus)) == 0) {
+                                       ndctl_bus_get_provider(bus)) == 0
+                               && strcmp(ndctl_bus_get_devname(bus_dup),
+                                       ndctl_bus_get_devname(bus)) == 0) {
                        free_bus(bus, NULL);
                        free(path);
                        return bus_dup;