Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1439517

Re: [PATCH 4/4] irqtime: drop local_irq_save/restore from irqtime_account_irq

From Paolo Bonzini <pbonzini@redhat.com>
Newsgroups linux.kernel
Subject Re: [PATCH 4/4] irqtime: drop local_irq_save/restore from irqtime_account_irq
Date 2016-07-08 16:40 +0200
Message-ID <rSF6F-60Q-27@gated-at.bofh.it> (permalink)
References <rPPYB-58Z-9@gated-at.bofh.it> <rPPYB-58Z-11@gated-at.bofh.it> <rSDex-4Mm-5@gated-at.bofh.it> <rSE0V-5l8-13@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


[Multipart message — attachments visible in raw view] - view raw

On 08/07/2016 15:19, Rik van Riel wrote:
> On Fri, 2016-07-08 at 14:30 +0200, Frederic Weisbecker wrote:
>> On Thu, Jun 30, 2016 at 03:35:50PM -0400, riel@redhat.com wrote:
>>> From: Rik van Riel <riel@redhat.com>
>>>
>>> Drop local_irq_save/restore from irqtime_account_irq.
>>> Instead, have softirq and hardirq track their time spent
>>> independently, with the softirq code subtracting hardirq
>>> time that happened during the duration of the softirq run.
>>>
>>> The softirq code can be interrupted by hardirq code at
>>> any point in time, but it can check whether it got a
>>> consistent snapshot of the timekeeping variables it wants,
>>> and loop around in the unlikely case that it did not.
>>>
>>> Signed-off-by: Rik van Riel <riel@redhat.com>
>>
>> So the purpose is to get rid of local_irq_save/restore()?
>> Is it really worth such complication?
> 
> local_irq_save/restore are quite slow, and look like the
> largest source of overhead in irq time accounting.

I'm looking at an upstream tree, without your patches applied,
but it seems to me that irqtime_account_irq is always called with
interrupts disabled:

irqtime_account_irq
	-> account_irq_enter_time
		-> __irq_enter
			-> HARDIRQ_ENTER	[1]
			-> irq_enter		[3]
		-> __do_softirq			[1]
	-> account_irq_exit_time
		-> __do_softirq			[1]
		-> __irq_exit
			-> HARDIRQ_EXIT		[1]
		-> irq_exit			[2]

	[1] = does local_irq_disable/enable
	[2] = contains WARN_ON_ONCE(!irqs_disabled())
	[3] = calls rcu_irq_enter(), which checks irqs_disabled()

I don't think your first two patches change this, so perhaps it's enough
to remove that local_irq-save/restore?  Either this, or ENEEDWEEKEND...

Paolo

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 4/4] irqtime: drop local_irq_save/restore from irqtime_account_irq riel@redhat.com - 2016-06-30 21:40 +0200
  Re: [PATCH 4/4] irqtime: drop local_irq_save/restore from  irqtime_account_irq Frederic Weisbecker <fweisbec@gmail.com> - 2016-07-08 14:40 +0200
    Re: [PATCH 4/4] irqtime: drop local_irq_save/restore from  irqtime_account_irq Rik van Riel <riel@redhat.com> - 2016-07-08 15:30 +0200
      Re: [PATCH 4/4] irqtime: drop local_irq_save/restore from  irqtime_account_irq Frederic Weisbecker <fweisbec@gmail.com> - 2016-07-08 16:10 +0200
      Re: [PATCH 4/4] irqtime: drop local_irq_save/restore from  irqtime_account_irq Paolo Bonzini <pbonzini@redhat.com> - 2016-07-08 16:40 +0200
        Re: [PATCH 4/4] irqtime: drop local_irq_save/restore from  irqtime_account_irq Rik van Riel <riel@redhat.com> - 2016-07-08 18:00 +0200
          Re: [PATCH 4/4] irqtime: drop local_irq_save/restore from  irqtime_account_irq Frederic Weisbecker <fweisbec@gmail.com> - 2016-07-09 02:00 +0200

csiph-web