linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] of: __of_update_property_sysfs only call __of_sysfs_remove_bin_file if of_node_is_attached
@ 2018-09-04 15:51 Andre Kalb
  2018-09-07 20:00 ` Frank Rowand
  0 siblings, 1 reply; 9+ messages in thread
From: Andre Kalb @ 2018-09-04 15:51 UTC (permalink / raw)
  To: robh+dt, frowand.list, devicetree, linux-kernel

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
___________________________________________________

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] of: __of_update_property_sysfs only call __of_sysfs_remove_bin_file if of_node_is_attached
  2018-09-04 15:51 [PATCH v2] of: __of_update_property_sysfs only call __of_sysfs_remove_bin_file if of_node_is_attached Andre Kalb
@ 2018-09-07 20:00 ` Frank Rowand
  2018-09-10  9:47   ` AW: " Andre Kalb
  0 siblings, 1 reply; 9+ messages in thread
From: Frank Rowand @ 2018-09-07 20:00 UTC (permalink / raw)
  To: Andre Kalb, robh+dt, devicetree, linux-kernel

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* AW: [PATCH v2] of: __of_update_property_sysfs only call __of_sysfs_remove_bin_file if of_node_is_attached
  2018-09-07 20:00 ` Frank Rowand
@ 2018-09-10  9:47   ` Andre Kalb
  2018-09-10 13:46     ` Rob Herring
  0 siblings, 1 reply; 9+ messages in thread
From: Andre Kalb @ 2018-09-10  9:47 UTC (permalink / raw)
  To: Frank Rowand, robh+dt, devicetree, linux-kernel

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] of: __of_update_property_sysfs only call __of_sysfs_remove_bin_file if of_node_is_attached
  2018-09-10  9:47   ` AW: " Andre Kalb
@ 2018-09-10 13:46     ` Rob Herring
  2018-09-13 15:21       ` AW: " Andre Kalb
  2018-09-19  9:25       ` Andre Kalb
  0 siblings, 2 replies; 9+ messages in thread
From: Rob Herring @ 2018-09-10 13:46 UTC (permalink / raw)
  To: Andre.Kalb; +Cc: Frank Rowand, devicetree, linux-kernel

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* AW: [PATCH v2] of: __of_update_property_sysfs only call __of_sysfs_remove_bin_file if of_node_is_attached
  2018-09-10 13:46     ` Rob Herring
@ 2018-09-13 15:21       ` Andre Kalb
  2018-09-26 21:05         ` Rob Herring
  2018-09-19  9:25       ` Andre Kalb
  1 sibling, 1 reply; 9+ messages in thread
From: Andre Kalb @ 2018-09-13 15:21 UTC (permalink / raw)
  To: Rob Herring; +Cc: Frank Rowand, devicetree, linux-kernel

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.

Best regards
Andre Kalb

From 1e86e351efa1a7cea31d157bafb0ae40b856cdcc Mon Sep 17 00:00:00 2001
From: Andre Kalb <Andre.Kalb@sma.de>
Date: Thu, 13 Sep 2018 16:42:48 +0200
Subject: [PATCH] Added new unittest

---
 drivers/of/unittest.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 2a547ca3d443..17f6cacb4fae 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -923,6 +923,24 @@ static int __init unittest_data_add(void)
                return -EINVAL;
        }

+       {
+               struct property *prop;
+
+               prop = kzalloc(sizeof(*prop), GFP_KERNEL);
+               if (!prop) {
+                       unittest(0, "kzalloc() failed\n");
+                       return -ENOMEM;
+               }
+
+               np = of_find_node_with_property(unittest_data_node, "prop-update");
+               unittest(np, "find prop-update");
+
+               prop->name = "prop-update";
+               prop->value = "new-property-data";
+               prop->length = strlen(prop->value) + 1;
+               unittest(of_update_property(np, prop) == 0, "Update a existing property failed\n");
+       }
+
        if (!of_root) {
                of_root = unittest_data_node;
                for_each_of_allnodes(np)
--
2.16.4


------------[ 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 ]---



> -----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

___________________________________________________

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
___________________________________________________

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2] of: __of_update_property_sysfs only call __of_sysfs_remove_bin_file if of_node_is_attached
  2018-09-10 13:46     ` Rob Herring
  2018-09-13 15:21       ` AW: " Andre Kalb
