From: Julia Lawall <julia.lawall@inria.fr>
To: Deepak R Varma <drv@mailo.com>
Cc: nicolas palix <nicolas.palix@imag.fr>, cocci <cocci@inria.fr>,
linux-kernel <linux-kernel@vger.kernel.org>,
Saurabh Singh Sengar <ssengar@microsoft.com>,
Praveen Kumar <kumarpraveen@linux.microsoft.com>
Subject: Re: [cocci] [PATCH] coccinelle: put_device: Include of_node_put to avoid false positives
Date: Sun, 26 Feb 2023 21:33:34 +0100 (CET) [thread overview]
Message-ID: <alpine.DEB.2.22.394.2302262130430.2761@hadrien> (raw)
In-Reply-To: <Y/u/mV6GUC5+l3m6@ubun2204.myguest.virtualbox.org>
On Mon, 27 Feb 2023, Deepak R Varma wrote:
> On Sat, Feb 25, 2023 at 08:17:01PM +0100, Julia Lawall wrote:
> > > The node reference increased by of_find_device_by_node() can also be
> > > released by using a call to of_node_put(). Hence when this exists, the
> > > script should not trigger a warning, which otherwise will be a false
> > > positive.
> >
> > Could you explain more about why of_node_put is sufficient?
>
> You are right. In fact, I think calling of_node_put() for a prior
> of_find_device_by_node() is buggy as a call to of_find_device_by_node()
> increments the kref for the device embedding the node and not the kref of the
> node. Hence a call to put_device() is required to decrement the device kref.
> Calling the of_node_put() attempts to decrement the kref of the node, which I
> think is not correct.
>
> May I request any comment on my understanding?
I think that decrementing a kref and reaching 0 would trigger some cleanup
action. I don't know what that cleanup action is in this case.
Did someone tell you that of_node_put was good enough? Perhaps that
person could provide more explanation.
In looking quickly though the code, the focus seemed to be on the device.
The of node is just used for comparison to check that we have the right
one. But I don't know if cleaning up the of node could somehow trigger
cleaning up the device as well.
julia
>
> Thank you,
> deepak.
>
> >
> > thanks,
> > julia
> >
> > > Also, improve the warning message to include of_node_put too is missing.
> > >
> > > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > > ---
> > > scripts/coccinelle/free/put_device.cocci | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/scripts/coccinelle/free/put_device.cocci
> > > b/scripts/coccinelle/free/put_device.cocci
> > > index f09f1e79bfa6..259195b501aa 100644
> > > --- a/scripts/coccinelle/free/put_device.cocci
> > > +++ b/scripts/coccinelle/free/put_device.cocci
> > > @@ -18,8 +18,10 @@ type T,T1,T2,T3;
> > >
> > > id = of_find_device_by_node@p1(x)
> > > ... when != e = id
> > > + when != of_node_put(x)
> > > if (id == NULL || ...) { ... return ...; }
> > > ... when != put_device(&id->dev)
> > > + when != of_node_put(x)
> > > when != platform_device_put(id)
> > > when != if (id) { ... put_device(&id->dev) ... }
> > > when != e1 = (T)id
> > > @@ -42,7 +44,7 @@ p2 << search.p2;
> > > @@
> > >
> > > coccilib.report.print_report(p2[0],
> > > - "ERROR: missing put_device; call
> > > of_find_device_by_node on line "
> > > + "ERROR: missing put_device or of_node_put; call
> > > of_find_device_by_node on line "
> > > + p1[0].line
> > > + ", but without a corresponding object release within this function.")
> > >
> > > --
> > > 2.34.1
>
>
>
prev parent reply other threads:[~2023-02-26 20:33 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-17 18:11 [cocci] [PATCH] coccinelle: put_device: Include of_node_put to avoid false positives Deepak R Varma
2023-02-25 19:17 ` Julia Lawall
2023-02-26 20:22 ` Deepak R Varma
2023-02-26 20:33 ` Julia Lawall [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.22.394.2302262130430.2761@hadrien \
--to=julia.lawall@inria.fr \
--cc=cocci@inria.fr \
--cc=drv@mailo.com \
--cc=kumarpraveen@linux.microsoft.com \
--cc=linux-kernel@vger.kernel.org \
--cc=nicolas.palix@imag.fr \
--cc=ssengar@microsoft.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).