Message-Id: <1481769711-14793-1-git-send-email-hofrat@osadl.org>
Date:   Thu, 15 Dec 2016 03:41:51 +0100
From:   Nicholas Mc Guire <hofrat@...dl.org>
To:     Julia Lawall <Julia.Lawall@...6.fr>
Cc:     Gilles Muller <Gilles.Muller@...6.fr>,
        Nicolas Palix <nicolas.palix@...g.fr>,
        Michal Marek <mmarek@...e.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Joe Perches <joe@...ches.com>, cocci@...teme.lip6.fr,
        linux-kernel@...r.kernel.org, Nicholas Mc Guire <hofrat@...dl.org>
Subject: [PATCH V2] Coccinelle: check usleep_range() usage

Documentation/timers/timers-howto.txt outlines the intended usage of 
usleep_range(), this spatch tries to locate missuse/out-of-spec cases.

Signed-off-by: Nicholas Mc Guire <hofrat@...dl.org>
---
V2: added context mode as suggested by Julia Lawall <julia.lawall@...6.fr>
    added min<max case sugested by Joe Perches <joe@...ches.com>
    added in the range checks as they are resonably reliable based on 
    a review of all 1648 call sites of usleep_range() 

1648 calls total                                                                
1488 pass numeric values only (90.29%)
  27 min below 10us (1.81%)
  40 min above 10ms (2.68%)
     min out of spec 4.50%
  76 preprocessor constants (4.61%)
   1 min below 10us (1.31%)
   8 min above 10ms (10.52%)
     min out of spec 11.84%
  85 expressions (5.15%)
1(0) min below 10us (1.50%)*
6(2) min above 10ms (7.50%)*
     min out of spec 9.0%
Errors:
  23 where min==max  (1.39%)
   0 where max < min (0.00%)

Total:
  Bugs: 6.48%-10.70%*
  Crit: 3.09%-3.15%* (min < 10, min==max, max < min)
  Detectable by coccinelle:
  Bugs: 74/103 (71.8%)
  Crit: 50/52 (96.1%)
* numbers estimated based on code review

Patch is againts 4.9.0 (localversion-next is next-20161214)

 scripts/coccinelle/api/bad_usleep_range.cocci | 88 +++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)
 create mode 100644 scripts/coccinelle/api/bad_usleep_range.cocci

diff --git a/scripts/coccinelle/api/bad_usleep_range.cocci b/scripts/coccinelle/api/bad_usleep_range.cocci
new file mode 100644
index 0000000..003e9ef
--- /dev/null
+++ b/scripts/coccinelle/api/bad_usleep_range.cocci
@@ -0,0 +1,88 @@
+/// report bad/problematic usleep_range usage
+//
+// This is a checker for the documented intended use of usleep_range
+// see: Documentation/timers/timers-howto.txt and
+// Link: http://lkml.org/lkml/2016/11/29/54 for some notes on
+//       when mdelay might not be a suitable replacement
+//
+// Limitations:
+//  * The numeric limits are only checked when numeric constants are in
+//    use (as of 4.9.0 thats 90.29% of the calls) no constant folding
+//    is done - so this can miss some out-of-range cases - but in 4.9.0
+//    it was catching 74 of the 103 bad cases (71.8%) and 50 of 52
+//    (96.1%) of the critical cases (min < 10 and min==max - there
+//  * There may be RT use-cases where both min < 10 and min==max)
+//    justified (e.g. high-throughput drivers on a shielded core)
+//
+// 1) warn if min == max
+//
+//  The problem is that usleep_range is calculating the delay by
+//      exp = ktime_add_us(ktime_get(), min)
+//      delta = (u64)(max - min) * NSEC_PER_USEC
+//  so delta is set to 0 if min==max
+//  and then calls
+//      schedule_hrtimeout_range(exp, 0,...)
+//  effectively this means that the clock subsystem has no room to
+//  optimize. usleep_range() is in non-atomic context so a 0 range
+//  makes very little sense as the task can be preempted anyway so
+//  there is no guarantee that the 0 range would be adding much
+//  precision - it just removes optimization potential, so it probably
+//  never really makes sense.
+//
+// 2) warn if min < 10 or min > 20ms
+//
+//  it makes little sense to use a non-atomic call for very short
+//  delays because the scheduling jitter will most likely exceed
+//  this limit - udelay() makes more sense in that case. For very
+//  large delays using hrtimers is useless as preemption becomes
+//  quite likely resulting in high inaccuracy anyway - so use
+//  jiffies based msleep and don't burden the hrtimer subsystem.
+//
+// 3) warn if max < min
+//
+//  Joe Perches <joe@...ches.com> added a check for this case
+//  that is definitely wrong.
+//
+// Confidence: Moderate
+// Copyright: (C) 2016 Nicholas Mc Guire, OSADL.  GPLv2.
+// Comments:
+// Options: --no-includes --include-headers
+
+virtual org
+virtual report
+virtual context
+
+@...lrangectx depends on context@
+expression E1,E2;
+position p;
+@@
+
+* usleep_range@p(E1,E2)
+
+
+@...lrange@
+expression E1,E2;
+position p;
+@@
+
+  usleep_range@p(E1,E2)
+
+@...ipt:python depends on !context@
+p << nullrange.p;
+min << nullrange.E1;
+max << nullrange.E2;
+@@
+
+if(min == max):
+   msg = "WARNING: usleep_range min == max (%s) - consider delta " % (min)
+   coccilib.report.print_report(p[0], msg)
+if str.isdigit(min):
+   if(int(min) < 10):
+      msg = "ERROR: usleep_range min (%s) less than 10us - consider using udelay()" % (min)
+      coccilib.report.print_report(p[0], msg)
+   if(20000 < int(min)):
+      msg = "ERROR: usleep_range min (%s) exceed 20m - consider using mslee()" % (min)
+      coccilib.report.print_report(p[0], msg)
+   if(int(max) < int(min)):
+      msg = "ERROR: usleep_range max (%s) less than min (%s)" % (max,min)
+      coccilib.report.print_report(p[0], msg)
-- 
2.1.4

Powered by blists - more mailing lists