mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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

             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®