From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 04ADCC4332F for ; Wed, 8 Nov 2023 09:50:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 616828D00B1; Wed, 8 Nov 2023 04:50:08 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 5C7248D00AD; Wed, 8 Nov 2023 04:50:08 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4B5908D00B1; Wed, 8 Nov 2023 04:50:08 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 3B1508D00AD for ; Wed, 8 Nov 2023 04:50:08 -0500 (EST) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 0DF2C8086E for ; Wed, 8 Nov 2023 09:50:08 +0000 (UTC) X-FDA: 81434316096.20.8C562BE Received: from mail2-relais-roc.national.inria.fr (mail2-relais-roc.national.inria.fr [192.134.164.83]) by imf14.hostedemail.com (Postfix) with ESMTP id 3891C10000D for ; Wed, 8 Nov 2023 09:50:04 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=inria.fr header.s=dc header.b=g7Roo5Zy; spf=pass (imf14.hostedemail.com: domain of julia.lawall@inria.fr designates 192.134.164.83 as permitted sender) smtp.mailfrom=julia.lawall@inria.fr; dmarc=pass (policy=none) header.from=inria.fr ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1699437005; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=HhxaM+9ct1KAcw8VIeJ+z4oy49b3afWo4vOTwZzrlic=; b=O4RJyIbbdlyPY141YM+iyC/CvanM4tgIw/9cSYowDqMCylvHnfKLi4DtzQ1lzzV4JWl4Qu zH8X1MMUgeln2EBJAJx97NsrwLB6CC6hLhcAYJlDaO9QPVIoRH8vydEpRBE3qJ1MGLpT05 j3emicLen6WUYenHRXydBQZb1nSiaWM= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=inria.fr header.s=dc header.b=g7Roo5Zy; spf=pass (imf14.hostedemail.com: domain of julia.lawall@inria.fr designates 192.134.164.83 as permitted sender) smtp.mailfrom=julia.lawall@inria.fr; dmarc=pass (policy=none) header.from=inria.fr ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1699437005; a=rsa-sha256; cv=none; b=p7QpeZ7Mii6aDpoRQwdgtm/D+0O1QaBD91SGMyj7WYh/sgJ4l773yOQEFhO1Hc7WKG/z7Z ElrixqHKbyxbESAIHJBPXhPFzX0fCVqfYPS3YfXgUpzaE6fv6LN0+2VBq8Fh9CjvtR9iq2 juRqa5FMAPmQIbztF4rTQTpMrB/S0GM= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=inria.fr; s=dc; h=date:from:to:cc:subject:in-reply-to:message-id: references:mime-version; bh=HhxaM+9ct1KAcw8VIeJ+z4oy49b3afWo4vOTwZzrlic=; b=g7Roo5ZyuocALj/3t3rO+3JAJsUh4HFvENaoQ+tK7ClXtLZlJAVtWNcC 8o8mQ/F9d8MrSwwp0nvHkjz/6lSP9qBeYJ3TVHiI83qKG9zJdReepaaiW jpeaqHjn9CGTVoL2kd0ADtxykV2AUwzzNU+JYr4VEy7/2z9tbw4JoGYd1 M=; X-IronPort-AV: E=Sophos;i="6.03,285,1694728800"; d="scan'208";a="135214412" Received: from 71-219-23-58.chvl.qwest.net (HELO hadrien) ([71.219.23.58]) by mail2-relais-roc.national.inria.fr with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Nov 2023 10:49:54 +0100 Date: Wed, 8 Nov 2023 04:49:52 -0500 (EST) From: Julia Lawall To: Ankur Arora cc: Julia Lawall , linux-kernel@vger.kernel.org, tglx@linutronix.de, peterz@infradead.org, torvalds@linux-foundation.org, paulmck@kernel.org, linux-mm@kvack.org, x86@kernel.org, akpm@linux-foundation.org, luto@kernel.org, bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com, mingo@redhat.com, juri.lelli@redhat.com, vincent.guittot@linaro.org, willy@infradead.org, mgorman@suse.de, jon.grimm@amd.com, bharata@amd.com, raghavendra.kt@amd.com, boris.ostrovsky@oracle.com, konrad.wilk@oracle.com, jgross@suse.com, andrew.cooper3@citrix.com, mingo@kernel.org, bristot@kernel.org, mathieu.desnoyers@efficios.com, geert@linux-m68k.org, glaubitz@physik.fu-berlin.de, anton.ivanov@cambridgegreys.com, mattst88@gmail.com, krypton@ulrich-teichert.org, rostedt@goodmis.org, David.Laight@ACULAB.COM, richard@nod.at, mjguzik@gmail.com, Nicolas Palix Subject: Re: [RFC PATCH 57/86] coccinelle: script to remove cond_resched() In-Reply-To: <875y2cr6ll.fsf@oracle.com> Message-ID: <43db286-7c8f-5c4-3f11-b8c103092c@inria.fr> References: <20231107215742.363031-1-ankur.a.arora@oracle.com> <20231107230822.371443-1-ankur.a.arora@oracle.com> <875y2cr6ll.fsf@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Rspamd-Queue-Id: 3891C10000D X-Rspam-User: X-Stat-Signature: 8me68nbagjddnwqticp5m7xn5ci3uycd X-Rspamd-Server: rspam01 X-HE-Tag: 1699437004-224784 X-HE-Meta: U2FsdGVkX19jbTYQ2hPr+NZAMzG+97EBI6Ha9N9ZfIuWQRq2BPj0qxjMB1heqYTBOF0gbH9ii/6q+dzTee2MphunuMspr1Py55LcMxyo7UwZQdG7mJE2zAlhxhC4jC944IH+tO7xG1vNDie4QSdPmwOpDw0KZPqctsGK6CQbEfluVjAHNUUfj9vI/4WZ39khD/kWsUdeLv5NpkEFAzlnVvvvgb/cwPWre3EFuvFQShw7IE+0sR0X5Er/FdVlc3THo48DbIRJ4dMYF8AiUKXWGQnylRyanreWALmkrd1LoG/HWr339pHXeb3lt0sJQCqAjo/MuPyHlxWvwUWjpe599cLmrgq5vw3AVkZx8PnM9oeNEX42SqG6kiIn2ivOEbbeNi1DX2ElkrelNiugu+17xi0kb+wpb6PZxLioVCQNuPwroxdZGOBHpYy3ra5T9cs9L0ILi8wgqdFhq77I1t8gZenCDJY4zYXU8gnfLEGWKRqCUVR/XhZL/bfShNVlSWynsjRJisH3eQ7ewQAL4h3Y+Cd+R0KTUSVC40Xz+kH6E/wt494a+wuEkqs5QO5Y/3N5Uw4n6kr03DXZ+efTRLtAN93/5dL7waHzcSxqK8u63Yx/rwU3spwtrUTznonlons2vVSebR8wST2kfBFiExat0tJ5TYLVI2Z7WMXnj8ctJZU0gmi9yK43Adq5qnoSaaXQC9132bdPWJOj6kdJq1k0YV352yaRyzb30zNr9zlSBu9tEH1qyRSof82fjyw5sfyirb1WFSwCEK5jN2clX5tB8b4is0HncHnhp0asjnSFTdILJHY7Z/WyXPskdclUAqe08DpLszoM46B7kqkMVFefa2PmUsXGDaPW77788dQc1moo0iM690/1rAY0ot7eHPeoDN942THVvPUl9oo9ByRQs1u0EtnbWowryi/p4NjxVjem1y8HPfyyRKqVKBAWtqh+/8r0QUt12SJ3iIvacPL CdP7yHes i3VIsuYmqPqsyWTqyi5/QYk4uSkJp9B2+8XbYQyAYrI1mezcQYCaeoyDASRersz8CZoI7iu7+eBZmtQULq+QbfGhR2Jjr0lASdxMc6sFKfg5ET2azw8b81hOhtzMR62GQjEsftwIFWfDNV5a9UoU3/sKbt2afOgp7+PxccktLmT5XiICIQN4VqIBu+aZCEkyWKFj+a3zvBzvz9Sw= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Wed, 8 Nov 2023, Ankur Arora wrote: > > Julia Lawall writes: > > > On Tue, 7 Nov 2023, Ankur Arora wrote: > > > >> Rudimentary script to remove the straight-forward subset of > >> cond_resched() and allies: > >> > >> 1) if (need_resched()) > >> cond_resched() > >> > >> 2) expression*; > >> cond_resched(); /* or in the reverse order */ > >> > >> 3) if (expression) > >> statement > >> cond_resched(); /* or in the reverse order */ > >> > >> The last two patterns depend on the control flow level to ensure > >> that the complex cond_resched() patterns (ex. conditioned ones) > >> are left alone and we only pick up ones which are only minimally > >> related the neighbouring code. > >> > >> Cc: Julia Lawall > >> Cc: Nicolas Palix > >> Signed-off-by: Ankur Arora > >> --- > >> scripts/coccinelle/api/cond_resched.cocci | 53 +++++++++++++++++++++++ > >> 1 file changed, 53 insertions(+) > >> create mode 100644 scripts/coccinelle/api/cond_resched.cocci > >> > >> diff --git a/scripts/coccinelle/api/cond_resched.cocci b/scripts/coccinelle/api/cond_resched.cocci > >> new file mode 100644 > >> index 000000000000..bf43768a8f8c > >> --- /dev/null > >> +++ b/scripts/coccinelle/api/cond_resched.cocci > >> @@ -0,0 +1,53 @@ > >> +// SPDX-License-Identifier: GPL-2.0-only > >> +/// Remove naked cond_resched() statements > >> +/// > >> +//# Remove cond_resched() statements when: > >> +//# - executing at the same control flow level as the previous or the > >> +//# next statement (this lets us avoid complicated conditionals in > >> +//# the neighbourhood.) > >> +//# - they are of the form "if (need_resched()) cond_resched()" which > >> +//# is always safe. > >> +//# > >> +//# Coccinelle generally takes care of comments in the immediate neighbourhood > >> +//# but might need to handle other comments alluding to rescheduling. > >> +//# > >> +virtual patch > >> +virtual context > >> + > >> +@ r1 @ > >> +identifier r; > >> +@@ > >> + > >> +( > >> + r = cond_resched(); > >> +| > >> +-if (need_resched()) > >> +- cond_resched(); > >> +) > > > > This rule doesn't make sense. The first branch of the disjunction will > > never match a a place where the second branch matches. Anyway, in the > > second branch there is no assignment, so I don't see what the first branch > > is protecting against. > > > > The disjunction is just useless. Whether it is there or or whether only > > the second brancha is there, doesn't have any impact on the result. > > > >> + > >> +@ r2 @ > >> +expression E; > >> +statement S,T; > >> +@@ > >> +( > >> + E; > >> +| > >> + if (E) S > > > > This case is not needed. It will be matched by the next case. > > > >> +| > >> + if (E) S else T > >> +| > >> +) > >> +-cond_resched(); > >> + > >> +@ r3 @ > >> +expression E; > >> +statement S,T; > >> +@@ > >> +-cond_resched(); > >> +( > >> + E; > >> +| > >> + if (E) S > > > > As above. > > > >> +| > >> + if (E) S else T > >> +) > > > > I have the impression that you are trying to retain some cond_rescheds. > > Could you send an example of one that you are trying to keep? Overall, > > the above rules seem a bit ad hoc. You may be keeping some cases you > > don't want to, or removing some cases that you want to keep. > > Right. I was trying to ensure that the script only handled the cases > that didn't have any "interesting" connections to the surrounding code. > > Just to give you an example of the kind of constructs that I wanted > to avoid: > > mm/memoy.c::zap_pmd_range(): > > if (addr != next) > pmd--; > } while (pmd++, cond_resched(), addr != end); > > mm/backing-dev.c::cleanup_offline_cgwbs_workfn() > > while (cleanup_offline_cgwb(wb)) > cond_resched(); > > > while (cleanup_offline_cgwb(wb)) > cond_resched(); > > But from a quick check the simplest coccinelle script does a much > better job than my overly complex (and incorrect) one: > > @r1@ > @@ > - cond_resched(); > > It avoids the first one. And transforms the second to: > > while (cleanup_offline_cgwb(wb)) > {} > > which is exactly what I wanted. Perfect! It could be good to run both scripts and compare the results. julia > > > Of course, if you are confident that the job is done with this semantic > > patch as it is, then that's fine too. > > Not at all. Thanks for pointing out the mistakes. > > > > -- > ankur >