On Tue, May 28, 2019 at 09:14:12PM +0200, Mathieu Malaterre wrote:
> On Tue, May 28, 2019 at 7:21 AM Michael Ellerman <m...@ellerman.id.au> wrote:
> > Mathieu Malaterre <ma...@debian.org> writes:
> > > Is there a way to dump more context (somewhere in of tree
> > > flattening?). I cannot make sense of the following:
> >
> > Hmm. Not that I know of.
> >
> > Those don't look related to OF flattening/unflattening. That's just
> > sysfs setup based on the unflattened device tree.
> >
> > The allocations are happening in safe_name() AFAICS.
> >
> > int __of_add_property_sysfs(struct device_node *np, struct property *pp)
> > {
> > ...
> > pp->attr.attr.name = safe_name(&np->kobj, pp->name);
> >
> > And the free is in __of_sysfs_remove_bin_file():
> >
> > void __of_sysfs_remove_bin_file(struct device_node *np, struct property
> > *prop)
> > {
> > if (!IS_ENABLED(CONFIG_SYSFS))
> > return;
> >
> > sysfs_remove_bin_file(&np->kobj, &prop->attr);
> > kfree(prop->attr.attr.name);
> >
>
> Right. That helped a lot !
>
> > There is this check which could be failing leading to us not calling the
> > free at all:
> >
> > void __of_remove_property_sysfs(struct device_node *np, struct property
> > *prop)
> > {
> > /* at early boot, bail here and defer setup to of_init() */
> > if (of_kset && of_node_is_attached(np))
> > __of_sysfs_remove_bin_file(np, prop);
> > }
> >
> >
> > So maybe stick a printk() in there to see if you're hitting that
> > condition, eg something like:
> >
> > if (of_kset && of_node_is_attached(np))
> > __of_sysfs_remove_bin_file(np, prop);
> > else
> > printk("%s: leaking prop %s on node %pOF\n", __func__,
> > prop->attr.attr.name, np);
> >
>
> If I understand correctly those are false positive. I was first
> starting to consider using something like kmemleak_not_leak, but I
> remember that I have been using kmemleak for a couple of years now.
> Those reports starting to show up only recently.
>
> Catalin, do you have an idea why on a non-SMP machine kmemleak reports
> leaks from:
>
> [...]
> void __init of_core_init(void)
> {
> [...]
> for_each_of_allnodes(np)
> __of_attach_node_sysfs(np);
It's likely that they are false positives but usually, rather than just
adding a kmemleak_not_leak(), it's better to figure out why kmemleak
reports them. The strings allocated above through kstrdup() can't be
tracked starting with the root objects. I think for the of stuff, this
should be the of_root pointer.
Is it only with non-SMP that this happens? I can't reproduce it on arm64
to be able to dig further.
Even better if you could bisect to the commit that's causing this.
--
Catalin
