* [PATCH] nvdimm/of_pmem: Provide a unique name for bus provider @ 2019-08-07 4:00 Aneesh Kumar K.V 2019-08-07 4:13 ` Dan Williams 2019-08-07 5:42 ` Vaibhav Jain 0 siblings, 2 replies; 7+ messages in thread From: Aneesh Kumar K.V @ 2019-08-07 4:00 UTC (permalink / raw) To: dan.j.williams Cc: Oliver O'Halloran, linuxppc-dev, Aneesh Kumar K.V, linux-nvdimm 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. 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); priv->bus_desc.module = THIS_MODULE; priv->bus_desc.of_node = np; -- 2.21.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] nvdimm/of_pmem: Provide a unique name for bus provider 2019-08-07 4:00 [PATCH] nvdimm/of_pmem: Provide a unique name for bus provider Aneesh Kumar K.V @ 2019-08-07 4:13 ` Dan Williams 2019-08-07 4:17 ` Aneesh Kumar K.V 2019-08-07 5:42 ` Vaibhav Jain 1 sibling, 1 reply; 7+ messages in thread From: Dan Williams @ 2019-08-07 4:13 UTC (permalink / raw) To: Aneesh Kumar K.V; +Cc: Oliver O'Halloran, linuxppc-dev, linux-nvdimm 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nvdimm/of_pmem: Provide a unique name for bus provider 2019-08-07 4:13 ` Dan Williams @ 2019-08-07 4:17 ` Aneesh Kumar K.V 2019-08-07 4:52 ` Dan Williams 0 siblings, 1 reply; 7+ messages in thread From: Aneesh Kumar K.V @ 2019-08-07 4:17 UTC (permalink / raw) To: Dan Williams; +Cc: Oliver O'Halloran, linuxppc-dev, linux-nvdimm 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; + } + list_add(&ctx->busses, &bus->list); rc = 0; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nvdimm/of_pmem: Provide a unique name for bus provider 2019-08-07 4:17 ` Aneesh Kumar K.V @ 2019-08-07 4:52 ` Dan Williams 2019-08-07 6:00 ` Aneesh Kumar K.V 0 siblings, 1 reply; 7+ messages in thread From: Dan Williams @ 2019-08-07 4:52 UTC (permalink / raw) To: Aneesh Kumar K.V; +Cc: Oliver O'Halloran, linuxppc-dev, linux-nvdimm 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; ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] nvdimm/of_pmem: Provide a unique name for bus provider 2019-08-07 4:52 ` Dan Williams @ 2019-08-07 6:00 ` Aneesh Kumar K.V 2019-08-07 15:34 ` Dan Williams 0 siblings, 1 reply; 7+ messages in thread From: Aneesh Kumar K.V @ 2019-08-07 6:00 UTC (permalink / raw) To: Dan Williams; +Cc: Oliver O'Halloran, linuxppc-dev, linux-nvdimm 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nvdimm/of_pmem: Provide a unique name for bus provider 2019-08-07 6:00 ` Aneesh Kumar K.V @ 2019-08-07 15:34 ` Dan Williams 0 siblings, 0 replies; 7+ messages in thread From: Dan Williams @ 2019-08-07 15:34 UTC (permalink / raw) To: Aneesh Kumar K.V; +Cc: Oliver O'Halloran, linuxppc-dev, linux-nvdimm On Tue, Aug 6, 2019 at 11:00 PM Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> wrote: > > 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. Great. I'll make a formal patch, and I'll amend the changelog of the proposed kernel change to say "older ndctl binaries mistakenly require" > > -aneesh ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nvdimm/of_pmem: Provide a unique name for bus provider 2019-08-07 4:00 [PATCH] nvdimm/of_pmem: Provide a unique name for bus provider Aneesh Kumar K.V 2019-08-07 4:13 ` Dan Williams @ 2019-08-07 5:42 ` Vaibhav Jain 1 sibling, 0 replies; 7+ messages in thread From: Vaibhav Jain @ 2019-08-07 5:42 UTC (permalink / raw) To: Aneesh Kumar K.V, dan.j.williams Cc: linuxppc-dev, linux-nvdimm, Oliver O'Halloran, Aneesh Kumar K.V "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes: > 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. > > 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); > priv->bus_desc.module = THIS_MODULE; > priv->bus_desc.of_node = np; > > -- > 2.21.0 > Tested-by: Vaibhav Jain <vaibhav@linux.ibm.com> -- Vaibhav Jain <vaibhav@linux.ibm.com> Linux Technology Center, IBM India Pvt. Ltd. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-08-07 15:40 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-07 4:00 [PATCH] nvdimm/of_pmem: Provide a unique name for bus provider Aneesh Kumar K.V 2019-08-07 4:13 ` Dan Williams 2019-08-07 4:17 ` Aneesh Kumar K.V 2019-08-07 4:52 ` Dan Williams 2019-08-07 6:00 ` Aneesh Kumar K.V 2019-08-07 15:34 ` Dan Williams 2019-08-07 5:42 ` Vaibhav Jain
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).