Hi Rob,
On Thu, Jul 20, 2023 at 1:03 AM Rob Herring <robh@xxxxxxxxxx> wrote:
> On Wed, Jul 19, 2023 at 05:00:01PM +0200, Geert Uytterhoeven wrote:
> > Formatting strings using "%pOF" while holding devtree_lock causes a
> > deadlock. Lockdep reports:
> >
> > of_get_parent from of_fwnode_get_parent+0x18/0x24
> > ^^^^^^^^^^^^^
> > of_fwnode_get_parent from fwnode_count_parents+0xc/0x28
> > fwnode_count_parents from fwnode_full_name_string+0x18/0xac
> > fwnode_full_name_string from device_node_string+0x1a0/0x404
> > device_node_string from pointer+0x3c0/0x534
> > pointer from vsnprintf+0x248/0x36c
> > vsnprintf from vprintk_store+0x130/0x3b4
> >
> > Fix this by making the locking cover only the parts that really need it.
> >
> > Fixes: 0d638a07d3a1e98a ("of: Convert to using %pOF instead of full_name")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> > ---
> > drivers/of/dynamic.c | 12 ++++++++++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> > index e311d406b1705306..eae45a1c673ee05f 100644
> > --- a/drivers/of/dynamic.c
> > +++ b/drivers/of/dynamic.c
> > @@ -601,13 +601,16 @@ static int __of_changeset_entry_apply(struct of_changeset_entry *ce)
> >
> > __of_changeset_entry_dump(ce);
> >
> > - raw_spin_lock_irqsave(&devtree_lock, flags);
> > switch (ce->action) {
> > case OF_RECONFIG_ATTACH_NODE:
> > + raw_spin_lock_irqsave(&devtree_lock, flags);
> > __of_attach_node(ce->np);
> > + raw_spin_unlock_irqrestore(&devtree_lock, flags);
>
> I think you could just move the spinlock into __of_attach_node(). The
> only other caller looks like this.
I'd rather not do that, as the double underscore is typically used to
indicate that this function does not take the lock.
Cfr. of_find_property() vs. __of_find_property().
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
