Skip to content

Commit

Permalink
genirq/timings: Fix timings buffer inspection
Browse files Browse the repository at this point in the history
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: bbba0e7 "genirq/timings: Add array suffix computation code"
Signed-off-by: Daniel Lezcano <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: [email protected]
Link: https://lkml.kernel.org/r/[email protected]
  • Loading branch information
dlezcano authored and KAGA-KOKO committed Jun 12, 2019
1 parent 619c1ba commit 2840eef
Showing 1 changed file with 18 additions and 5 deletions.
23 changes: 18 additions & 5 deletions kernel/irq/timings.c
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -526,11 +543,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)
Expand Down

0 comments on commit 2840eef

Please sign in to comment.