| Message ID | Y+/DQ6l0pDcxC2c3@ubun2204.myguest.virtualbox.org |
|---|---|
| State | New |
| Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org>
Delivered-To: ouuuleilei@gmail.com
Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp1041202wrn;
Fri, 17 Feb 2023 10:21:50 -0800 (PST)
X-Google-Smtp-Source:
AK7set8+ef4TjwwHTA6UknvpRO51Zr/IjeAk8u8afdCNOTpQHOVde3Vnzuc6x3g96qFYC2HzbkHa
X-Received: by 2002:a17:907:3f1d:b0:878:54e3:e3e1 with SMTP id
hq29-20020a1709073f1d00b0087854e3e3e1mr13128668ejc.73.1676658110184;
Fri, 17 Feb 2023 10:21:50 -0800 (PST)
ARC-Seal: i=1; a=rsa-sha256; t=1676658110; cv=none;
d=google.com; s=arc-20160816;
b=IjWKkP7A08vJx2g45Z2bw++rUPzbPe8z1zWOqTsH0cEbUfnUm+YM+tlBDG7yIfAqut
knxLgT6iQqkB8/skM6l38uShZ1P3mLr94yzMCLiSl49ul6+ol3LdOBDrZV8ZZfwTqiuI
J6CSuy1lMhXXBseASUFn7k62nb7brzcIQ2OBuWYC4dUP7pNXG/Ig92+PZ6E4difnIR8f
cKILoTKeLUcauPPFeP5QKdgzebul8PYclcULtaYDb3kVNh4BUr+X+qYJgSX76uPwk+Xg
mn3TydXn6i5WL8dLv1LvNtdMaIQSZJ8wR4yyQxTZeBKjtGD7QsDTyE04tmWOfTARqWWv
W+aw==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;
s=arc-20160816;
h=list-id:precedence:content-disposition:mime-version:message-id
:subject:cc:to:from:date:dkim-signature;
bh=uF1Fo/vxuXDHK1hx9obu+PtDsoT83kRlDffe6osG7tA=;
b=njsODdBSxDpcPNtSeZCz+EUJtk4nkA0KxcH/TnFBsqK6NdwCsffQD8hjXWleixL4M8
FVnsjU4L9IadO5Sp4uJyB5pJRMwGQ4HAOEJ3sxTYo0lIAgv1y8lTZ315YptIqItksfHF
zLUoFBSGfvxum8xAZnZkTpjYiwb0796vADWMvwTwYQZvPLRXthrbNDajF59ar4edoYRa
7ui51xhq+lMunWOx4ECRARrWu2JhtCP7ay7k1tnavk6bHjrtrPLwNjqEXXmy9bsC0sD8
4OuGf2K7vWY3jLz8BBuh/Lm5UfgH25s0wovBVDVXUqHs44JMAP2dJbcSnWIuBV6IkQXk
R2dw==
ARC-Authentication-Results: i=1; mx.google.com;
dkim=fail header.i=@mailo.com header.s=mailo header.b=Law7bSE6;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=mailo.com
Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20])
by mx.google.com with ESMTP id
j18-20020a170906105200b0088e6b7aff33si6784318ejj.631.2023.02.17.10.21.27;
Fri, 17 Feb 2023 10:21:50 -0800 (PST)
Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
client-ip=2620:137:e000::1:20;
Authentication-Results: mx.google.com;
dkim=fail header.i=@mailo.com header.s=mailo header.b=Law7bSE6;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=mailo.com
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S229561AbjBQSL3 (ORCPT <rfc822;aimixsaka@gmail.com> + 99 others);
Fri, 17 Feb 2023 13:11:29 -0500
Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59572 "EHLO
lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S229463AbjBQSL2 (ORCPT
<rfc822;linux-kernel@vger.kernel.org>);
Fri, 17 Feb 2023 13:11:28 -0500
Received: from msg-1.mailo.com (msg-1.mailo.com [213.182.54.11])
by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DC11F93D8
for <linux-kernel@vger.kernel.org>;
Fri, 17 Feb 2023 10:11:26 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=mailo.com; s=mailo;
t=1676657483; bh=B1VmI+k4wIVX7khCOAIv+GNjdsvNDDHabRSUeTur1gA=;
h=X-EA-Auth:Date:From:To:Cc:Subject:Message-ID:MIME-Version:
Content-Type;
b=Law7bSE6Glg0+9yG/HAooltXnzZ6dwxgwTYT8oHj/qfMR5xu9fQWY8jGmXEXg9BL4
QBWpLTlAa5TNX2lIfN2pGuLlYzkM8cHJxq9DrDOjrG0nDDShGndLt0fspqfrv90qVq
sx4XpEYmZ64P7JBJw0wJbSpJZ9e86UliEi2EUO7s=
Received: by b-6.in.mailobj.net [192.168.90.16] with ESMTP
via ip-206.mailobj.net [213.182.55.206]
Fri, 17 Feb 2023 19:11:23 +0100 (CET)
X-EA-Auth:
1k3b9EoeNntTY48xB+Me88rLNyQ3wA2W1Ud1IgEcZASYaFhVB1O10fXPILulqKxnzytB6riIxpGVFjeDWvy9fpAJMEJUdF3S
Date: Fri, 17 Feb 2023 23:41:15 +0530
From: Deepak R Varma <drv@mailo.com>
To: Julia Lawall <Julia.Lawall@inria.fr>,
Nicolas Palix <nicolas.palix@imag.fr>, cocci@inria.fr,
linux-kernel@vger.kernel.org
Cc: Saurabh Singh Sengar <ssengar@microsoft.com>,
Praveen Kumar <kumarpraveen@linux.microsoft.com>,
Deepak R Varma <drv@mailo.com>
Subject: [PATCH] coccinelle: put_device: Include of_node_put to avoid false
positives
Message-ID: <Y+/DQ6l0pDcxC2c3@ubun2204.myguest.virtualbox.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS
autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?=
X-GMAIL-THRID: =?utf-8?q?1758103454398479997?=
X-GMAIL-MSGID: =?utf-8?q?1758103454398479997?=
|
| Series |
coccinelle: put_device: Include of_node_put to avoid false positives
|
|
Commit Message
Deepak R Varma
Feb. 17, 2023, 6:11 p.m. UTC
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.
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(-)
Comments
> 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? 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
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? 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
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 > > >
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.")