@ 2018-09-19  9:25       ` Andre Kalb
  1 sibling, 0 replies; 9+ messages in thread
From: Andre Kalb @ 2018-09-19  9:25 UTC (permalink / raw)
  To: Rob Herring; +Cc: Frank Rowand, devicetree, linux-kernel

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

___________________________________________________

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
___________________________________________________

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] of: __of_update_property_sysfs only call __of_sysfs_remove_bin_file if of_node_is_attached
  2018-09-13 15:21       ` AW: " Andre Kalb
@ 2018-09-26 21:05         ` Rob Herring
  2018-09-27  1:35           ` Frank Rowand
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2018-09-26 21:05 UTC (permalink / raw)
  To: Andre Kalb; +Cc: Frank Rowand, devicetree, linux-kernel

On Thu, Sep 13, 2018 at 03:21:27PM +0000, Andre Kalb wrote:
> 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.

Okay, can you resend both patches in one series rather than pasted in 
here.

Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] of: __of_update_property_sysfs only call __of_sysfs_remove_bin_file if of_node_is_attached
  2018-09-26 21:05         ` Rob Herring
@ 2018-09-27  1:35           ` Frank Rowand
  0 siblings, 0 replies; 9+ messages in thread
From: Frank Rowand @ 2018-09-27  1:35 UTC (permalink / raw)
  To: Rob Herring, Andre Kalb; +Cc: devicetree, linux-kernel

On 09/26/18 14:05, Rob Herring wrote:
> On Thu, Sep 13, 2018 at 03:21:27PM +0000, Andre Kalb wrote:
>> 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.
> 
> Okay, can you resend both patches in one series rather than pasted in 
> here.
> 
> Rob

I don't understand the need for the patch to __of_update_property_sysfs()
in the context of the current mainline Linux kernel.

As far as I know, there are no callers of of_update_property() against
a tree that is detached.  As such, the patch is adding complexity
without adding any functionality.

-Frank

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] of: __of_update_property_sysfs only call __of_sysfs_remove_bin_file if of_node_is_attached
@ 2018-09-17  7:42 Andre Kalb
  0 siblings, 0 replies; 9+ messages in thread
From: Andre Kalb @ 2018-09-17  7:42 UTC (permalink / raw)
  To: Rob Herring; +Cc: Frank Rowand, devicetree, linux-kernel

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.

Best regards
Andre Kalb


> On Thu, Sep 13, 2018 at 4:42 PM Andre Kalb <Andre.Kalb@sma.de> wrote:
> From 1e86e351efa1a7cea31d157bafb0ae40b856cdcc Mon Sep 17 00:00:00
> 2001
> From: Andre Kalb <Andre.Kalb@sma.de>
> Date: Thu, 13 Sep 2018 16:42:48 +0200
> Subject: [PATCH] Added new unittest
>
> ---
>  drivers/of/unittest.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index
> 2a547ca3d443..17f6cacb4fae 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -923,6 +923,24 @@ static int __init unittest_data_add(void)
>               return -EINVAL;
>       }
>
> +     {
> +             struct property *prop;
> +
> +             prop = kzalloc(sizeof(*prop), GFP_KERNEL);
> +             if (!prop) {
> +                     unittest(0, "kzalloc() failed\n");
> +                     return -ENOMEM;
> +             }
> +
> +             np = of_find_node_with_property(unittest_data_node, "prop-
> update");
> +             unittest(np, "find prop-update");
> +
> +             prop->name = "prop-update";
> +             prop->value = "new-property-data";
> +             prop->length = strlen(prop->value) + 1;
> +             unittest(of_update_property(np, prop) == 0, "Update a existing
> property failed\n");
> +     }
> +
>       if (!of_root) {
>               of_root = unittest_data_node;
>               for_each_of_allnodes(np)
> --
> 2.16.4

------------[ 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 ]---



> -----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

___________________________________________________

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
___________________________________________________

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-09-27  1:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-04 15:51 [PATCH v2] of: __of_update_property_sysfs only call __of_sysfs_remove_bin_file if of_node_is_attached Andre Kalb
2018-09-07 20:00 ` Frank Rowand
2018-09-10  9:47   ` AW: " Andre Kalb
2018-09-10 13:46     ` Rob Herring
2018-09-13 15:21       ` AW: " Andre Kalb
2018-09-26 21:05         ` Rob Herring
2018-09-27  1:35           ` Frank Rowand
2018-09-19  9:25       ` Andre Kalb
2018-09-17  7:42 Andre Kalb

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).