public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: nicolas.pitre@linaro.org, shreyas@linux.vnet.ibm.com,
	linux-kernel@vger.kernel.org, peterz@infradead.org,
	rafael@kernel.org, vincent.guittot@linaro.org
Subject: Re: [PATCH V7] irq: Track the interrupt timings
Date: Thu, 23 Jun 2016 11:39:11 +0200	[thread overview]
Message-ID: <576BAE3F.5030603@linaro.org> (raw)
In-Reply-To: <alpine.DEB.2.11.1606231037380.5839@nanos>

On 06/23/2016 10:41 AM, Thomas Gleixner wrote:
> On Fri, 17 Jun 2016, Daniel Lezcano wrote:
>> The interrupt framework gives a lot of information about each interrupt.
>>
>> It does not keep track of when those interrupts occur though.
>>
>> This patch provides a mean to record the elapsed time between successive
>> interrupt occurrences in a per-IRQ per-CPU circular buffer to help with the
>> prediction of the next occurrence using a statistical model.
>>
>> A new function is added to browse the different interrupts and retrieve the
>> timing information stored in it.
>>
>> A static key is introduced so when the irq prediction is switched off at
>> runtime, we can reduce the overhead near to zero. The irq timings is
>> supposed to be potentially used by different sub-systems and for this reason
>> the static key is a ref counter, so when the last use releases the irq
>> timings that will result on the effective deactivation of the irq measurement.
>
> Before merging this I really have to ask a few more questions. I'm a bit
> worried about the usage site of this. It's going to iterate over all
> interrupts in the system to do a next interrupt prediction. On larger machines
> that's going to be quite some work and you touch a gazillion of cache lines
> and many of them just to figure out that nothing happened.
>
> Is it really required to do this per interrupt rather than providing per cpu
> statistics of interrupts which arrived in the last X seconds or whatever
> timeframe is relevant for this.

Perhaps I am misunderstanding but if the statistics are done per cpu 
without tracking per irq timings, it is not possible to extract a 
repeating pattern for each irq and have an accurate prediction.

Today, the code stores per cpu and per irq timings and the usage is to 
compute the next irq event by taking the earliest next irq event on the 
current cpu.

@@ -51,6 +52,9 @@ struct irq_desc {
         struct irq_data         irq_data;
         unsigned int __percpu   *kstat_irqs;
         irq_flow_handler_t      handle_irq;
+#ifdef CONFIG_IRQ_TIMINGS
+       struct irq_timings __percpu *timings;
+#endif
  #ifdef CONFIG_IRQ_PREFLOW_FASTEOI
         irq_preflow_handler_t   preflow_handler;
  #endif

If we step back and look at the potential users of this framework, we have:

  - mobile: by nature the interrupt line number is small and the devices 
are "slow"

  - desktop and laptop : a few interrupts are really interesting us, 
ethernet and sdd (the other ones are rare, or ignored like timers or IPI)

  - server : the interrupt line number is bigger, but not so much.

  - other big system: I don't know

Usually, server and super sized system want full performance and low 
latency. For this reason the kernel is configured with periodic tick and 
that makes the next prediction algorithm superfluous, especially when 
the latency is set to 0. So I don't think the irq timings + next irq 
event code path will be ever used in this case.

As you mentioned it, there are some parts we can make evolve and 
optimize like preventing to lookup an empty irq events cpu.





-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

  reply	other threads:[~2016-06-23  9:39 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-14 16:33 [PATCH V5] irq: Track the interrupt timings Daniel Lezcano
2016-06-14 17:46 ` Nicolas Pitre
2016-06-14 18:11   ` Thomas Gleixner
2016-06-14 19:52     ` Daniel Lezcano
2016-06-14 20:10     ` [PATCH V6] " Daniel Lezcano
2016-06-14 20:38       ` Nicolas Pitre
2016-06-17 13:46       ` Thomas Gleixner
2016-06-17 17:16         ` [PATCH V7] " Daniel Lezcano
2016-06-23  8:41           ` Thomas Gleixner
2016-06-23  9:39             ` Daniel Lezcano [this message]
2016-06-23 10:12               ` Thomas Gleixner
2016-06-23 13:12                 ` 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=576BAE3F.5030603@linaro.org \
    --to=daniel.lezcano@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nicolas.pitre@linaro.org \
    --cc=peterz@infradead.org \
    --cc=rafael@kernel.org \
    --cc=shreyas@linux.vnet.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=vincent.guittot@linaro.org \
    /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