| Message ID | 1539657458-24401-5-git-send-email-frowand.list@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | of: overlay: validation checks, subsequent fixes | expand |
On Mon, Oct 15, 2018 at 07:37:24PM -0700, frowand.list@gmail.com wrote: > From: Frank Rowand <frank.rowand@sony.com> > > "of: overlay: add missing of_node_get() in __of_attach_node_sysfs" > added a missing of_node_get() to __of_attach_node_sysfs(). This > results in a refcount imbalance for nodes attached with > dlpar_attach_node(). The calling sequence from dlpar_attach_node() > to __of_attach_node_sysfs() is: > > dlpar_attach_node() > of_attach_node() > __of_attach_node_sysfs() IIRC, there's a long standing item in the todo (Grant's) to convert the open coded dlpar code. Maybe you want to do that first? > > Signed-off-by: Frank Rowand <frank.rowand@sony.com> > --- > > ***** UNTESTED. I need people with the affected PowerPC systems > ***** (systems that dynamically allocate and deallocate > ***** devicetree nodes) to test this patch. Can't we write a test case that does the same thing? > > arch/powerpc/platforms/pseries/dlpar.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c > index a0b20c03f078..e3010b14aea5 100644 > --- a/arch/powerpc/platforms/pseries/dlpar.c > +++ b/arch/powerpc/platforms/pseries/dlpar.c > @@ -272,6 +272,8 @@ int dlpar_detach_node(struct device_node *dn) > if (rc) > return rc; > > + of_node_put(dn); > + > return 0; > } > > -- > Frank Rowand <frank.rowand@sony.com> >
On 10/18/18 10:09, Rob Herring wrote: > On Mon, Oct 15, 2018 at 07:37:24PM -0700, frowand.list@gmail.com wrote: >> From: Frank Rowand <frank.rowand@sony.com> >> >> "of: overlay: add missing of_node_get() in __of_attach_node_sysfs" >> added a missing of_node_get() to __of_attach_node_sysfs(). This >> results in a refcount imbalance for nodes attached with >> dlpar_attach_node(). The calling sequence from dlpar_attach_node() >> to __of_attach_node_sysfs() is: >> >> dlpar_attach_node() >> of_attach_node() >> __of_attach_node_sysfs() > > IIRC, there's a long standing item in the todo (Grant's) to convert the > open coded dlpar code. Maybe you want to do that first? I'd like to avoid extra delays to getting the current (with necesary fixes) series accepted because the series is rather intrusive and could have conflicts with other patches. I'm also worried that I don't have access to any of the systems that use the dynamic overlay code, and I don't have any way to test the changes. Can we encourage the users of this code to convert the open coded dlpar code? >> >> Signed-off-by: Frank Rowand <frank.rowand@sony.com> >> --- >> >> ***** UNTESTED. I need people with the affected PowerPC systems >> ***** (systems that dynamically allocate and deallocate >> ***** devicetree nodes) to test this patch. > > Can't we write a test case that does the same thing? I could write a simplistic test case, but I'm concerned that simplistic is not anywhere near sufficient. And my test case would reflect the same assumptions I already have when I wrote this patch. >> >> arch/powerpc/platforms/pseries/dlpar.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c >> index a0b20c03f078..e3010b14aea5 100644 >> --- a/arch/powerpc/platforms/pseries/dlpar.c >> +++ b/arch/powerpc/platforms/pseries/dlpar.c >> @@ -272,6 +272,8 @@ int dlpar_detach_node(struct device_node *dn) >> if (rc) >> return rc; >> >> + of_node_put(dn); >> + >> return 0; >> } >> >> -- >> Frank Rowand <frank.rowand@sony.com> >> >
On Thu, Oct 18, 2018 at 2:09 PM Frank Rowand <frowand.list@gmail.com> wrote: > > On 10/18/18 10:09, Rob Herring wrote: > > On Mon, Oct 15, 2018 at 07:37:24PM -0700, frowand.list@gmail.com wrote: > >> From: Frank Rowand <frank.rowand@sony.com> > >> > >> "of: overlay: add missing of_node_get() in __of_attach_node_sysfs" > >> added a missing of_node_get() to __of_attach_node_sysfs(). This > >> results in a refcount imbalance for nodes attached with > >> dlpar_attach_node(). The calling sequence from dlpar_attach_node() > >> to __of_attach_node_sysfs() is: > >> > >> dlpar_attach_node() > >> of_attach_node() > >> __of_attach_node_sysfs() > > > > IIRC, there's a long standing item in the todo (Grant's) to convert the > > open coded dlpar code. Maybe you want to do that first? > > I'd like to avoid extra delays to getting the current (with necesary > fixes) series accepted because the series is rather intrusive and > could have conflicts with other patches. > > I'm also worried that I don't have access to any of the systems that > use the dynamic overlay code, and I don't have any way to test the > changes. Mainly I was thinking you are asking them to test changes now, so I was thinking better to do that once than twice. Either way is fine though. > Can we encourage the users of this code to convert the open coded > dlpar code? That would be ideal... Rob
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c index a0b20c03f078..e3010b14aea5 100644 --- a/arch/powerpc/platforms/pseries/dlpar.c +++ b/arch/powerpc/platforms/pseries/dlpar.c @@ -272,6 +272,8 @@ int dlpar_detach_node(struct device_node *dn) if (rc) return rc; + of_node_put(dn); + return 0; }