diff mbox series

[v2] of: __of_update_property_sysfs only call __of_sysfs_remove_bin_file if of_node_is_attached

Message ID AM6PR04MB52394FDB1266388BCE1374E3FC030@AM6PR04MB5239.eurprd04.prod.outlook.com
State Superseded, archived
Headers show
Series [v2] of: __of_update_property_sysfs only call __of_sysfs_remove_bin_file if of_node_is_attached | expand

Commit Message

Andre Kalb Sept. 4, 2018, 3:51 p.m. UTC 
To prevent warning "kernfs: can not remove '(null)', no directory" if an overlay isn't applied to the active devicetree.

Using of_remove_property and then of_add_property doesn't show the warning.

Signed-off-by: Andre Kalb <Andre.Kalb@sma.de>
---
Changes in v2:
  - Fix typo

 drivers/of/kobj.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--
2.17.1

Comments

Frank Rowand Sept. 7, 2018, 8 p.m. UTC | #1 
Hi Andred,

On 09/04/18 08:51, Andre Kalb wrote:
> To prevent warning "kernfs: can not remove '(null)', no directory" if an overlay isn't applied to the active devicetree.
> 
> Using of_remove_property and then of_add_property doesn't show the warning.
> 
> Signed-off-by: Andre Kalb <Andre.Kalb@sma.de>
> ---
> Changes in v2:
>   - Fix typo
> 
>  drivers/of/kobj.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/of/kobj.c b/drivers/of/kobj.c index 7a0a18980b98..962b660e8ad1 100644
> --- a/drivers/of/kobj.c
> +++ b/drivers/of/kobj.c
> @@ -104,7 +104,7 @@ void __of_update_property_sysfs(struct device_node *np, struct property *newprop
>                 struct property *oldprop)
>  {
>         /* At early boot, bail out and defer setup to of_init() */
> -       if (!of_kset)
> +       if (!of_kset || !of_node_is_attached(np))
>                 return;
> 
>         if (oldprop)
> --
> 2.17.1
> 
> 
> ___________________________________________________
> 
> SMA Solar Technology AG
> Aufsichtsrat: Dr. Erik Ehrentraut (Vorsitzender)
> Vorstand: Ulrich Hadding, Dr.-Ing. Juergen Reinert, Pierre-Pascal Urbon
> Handelsregister: Amtsgericht Kassel HRB 3972
> Sitz der Gesellschaft: 34266 Niestetal
> USt-ID-Nr. DE 113 08 59 54
> WEEE-Reg.-Nr. DE 95881150
> ___________________________________________________
> 

What is the calling path that results in the warning?

-Frank
Andre Kalb Sept. 10, 2018, 9:47 a.m. UTC | #2 
Hi Frank,

> -----Ursprüngliche Nachricht-----
> Von: Frank Rowand [mailto:frowand.list@gmail.com]
> Gesendet: Freitag, 7. September 2018 22:01
> An: Andre Kalb; robh+dt@kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Betreff: Re: [PATCH v2] of: __of_update_property_sysfs only call
> __of_sysfs_remove_bin_file if of_node_is_attached
> 
> Hi Andred,
> 
> On 09/04/18 08:51, Andre Kalb wrote:
> > To prevent warning "kernfs: can not remove '(null)', no directory" if an overlay
> isn't applied to the active devicetree.
> >
> > Using of_remove_property and then of_add_property doesn't show the
> warning.
> >
> > Signed-off-by: Andre Kalb <Andre.Kalb@sma.de>
> > ---
> > Changes in v2:
> >   - Fix typo
> >
> >  drivers/of/kobj.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/of/kobj.c b/drivers/of/kobj.c index
> > 7a0a18980b98..962b660e8ad1 100644
> > --- a/drivers/of/kobj.c
> > +++ b/drivers/of/kobj.c
> > @@ -104,7 +104,7 @@ void __of_update_property_sysfs(struct device_node
> *np, struct property *newprop
> >                 struct property *oldprop)  {
> >         /* At early boot, bail out and defer setup to of_init() */
> > -       if (!of_kset)
> > +       if (!of_kset || !of_node_is_attached(np))
> >                 return;
> >
> >         if (oldprop)
> > --
> > 2.17.1
> >
> >
> > ___________________________________________________
> >
> > SMA Solar Technology AG
> > Aufsichtsrat: Dr. Erik Ehrentraut (Vorsitzender)
> > Vorstand: Ulrich Hadding, Dr.-Ing. Juergen Reinert, Pierre-Pascal
> > Urbon
> > Handelsregister: Amtsgericht Kassel HRB 3972 Sitz der Gesellschaft:
> > 34266 Niestetal USt-ID-Nr. DE 113 08 59 54 WEEE-Reg.-Nr. DE 95881150
> > ___________________________________________________
> >
> 
> What is the calling path that results in the warning?
> 
> -Frank

There is the callstack of the warning.

[   10.782830] ------------[ cut here ]------------
[   10.830357] WARNING: CPU: 0 PID: 170 at /linux-4.x/fs/kernfs/dir.c:1276 kernfs_remove_by_name_ns+0x30/0x80()
[   10.928997] kernfs: can not remove '(null)', no directory
[   10.993107] Modules linked in: module_capemgr(+)
[   11.045750] CPU: 0 PID: 170 Comm: systemd-udevd Not tainted 4.4.143-00158-g8e5ca65ec7ee-dirty #114
[   11.159764] Hardware name: Generic AM33XX (Flattened Device Tree)
[   11.260989] Backtrace:
[   11.276618] [<c010b51c>] (dump_backtrace) from [<c010b700>] (show_stack+0x18/0x1c)
[   11.340099]  r6:c071b211 r5:00000009 r4:00000000 r3:d0c4896c
[   11.394058] [<c010b6e8>] (show_stack) from [<c033f220>] (dump_stack+0x20/0x28)
[   11.401704] [<c033f200>] (dump_stack) from [<c012684c>] (warn_slowpath_common+0x90/0xb8)
[   11.471765] [<c01267bc>] (warn_slowpath_common) from [<c01268ac>] (warn_slowpath_fmt+0x38/0x40)
[   11.509244]  r8:bf000d30 r7:600f0113 r6:00000000 r5:00000000 r4:00000000
[   11.571483] [<c0126878>] (warn_slowpath_fmt) from [<c024cb28>] (kernfs_remove_by_name_ns+0x30/0x80)
[   11.586620]  r3:00000000 r2:c071b2e7
[   11.603293] [<c024caf8>] (kernfs_remove_by_name_ns) from [<c024e2e8>] (sysfs_remove_bin_file+0x1c/0x20)
[   11.665357]  r6:cf6d2664 r5:cf6d2664 r4:cf6d27e4 r3:cf099580
[   11.703347] [<c024e2cc>] (sysfs_remove_bin_file) from [<c048d4d4>] (__of_sysfs_remove_bin_file+0x1c/0x28)
[   11.750587] [<c048d4b8>] (__of_sysfs_remove_bin_file) from [<c048d674>] (__of_update_property_sysfs+0x34/0x48)
[   11.805954]  r4:cf53b590 r3:cf099580
[   11.825184] [<c048d640>] (__of_update_property_sysfs) from [<c048d724>] (of_update_property+0x9c/0xdc)
[   11.891243]  r5:cf53b590 r4:00000000
[   11.904186] [<c048d688>] (of_update_property) from [<bf000674>] (module_capemgr_slot_scan+0x590/0x69c [module_capemgr])
[   12.005277]  r7:00000001 r6:00000000 r5:cf191610 r4:cf53b590
[   12.011373] [<bf0000e4>] (module_capemgr_slot_scan [module_capemgr]) from [<bf0007bc>] (module_capemgr_probe+0x3c/0x58 [module_capemgr])
[   12.115310]  r10:bf000e4c r9:00000004 r8:c0968160 r7:00000000 r6:bf000ce4 r5:bf000780
[   12.128143]  r4:cf191600
[   12.155023] [<bf000780>] (module_capemgr_probe [module_capemgr]) from [<c03b5464>] (platform_drv_probe+0x58/0xa4)
[   12.184861]  r4:cf191610 r3:fffffdfb
[   12.204949] [<c03b540c>] (platform_drv_probe) from [<c03b3a4c>] (driver_probe_device+0x194/0x414)
[   12.254228]  r6:c099538c r5:bf000ce4 r4:cf191610 r3:c03b540c
[   12.260305] [<c03b38b8>] (driver_probe_device) from [<c03b3d3c>] (__driver_attach+0x70/0x94)
[   12.294832]  r9:c090204c r8:c0968088 r7:00000000 r6:bf000ce4 r5:cf191644 r4:cf191610
[   12.309680] [<c03b3ccc>] (__driver_attach) from [<c03b1ce4>] (bus_for_each_dev+0x7c/0x90)
[   12.374480]  r6:c03b3ccc r5:bf000ce4 r4:00000000 r3:c03b3ccc
[   12.390638] [<c03b1c68>] (bus_for_each_dev) from [<c03b3458>] (driver_attach+0x20/0x28)
[   12.424719]  r6:c093c000 r5:cf5aa180 r4:bf000ce4
[   12.444029] [<c03b3438>] (driver_attach) from [<c03b2fa4>] (bus_add_driver+0x11c/0x248)
[   12.486713] [<c03b2e88>] (bus_add_driver) from [<c03b43f8>] (driver_register+0xa4/0xe8)
[   12.548896]  r8:00000000 r7:c0904d60 r6:bf003000 r5:c0904d60 r4:bf000ce4
[   12.578947] [<c03b4354>] (driver_register) from [<c03b537c>] (__platform_driver_register+0x38/0x4c)
[   12.629857]  r5:c0904d60 r4:cf66f700
[   12.636955] [<c03b5344>] (__platform_driver_register) from [<bf003018>] (module_capemgr_driver_init+0x18/0x24 [module_capemgr])
[   12.710512] [<bf003000>] (module_capemgr_driver_init [module_capemgr]) from [<c0101794>] (do_one_initcall+0x1b4/0x1f8)
[   12.743788] [<c01015e0>] (do_one_initcall) from [<c01a74ec>] (do_init_module+0x64/0x1c0)
[   12.752350]  r9:00000000 r8:cf66dec0 r7:00000018 r6:cf66f740 r5:bf000e40 r4:bf000e40
[   12.789673] [<c01a7488>] (do_init_module) from [<c0180c84>] (load_module+0x18f4/0x1998)
[   12.819328]  r6:00000000 r5:bf000e40 r4:cf66df44
[   12.843368] [<c017f390>] (load_module) from [<c0180f30>] (SyS_finit_module+0x90/0xa8)
[   12.878387]  r10:00000080 r9:cf66c000 r8:c0107a44 r7:0000017b r6:00000007 r5:00ae3688
[   12.907627]  r4:00000000
[   12.910386] [<c0180ea0>] (SyS_finit_module) from [<c0107a20>] (__sys_trace_return+0x0/0x10)
[   12.940643]  r6:00000000 r5:00000000 r4:00ae3d80
[   12.966153] ---[ end trace 5390dec54f12ed11 ]---

Best regards,
Andre Kalb
Rob Herring Sept. 10, 2018, 1:46 p.m. UTC | #3 
On Mon, Sep 10, 2018 at 4:51 AM Andre Kalb <Andre.Kalb@sma.de> wrote:
>
> Hi Frank,
>
> > -----Ursprüngliche Nachricht-----
> > Von: Frank Rowand [mailto:frowand.list@gmail.com]
> > Gesendet: Freitag, 7. September 2018 22:01
> > An: Andre Kalb; robh+dt@kernel.org; devicetree@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > Betreff: Re: [PATCH v2] of: __of_update_property_sysfs only call
> > __of_sysfs_remove_bin_file if of_node_is_attached
> >
> > Hi Andred,
> >
> > On 09/04/18 08:51, Andre Kalb wrote:
> > > To prevent warning "kernfs: can not remove '(null)', no directory" if an overlay
> > isn't applied to the active devicetree.
> > >
> > > Using of_remove_property and then of_add_property doesn't show the
> > warning.

[...]

> > What is the calling path that results in the warning?
> >
> > -Frank
>
> There is the callstack of the warning.
>
> [   10.782830] ------------[ cut here ]------------
> [   10.830357] WARNING: CPU: 0 PID: 170 at /linux-4.x/fs/kernfs/dir.c:1276 kernfs_remove_by_name_ns+0x30/0x80()
> [   10.928997] kernfs: can not remove '(null)', no directory
> [   10.993107] Modules linked in: module_capemgr(+)
> [   11.045750] CPU: 0 PID: 170 Comm: systemd-udevd Not tainted 4.4.143-00158-g8e5ca65ec7ee-dirty #114

158 patches on top of an almost 3 year old kernel...

> [   12.011373] [<bf0000e4>] (module_capemgr_slot_scan [module_capemgr]) from [<bf0007bc>] (module_capemgr_probe+0x3c/0x58 [module_capemgr])

And a driver that's not upstream.

Not saying the fix isn't valid, but please reproduce on recent
mainline. Add a unittest if you have to.

Rob
Andre Kalb Sept. 19, 2018, 9:25 a.m. UTC | #4 
Hi Rob,

I have used an other hardware to check the patch. I hope it doesn't matter. I added few lines at the untitest.c. All existing unittest use an attached sysfs, therefore the bug isn't detectable.

The patch of the unittest is posted: https://lore.kernel.org/patchwork/patch/985738/

------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at fs/kernfs/dir.c:1481 kernfs_remove_by_name_ns+0x9c/0xa4
kernfs: can not remove '(null)', no directory
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 4.18.7 #3
Hardware name: Freescale i.MX6 Ultralite (Device Tree)
[<c010f338>] (unwind_backtrace) from [<c010cefc>] (show_stack+0x10/0x14)
[<c010cefc>] (show_stack) from [<c0122004>] (__warn+0xe0/0xf8)
[<c0122004>] (__warn) from [<c0122060>] (warn_slowpath_fmt+0x44/0x68)
[<c0122060>] (warn_slowpath_fmt) from [<c02c6540>] (kernfs_remove_by_name_ns+0x9c/0xa4)
[<c02c6540>] (kernfs_remove_by_name_ns) from [<c055a704>] (__of_update_property_sysfs+0x38/0x50)
[<c055a704>] (__of_update_property_sysfs) from [<c0557b94>] (of_update_property+0xc0/0x104)
[<c0557b94>] (of_update_property) from [<c09521bc>] (of_unittest+0x1b8/0x28d4)
[<c09521bc>] (of_unittest) from [<c0102d88>] (do_one_initcall+0x40/0x218)
[<c0102d88>] (do_one_initcall) from [<c090101c>] (kernel_init_freeable+0x24c/0x2e4)
[<c090101c>] (kernel_init_freeable) from [<c06a1f6c>] (kernel_init+0x8/0x110)
[<c06a1f6c>] (kernel_init) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
Exception stack(0xcc099fb0 to 0xcc099ff8)
9fa0:                                     00000000 00000000 00000000 00000000
9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
---[ end trace 4522f69e0760e4d5 ]---

Best regards
Andre Kalb

> -----Ursprüngliche Nachricht-----
> Von: Rob Herring [mailto:robh+dt@kernel.org]
> Gesendet: Montag, 10. September 2018 15:47
> An: Andre Kalb
> Cc: Frank Rowand; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> Betreff: Re: [PATCH v2] of: __of_update_property_sysfs only call
> __of_sysfs_remove_bin_file if of_node_is_attached
>
> On Mon, Sep 10, 2018 at 4:51 AM Andre Kalb <Andre.Kalb@sma.de> wrote:
> >
> > Hi Frank,
> >
> > > -----Ursprüngliche Nachricht-----
> > > Von: Frank Rowand [mailto:frowand.list@gmail.com]
> > > Gesendet: Freitag, 7. September 2018 22:01
> > > An: Andre Kalb; robh+dt@kernel.org; devicetree@vger.kernel.org;
> > > linux- kernel@vger.kernel.org
> > > Betreff: Re: [PATCH v2] of: __of_update_property_sysfs only call
> > > __of_sysfs_remove_bin_file if of_node_is_attached
> > >
> > > Hi Andred,
> > >
> > > On 09/04/18 08:51, Andre Kalb wrote:
> > > > To prevent warning "kernfs: can not remove '(null)', no directory"
> > > > if an overlay
> > > isn't applied to the active devicetree.
> > > >
> > > > Using of_remove_property and then of_add_property doesn't show the
> > > warning.
>
> [...]
>
> > > What is the calling path that results in the warning?
> > >
> > > -Frank
> >
> > There is the callstack of the warning.
> >
> > [   10.782830] ------------[ cut here ]------------
> > [   10.830357] WARNING: CPU: 0 PID: 170 at /linux-4.x/fs/kernfs/dir.c:1276
> kernfs_remove_by_name_ns+0x30/0x80()
> > [   10.928997] kernfs: can not remove '(null)', no directory
> > [   10.993107] Modules linked in: module_capemgr(+)
> > [   11.045750] CPU: 0 PID: 170 Comm: systemd-udevd Not tainted 4.4.143-
> 00158-g8e5ca65ec7ee-dirty #114
>
> 158 patches on top of an almost 3 year old kernel...
>
> > [   12.011373] [<bf0000e4>] (module_capemgr_slot_scan [module_capemgr])
> from [<bf0007bc>] (module_capemgr_probe+0x3c/0x58 [module_capemgr])
>
> And a driver that's not upstream.
>
> Not saying the fix isn't valid, but please reproduce on recent mainline. Add a
> unittest if you have to.
>
> Rob
diff mbox series

Patch

diff --git a/drivers/of/kobj.c b/drivers/of/kobj.c index 7a0a18980b98..962b660e8ad1 100644
--- a/drivers/of/kobj.c
+++ b/drivers/of/kobj.c
@@ -104,7 +104,7 @@  void __of_update_property_sysfs(struct device_node *np, struct property *newprop
                struct property *oldprop)
 {
        /* At early boot, bail out and defer setup to of_init() */
-       if (!of_kset)
+       if (!of_kset || !of_node_is_attached(np))
                return;

        if (oldprop)