lkml.org 
[lkml]   [2017]   [Jul]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH] device property: Fix usecount for of_graph_get_port_parent()
    On Fri, Jul 28, 2017 at 3:23 AM, Tony Lindgren <tony@atomide.com> wrote:
    > * Tony Lindgren <tony@atomide.com> [170727 23:02]:
    >> * Rob Herring <robh+dt@kernel.org> [170727 15:19]:
    >> > On Thu, Jul 27, 2017 at 4:44 AM, Tony Lindgren <tony@atomide.com> wrote:
    >> > > --- a/drivers/of/property.c
    >> > > +++ b/drivers/of/property.c
    >> > > @@ -708,6 +708,15 @@ struct device_node *of_graph_get_port_parent(struct device_node *node)
    >> > > {
    >> > > unsigned int depth;
    >> > >
    >> > > + if (!node)
    >> > > + return NULL;
    >> > > +
    >> > > + /*
    >> > > + * Preserve usecount for passed in node as of_get_next_parent()
    >> > > + * will do of_node_put() on it.
    >> > > + */
    >> > > + of_node_get(node);
    >> >
    >> > I think this messes up of_graph_get_remote_port_parent(). First it
    >> > calls of_graph_get_remote_endpoint which returns the endpoint node
    >> > with ref count incremented. Then you are incrementing it again here.
    >>
    >> Hmm OK looks like I missed that one. If we want to have
    >> of_graph_get_port_parent not trash the node passed to it, we should
    >> just change things there too:
    >
    > Found few more things to fix with git grep -C20 of_graph_get_port_parent,
    > below is v2.
    >
    > Can you all prese review again and do some testing. I also fixed up
    > audio-graph-scu-card but can't test that one.
    >
    > Regards,
    >
    > Tony
    >
    > 8< ------
    > From tony Mon Sep 17 00:00:00 2001
    > From: Tony Lindgren <tony@atomide.com>
    > Date: Thu, 27 Jul 2017 01:30:16 -0700
    > Subject: [PATCHv2] device property: Fix usecount for
    > of_graph_get_port_parent()
    >
    > Fix inconsistent use of of_graph_get_port_parent() where
    > asoc_simple_card_parse_graph_dai() does of_node_get() before
    > calling it while other callers do not. We can fix this by
    > not trashing the node passed to of_graph_get_port_parent().
    >
    > Let's also make sure the callers have correct refcounts and remove
    > related incorrect of_node_put() calls for of_for_each_phandle
    > as that's done by of_phandle_iterator_next() except when
    > we break out of the loop early.
    >
    > Let's fix both issues with a single patch to avoid kobject
    > refcounts getting messed up more if two patches are merged
    > separately.
    >
    > Otherwise strange issues can happen caused by memory corruption
    > caused by too many kobject_del() calls such as:
    >
    > BUG: sleeping function called from invalid context at
    > kernel/locking/mutex.c:747
    > ...
    > (___might_sleep)
    > (__mutex_lock)
    > (mutex_lock_nested)
    > (kernfs_remove)
    > (kobject_del)
    > (kobject_put)
    > (of_get_next_parent)
    > (of_graph_get_port_parent)
    > (asoc_simple_card_parse_graph_dai [snd_soc_simple_card_utils])
    > (asoc_graph_card_probe [snd_soc_audio_graph_card])
    >
    > Fixes: 0ef472a973eb ("of_graph: add of_graph_get_port_parent()")
    > Fixes: 2692c1c63c29 ("ASoC: add audio-graph-card support")
    > Fixes: 1689333f8311 ("ASoC: simple-card-utils: add asoc_simple_card_parse_graph_dai()")
    > Cc: Mark Brown <broonie@kernel.org>
    > Cc: Takashi Iwai <tiwai@suse.com>
    > Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
    > Cc: alsa-devel@alsa-project.org
    > Signed-off-by: Tony Lindgren <tony@atomide.com>
    > ---
    > drivers/of/property.c | 17 +++++++++++++++--
    > sound/soc/generic/audio-graph-card.c | 10 +++++-----
    > sound/soc/generic/audio-graph-scu-card.c | 15 +++++++++------
    > sound/soc/generic/simple-card-utils.c | 8 +++-----
    > sound/soc/soc-core.c | 2 ++
    > 5 files changed, 34 insertions(+), 18 deletions(-)

    LGTM. I'm assuming this will go thru ALSA tree.

    Reviewed-by: Rob Herring <robh@kernel.org>

    Rob

    \
     
     \ /
      Last update: 2017-07-29 00:59    [W:5.085 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site