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

["Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>]

On 8/7/19 10:22 AM, Dan Williams wrote:
> On Tue, Aug 6, 2019 at 9:17 PM Aneesh Kumar K.V
> <aneesh.kumar@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.kumar@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.kumar@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;
> 

That worked.

-aneesh