Re: [PATCH 1/4] x86/IRQ: don't keep EOI timer running without need

From: "Jan Beulich" <JBeulich@suse.com>
To: Roger Pau Monne <roger.pau@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
	Julien Grall <julien.grall@arm.com>,
	xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH 1/4] x86/IRQ: don't keep EOI timer running without need
Date: Thu, 16 May 2019 04:50:22 -0600	[thread overview]
Message-ID: <5CDD406E020000780022F906@prv1-mh.provo.novell.com> (raw)
In-Reply-To: <20190516103238.pm6ducegde3xw5wz@Air-de-Roger>

>>> On 16.05.19 at 12:32, <roger.pau@citrix.com> wrote:
> On Wed, May 08, 2019 at 06:46:25AM -0600, Jan Beulich wrote:
>> The timer needs to remain active only until all pending IRQ instances
>> have seen EOIs from their respective domains. Stop it when the in-flight
>> count has reached zero in desc_guest_eoi(). Note that this is race free
>> (with __do_IRQ_guest()), as the IRQ descriptor lock is being held at
>> that point.
>> 
>> Also pull up stopping of the timer in __do_IRQ_guest() itself: Instead
>> of stopping it immediately before re-setting, stop it as soon as we've
>> made it past any early returns from the function (and hence we're sure
>> it'll get set again).
>> 
>> Finally bail from the actual timer handler in case we find the timer
>> already active again by the time we've managed to acquire the IRQ
>> descriptor lock. Without this we may forcibly EOI an IRQ immediately
>> after it got sent to a guest. For this, timer_is_active() gets split out
>> of active_timer(), deliberately moving just one of the two ASSERT()s (to
>> allow the function to be used also on a never initialized timer).
> 
> AFAICT timer_is_active is exclusively used in irq_guest_eoi_timer_fn,
> which must have initialized the timer in order for
> irq_guest_eoi_timer_fn to be called, and hence I'm not sure why you
> need to be able to call timer_is_active with an uninitialized timer.

It's not needed here, but I consider this useful behavior when used
outside of the specific timer's handler.

> Is this maybe used by other patches?

None that I would have in the works.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel