From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753884AbdCIWkJ (ORCPT ); Thu, 9 Mar 2017 17:40:09 -0500 Received: from mx0a-00010702.pphosted.com ([148.163.156.75]:60269 "EHLO mx0b-00010702.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752628AbdCIWkH (ORCPT ); Thu, 9 Mar 2017 17:40:07 -0500 Date: Thu, 9 Mar 2017 15:37:24 -0600 From: Julia Cartwright To: Julia Lawall CC: Gilles Muller , Nicolas Palix , Michal Marek , , Thomas Gleixner , Sebastian Andrzej Siewior , Linus Walleij , Subject: Re: [PATCH 01/19] Coccinelle: locks: identify callers of spin_lock{,_irq,_irqsave}() in irqchip implementations Message-ID: <20170309213724.GA32162@jcartwri.amer.corp.natinst.com> References: <23190c912611c5d1d36529488e46706cdc3e4d87.1489015238.git.julia@ni.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="AqsLC8rIMeq19msA" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.7.2 (2016-11-26) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-03-09_18:,, signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1702020001 definitions=main-1703090157 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-03-09_18:,, signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=30 priorityscore=1501 malwarescore=0 suspectscore=2 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=30 reason=mlx scancount=1 engine=8.0.1-1702020001 definitions=main-1703090157 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --AqsLC8rIMeq19msA Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hello Julia- Thanks for the feedback. On Thu, Mar 09, 2017 at 09:15:21PM +0100, Julia Lawall wrote: > > +@match2 depends on match@ > > +identifier match.__irq_mask; > > +identifier data; > > +identifier x; > > +identifier l; > > +type T; > > +position j0; > > +expression flags; > > +@@ > > + static void __irq_mask(struct irq_data *data) > > + { > > + ... > > + T *x; > > + ... > > +( > > + spin_lock_irqsave(&x->l@j0, flags); > > +| > > + spin_lock_irq(&x->l@j0); > > +| > > + spin_lock(&x->l@j0); > > +) > > + ... > > + } >=20 > I guess that here you want a match if there is a lock anywhere in the > function? Most generally, yes. Any invocation of spin_lock{,_irq,_irqsave}() in the irq_mask callback of an irq_chip implementation (irq_mask is only _one_ such problematic callback, but a fairly representative one). I should probably introduce a more generic report-mode rule which more matches spin_lock{,_irq,_irqsave}(e), leaving this rule which requires the lock accessed through local pointer-indirection only used as a condition for patch mode. I'll play with this a bit. > Currently, the rule requires that the lock appear on every > control-flow path. If you put exists after depends on match in the rule > header, it will match if there exists a control-flow patch that contains a > local call. Thanks, this makes sense. > Also, ... matches the shortest path between the pattern before the ... and > the pattern after. Thus, x would have to be the first variable in the > function of pointer type. To eliminate this constraint, put when any on > each of the ...s. This will additionally allow more than one lock call in > the function. >=20 > All in all, I would suggest the following for this rule: >=20 > @match2 depends on match exists@ > identifier match.__irq_mask; > identifier data; > identifier x; > identifier l; > type T; > position j0; > expression flags; > @@ > static void __irq_mask(struct irq_data *data) > { > ... when any > T *x; > ... when any > ( > spin_lock_irqsave(&x->l@j0, flags); > | > spin_lock_irq(&x->l@j0); > | > spin_lock(&x->l@j0); > ) > ... when any > } Great, thanks, Julia! - The Other Julia --AqsLC8rIMeq19msA Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEEgKAEF431w1EL96k9jNrC4UVNdG8FAljByxAACgkQjNrC4UVN dG+mrQf/erCskflts0/LDFSn29LKTAnE0lHtqZAg6ApSg19M29zdfqe6ClceJU6y iXz9WhqXW8+cxoD9zrcSus5/utEKV9mLeVAE370U5cKFFdjHRxvANkdbChwpexW1 f7aYIz4UMxj2BelmajR2x2DAEuxu1JmePJb1zKwfrkAN3VLMKZqdIl5rOJxZrmIS FwGPPrf/+s9bnTViIsQkBEXQTpdjMNxbJk7IfX4tvVRxXNjpPORYSf9FFwHz6NWo EqhjfTt69Nk3TfWDYTZT04DBUSU/z3caVf809moXirVDdpNeE7wUQanE6wxVAnYW wflkwKXaXrSP0rNFgHEFD4om0r5j+g== =HwzR -----END PGP SIGNATURE----- --AqsLC8rIMeq19msA-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: julia@ni.com (Julia Cartwright) Date: Thu, 9 Mar 2017 15:37:24 -0600 Subject: [Cocci] [PATCH 01/19] Coccinelle: locks: identify callers of spin_lock{, _irq, _irqsave}() in irqchip implementations In-Reply-To: References: <23190c912611c5d1d36529488e46706cdc3e4d87.1489015238.git.julia@ni.com> Message-ID: <20170309213724.GA32162@jcartwri.amer.corp.natinst.com> To: cocci@systeme.lip6.fr List-Id: cocci@systeme.lip6.fr Hello Julia- Thanks for the feedback. On Thu, Mar 09, 2017 at 09:15:21PM +0100, Julia Lawall wrote: > > + at match2 depends on match@ > > +identifier match.__irq_mask; > > +identifier data; > > +identifier x; > > +identifier l; > > +type T; > > +position j0; > > +expression flags; > > +@@ > > + static void __irq_mask(struct irq_data *data) > > + { > > + ... > > + T *x; > > + ... > > +( > > + spin_lock_irqsave(&x->l at j0, flags); > > +| > > + spin_lock_irq(&x->l at j0); > > +| > > + spin_lock(&x->l at j0); > > +) > > + ... > > + } > > I guess that here you want a match if there is a lock anywhere in the > function? Most generally, yes. Any invocation of spin_lock{,_irq,_irqsave}() in the irq_mask callback of an irq_chip implementation (irq_mask is only _one_ such problematic callback, but a fairly representative one). I should probably introduce a more generic report-mode rule which more matches spin_lock{,_irq,_irqsave}(e), leaving this rule which requires the lock accessed through local pointer-indirection only used as a condition for patch mode. I'll play with this a bit. > Currently, the rule requires that the lock appear on every > control-flow path. If you put exists after depends on match in the rule > header, it will match if there exists a control-flow patch that contains a > local call. Thanks, this makes sense. > Also, ... matches the shortest path between the pattern before the ... and > the pattern after. Thus, x would have to be the first variable in the > function of pointer type. To eliminate this constraint, put when any on > each of the ...s. This will additionally allow more than one lock call in > the function. > > All in all, I would suggest the following for this rule: > > @match2 depends on match exists@ > identifier match.__irq_mask; > identifier data; > identifier x; > identifier l; > type T; > position j0; > expression flags; > @@ > static void __irq_mask(struct irq_data *data) > { > ... when any > T *x; > ... when any > ( > spin_lock_irqsave(&x->l at j0, flags); > | > spin_lock_irq(&x->l at j0); > | > spin_lock(&x->l at j0); > ) > ... when any > } Great, thanks, Julia! - The Other Julia -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 488 bytes Desc: not available URL: