From: Nicholas Mc Guire <hofrat@osadl.org>
To: Julia Lawall <Julia.Lawall@lip6.fr>
Cc: Gilles Muller <Gilles.Muller@lip6.fr>,
Nicolas Palix <nicolas.palix@imag.fr>,
Michal Marek <mmarek@suse.com>, Joe Perches <joe@perches.com>,
cocci@systeme.lip6.fr, linux-kernel@vger.kernel.org,
Nicholas Mc Guire <hofrat@osadl.org>
Subject: [PATCH RFC] coccinelle: flag double conversions in msleep*
Date: Thu, 7 Jan 2016 21:17:58 +0100 [thread overview]
Message-ID: <1452197878-26296-1-git-send-email-hofrat@osadl.org> (raw)
This spatch flags cases where msleep/msleep_interruptible converts the
timeout parameter with jiffies_to_msecs(). msleep* performs a conversion
of the milliseconds passed in back to jiffies and then calls the
appropriate schedule_timeout* variant.
In cases where the timeout is constant, there is effectively no
difference with respect to the generated code (atleast not for the x86
.lst files of a few cases checked) but for cases where the timeout is
non-constant there is an actual call to jiffies_to_msecs() at runtime,
that overhead can be removed.
Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---
As of linux-next next-20160107 it finds 24 cases in the kernel
An example of where this makes a slight code difference is
drivers/scsi/osst.c:osst_wait_ready()
< if (initial_delay > 0)
< 2293: 7e 0f jle 22a4 <osst_wait_ready+0x34>
< msleep(jiffies_to_msecs(initial_delay));
< 2295: 48 63 f9 movslq %ecx,%rdi
< 2298: e8 00 00 00 00 callq 229d <osst_wait_ready+0x2d>
< 2299: R_X86_64_PC32 jiffies_to_msecs-0x4
< 229d: 89 c7 mov %eax,%edi
< 229f: e8 00 00 00 00 callq 22a4 <osst_wait_ready+0x34>
< 22a0: R_X86_64_PC32 msleep-0x4
<
< memset(cmd, 0, MAX_COMMAND_SIZE);
vs:
> if (initial_delay > 0)
> 2293: 7e 08 jle 229d <osst_wait_ready+0x2d>
> schedule_timeout_uninterruptible(initial_delay);
> 2295: 48 63 f9 movslq %ecx,%rdi
> 2298: e8 00 00 00 00 callq 229d <osst_wait_ready+0x2d>
> 2299: R_X86_64_PC32 schedule_timeout_uninterruptible-0x4
>
> memset(cmd, 0, MAX_COMMAND_SIZE);
This patch was tested with coccinelle version 1.0.4
Confidence: Medium as I did not find any false positives for the 24
cases that were found - some of the patches generated do not actually
improve readability though (e.g. drivers/i2c/busses/i2c-highlander.c)
but technically it is not incorrect ither.
patch is against linux-next (localversion-next is next-20160107)
scripts/coccinelle/api/msleep_double_convert.cocci | 63 ++++++++++++++++++++++
1 file changed, 63 insertions(+)
create mode 100644 scripts/coccinelle/api/msleep_double_convert.cocci
diff --git a/scripts/coccinelle/api/msleep_double_convert.cocci b/scripts/coccinelle/api/msleep_double_convert.cocci
new file mode 100644
index 0000000..b39dd36
--- /dev/null
+++ b/scripts/coccinelle/api/msleep_double_convert.cocci
@@ -0,0 +1,63 @@
+/// Check for double conversion of timeouts values
+//
+// for constants this actually makes little sense - other than
+// for readability - in cases where non-constants are involved there is
+// a call to jiffies_to_msecs that is saved here.
+//
+// Confidence: Medium
+// Copyright: (C) 2015 Nicholas Mc Guire <hofrat@osadl.org>, OSADL, GPL v2
+// URL: http://coccinelle.lip6.fr
+// Options: --no-includes --include-headers
+
+virtual org
+virtual patch
+virtual report
+
+@depends on patch@
+constant C;
+expression E;
+@@
+
+(
+- msleep_interruptible(jiffies_to_msecs(C))
++ schedule_timeout_interruptible(C)
+|
+- msleep_interruptible(jiffies_to_msecs(E))
++ schedule_timeout_interruptible(E)
+|
+- msleep(jiffies_to_msecs(C))
++ schedule_timeout(C)
+|
+- msleep(jiffies_to_msecs(E))
++ schedule_timeout(E)
+)
+
+@cc depends on !patch && (org || report)@
+constant C;
+expression E;
+position p;
+@@
+
+(
+* msleep_interruptible@p(jiffies_to_msecs(C))
+|
+* msleep_interruptible@p(jiffies_to_msecs(E))
+|
+* msleep@p(jiffies_to_msecs(C))
+|
+* msleep@p(jiffies_to_msecs(E))
+)
+
+@script:python depends on org@
+p << cc.p;
+@@
+
+msg = "INFO: schedule_timeout*() preferred if timeout in jiffies"
+coccilib.org.print_safe_todo(p[0], msg)
+
+@script:python depends on report@
+p << cc.p;
+@@
+
+msg = "INFO: schedule_timeout*() preferred if timeout in jiffies"
+coccilib.report.print_report(p[0], msg)
--
2.1.4
next reply other threads:[~2016-01-07 20:22 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-07 20:17 Nicholas Mc Guire [this message]
2016-01-07 21:24 ` Julia Lawall
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=1452197878-26296-1-git-send-email-hofrat@osadl.org \
--to=hofrat@osadl.org \
--cc=Gilles.Muller@lip6.fr \
--cc=Julia.Lawall@lip6.fr \
--cc=cocci@systeme.lip6.fr \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mmarek@suse.com \
--cc=nicolas.palix@imag.fr \
/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
all inboxes | Powered by JetHome®