All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org, andriy.shevchenko@linux.intel.com
Subject: [PATCH V2 2/9] genirq/timings: Fix timings buffer inspection
Date: Fri, 24 May 2019 13:16:08 +0200	[thread overview]
Message-ID: <20190524111615.4891-3-daniel.lezcano@linaro.org> (raw)
In-Reply-To: <20190524111615.4891-1-daniel.lezcano@linaro.org>

It appears the index beginning computation is not correct, the current
code does:

     i = (irqts->count & IRQ_TIMINGS_MASK) - 1

If irqts->count is equal to zero, we end up with an index equal to -1,
but that does not happen because the function checks against zero
before and returns in such case.

However, if irqts->count is a multiple of IRQ_TIMINGS_SIZE, the
resulting & bit op will be zero and leads also to a -1 index.

Re-introduce the iteration loop belonging to the previous variance
code which was correct.

Fixes: bbba0e7c5cda "genirq/timings: Add array suffix computation code"
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 kernel/irq/timings.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/kernel/irq/timings.c b/kernel/irq/timings.c
index 60362aca4ca4..250bb00ccd85 100644
--- a/kernel/irq/timings.c
+++ b/kernel/irq/timings.c
@@ -267,6 +267,23 @@ void irq_timings_disable(void)
 #define PREDICTION_MAX		10 /* 2 ^ PREDICTION_MAX useconds */
 #define PREDICTION_BUFFER_SIZE	16 /* slots for EMAs, hardly more than 16 */
 
+/*
+ * Number of elements in the circular buffer: If it happens it was
+ * flushed before, then the number of elements could be smaller than
+ * IRQ_TIMINGS_SIZE, so the count is used, otherwise the array size is
+ * used as we wrapped. The index begins from zero when we did not
+ * wrap. That could be done in a nicer way with the proper circular
+ * array structure type but with the cost of extra computation in the
+ * interrupt handler hot path. We choose efficiency.
+ */
+#define for_each_irqts(i, irqts)					\
+	for (i = irqts->count < IRQ_TIMINGS_SIZE ?			\
+		     0 : irqts->count & IRQ_TIMINGS_MASK,		\
+		     irqts->count = min(IRQ_TIMINGS_SIZE,		\
+					irqts->count);			\
+	     irqts->count > 0; irqts->count--,				\
+		     i = (i + 1) & IRQ_TIMINGS_MASK)
+
 struct irqt_stat {
 	u64	last_ts;
 	u64	ema_time[PREDICTION_BUFFER_SIZE];
@@ -528,11 +545,7 @@ u64 irq_timings_next_event(u64 now)
 	 * model while decrementing the counter because we consume the
 	 * data from our circular buffer.
 	 */
-
-	i = (irqts->count & IRQ_TIMINGS_MASK) - 1;
-	irqts->count = min(IRQ_TIMINGS_SIZE, irqts->count);
-
-	for (; irqts->count > 0; irqts->count--, i = (i + 1) & IRQ_TIMINGS_MASK) {
+	for_each_irqts(i, irqts) {
 		irq = irq_timing_decode(irqts->values[i], &ts);
 		s = idr_find(&irqt_stats, irq);
 		if (s)
-- 
2.17.1


  parent reply	other threads:[~2019-05-24 11:17 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-24 11:16 [PATCH V2 0/9] genirq/timings: Fixes and selftests Daniel Lezcano
2019-05-24 11:16 ` [PATCH V2 1/9] genirq/timings: Fix next event index function Daniel Lezcano
2019-05-24 13:51   ` Andy Shevchenko
2019-05-24 11:16 ` Daniel Lezcano [this message]
2019-05-24 11:16 ` [PATCH V2 3/9] genirq/timings: Optimize the period detection speed Daniel Lezcano
2019-05-24 11:16 ` [PATCH V2 4/9] genirq/timings: Use the min kernel macro Daniel Lezcano
2019-05-24 13:57   ` Andy Shevchenko
2019-05-24 16:11     ` Daniel Lezcano
2019-05-24 11:16 ` [PATCH V2 5/9] genirq/timings: Encapsulate timings push Daniel Lezcano
2019-05-24 11:16 ` [PATCH V2 6/9] genirq/timings: Encapsulate storing function Daniel Lezcano
2019-05-24 11:16 ` [PATCH V2 7/9] genirq/timings: Add selftest for circular array Daniel Lezcano
2019-05-24 14:00   ` Andy Shevchenko
2019-05-24 16:00     ` Daniel Lezcano
2019-05-24 11:16 ` [PATCH V2 8/9] genirq/timings: Add selftest for irqs circular buffer Daniel Lezcano
2019-05-24 11:16 ` [PATCH V2 9/9] genirq/timings: Add selftest for next event computation Daniel Lezcano

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=20190524111615.4891-3-daniel.lezcano@linaro.org \
    --to=daniel.lezcano@linaro.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.