From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752052AbdH2JXL (ORCPT ); Tue, 29 Aug 2017 05:23:11 -0400 Received: from mail3-relais-sop.national.inria.fr ([192.134.164.104]:28638 "EHLO mail3-relais-sop.national.inria.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751715AbdH2JXE (ORCPT ); Tue, 29 Aug 2017 05:23:04 -0400 X-IronPort-AV: E=Sophos;i="5.41,444,1498514400"; d="scan'208";a="235599331" Date: Tue, 29 Aug 2017 11:23:01 +0200 (CEST) From: Julia Lawall X-X-Sender: jll@hadrien To: "Reshetova, Elena" cc: "linux-kernel@vger.kernel.org" , "cocci@systeme.lip6.fr" , "Gilles.Muller@lip6.fr" , "nicolas.palix@imag.fr" , "mmarek@suse.com" , "keescook@chromium.org" , "ishkamiel@gmail.com" Subject: RE: [PATCH v3] provide rule for finding refcounters In-Reply-To: <2236FBA76BA1254E88B949DDB74E612B6FF5F9AD@IRSMSX102.ger.corp.intel.com> Message-ID: References: <1502884342-10702-1-git-send-email-elena.reshetova@intel.com> <2236FBA76BA1254E88B949DDB74E612B6FF5F9AD@IRSMSX102.ger.corp.intel.com> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 29 Aug 2017, Reshetova, Elena wrote: > Hi, I am very sorry for the delayed reply. Finally unrigging my inbox :( > > > A few more small issues: > > > > When you deleted the disjunction, you kept the surrounding parentheses. > > you can drop them (lines 83 and 85). > > > > I guess that the "del" regular expression is supposed to be matching > > delete. But it also matches delayed, eg > > > > net/batman-adv/bridge_loop_avoidance.c:1495:8-27: > > atomic_dec_and_test variation before object free at line 1507. > > Actually the idea is to match them both :) "delete" because it is obvious, > "delay", exactly because "queue_delayed_work" (in addition to "queue_work") is a common way some > structure destruction might be scheduled. It might give false positives, since > the queued work might not be related to freeing the object, but at least > we don't miss such cases. The issue also that you do want to have "del" pattern > since I think some functions are of kind xyz_del() also and I want to catch them as well. > Of course del then might catch some other non-queue related "delay", but I haven't seen that many > to consider it a problem. > > > > > In the following result, the lines are at least quite far apart. I don't > > know if there is some way to consider this to be a false positive: > > > > fs/btrfs/disk-io.c:708:14-33: atomic_dec_and_test > > variation before object free at line 775. > > I actually think this is a valid result. Yes, there are a lot of things happening > in between the dec_and_test and actual free on the buffer, but kernel structures > can be so complicated that it might legitimately (like in this case) take that long > for it to cleanup before the real free can be done. OK, if you are happy with the results of the regexps, I think that the only remaining improvement was an extra pair of () where there was no longer a disjuction. If you want to send it back, then I can ack it. julia > > Best Regards, > Elena. > > > > > julia > > > > On Wed, 16 Aug 2017, Elena Reshetova wrote: > > > > > changes in v3: > > > Removed unnessesary rule 4 conditions pointed by Julia. > > > > > > changes in v2: > > > Following the suggestion from Julia the first rule is split into > > > 2. The output does not differ that much between these two versions, > > > but rule became more precise. > > > > > > Elena Reshetova (1): > > > Coccinelle: add atomic_as_refcounter script > > > > > > scripts/coccinelle/api/atomic_as_refcounter.cocci | 133 > > ++++++++++++++++++++++ > > > 1 file changed, 133 insertions(+) > > > create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci > > > > > > -- > > > 2.7.4 > > > > > > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: julia.lawall@lip6.fr (Julia Lawall) Date: Tue, 29 Aug 2017 11:23:01 +0200 (CEST) Subject: [Cocci] [PATCH v3] provide rule for finding refcounters In-Reply-To: <2236FBA76BA1254E88B949DDB74E612B6FF5F9AD@IRSMSX102.ger.corp.intel.com> References: <1502884342-10702-1-git-send-email-elena.reshetova@intel.com> <2236FBA76BA1254E88B949DDB74E612B6FF5F9AD@IRSMSX102.ger.corp.intel.com> Message-ID: To: cocci@systeme.lip6.fr List-Id: cocci@systeme.lip6.fr On Tue, 29 Aug 2017, Reshetova, Elena wrote: > Hi, I am very sorry for the delayed reply. Finally unrigging my inbox :( > > > A few more small issues: > > > > When you deleted the disjunction, you kept the surrounding parentheses. > > you can drop them (lines 83 and 85). > > > > I guess that the "del" regular expression is supposed to be matching > > delete. But it also matches delayed, eg > > > > net/batman-adv/bridge_loop_avoidance.c:1495:8-27: > > atomic_dec_and_test variation before object free at line 1507. > > Actually the idea is to match them both :) "delete" because it is obvious, > "delay", exactly because "queue_delayed_work" (in addition to "queue_work") is a common way some > structure destruction might be scheduled. It might give false positives, since > the queued work might not be related to freeing the object, but at least > we don't miss such cases. The issue also that you do want to have "del" pattern > since I think some functions are of kind xyz_del() also and I want to catch them as well. > Of course del then might catch some other non-queue related "delay", but I haven't seen that many > to consider it a problem. > > > > > In the following result, the lines are at least quite far apart. I don't > > know if there is some way to consider this to be a false positive: > > > > fs/btrfs/disk-io.c:708:14-33: atomic_dec_and_test > > variation before object free at line 775. > > I actually think this is a valid result. Yes, there are a lot of things happening > in between the dec_and_test and actual free on the buffer, but kernel structures > can be so complicated that it might legitimately (like in this case) take that long > for it to cleanup before the real free can be done. OK, if you are happy with the results of the regexps, I think that the only remaining improvement was an extra pair of () where there was no longer a disjuction. If you want to send it back, then I can ack it. julia > > Best Regards, > Elena. > > > > > julia > > > > On Wed, 16 Aug 2017, Elena Reshetova wrote: > > > > > changes in v3: > > > Removed unnessesary rule 4 conditions pointed by Julia. > > > > > > changes in v2: > > > Following the suggestion from Julia the first rule is split into > > > 2. The output does not differ that much between these two versions, > > > but rule became more precise. > > > > > > Elena Reshetova (1): > > > Coccinelle: add atomic_as_refcounter script > > > > > > scripts/coccinelle/api/atomic_as_refcounter.cocci | 133 > > ++++++++++++++++++++++ > > > 1 file changed, 133 insertions(+) > > > create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci > > > > > > -- > > > 2.7.4 > > > > > > >