Re: [Xen-devel] [PATCH] x86/flush: use APIC ALLBUT destination shorthand when possible

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Jan 08, 2020 at 02:54:57PM +0100, Jan Beulich wrote:
> On 08.01.2020 14:30, Roger Pau Monné  wrote:
> > On Fri, Jan 03, 2020 at 01:55:51PM +0100, Jan Beulich wrote:
> >> On 03.01.2020 13:34, Roger Pau Monné wrote:
> >>> On Fri, Jan 03, 2020 at 01:08:20PM +0100, Jan Beulich wrote:
> >>>> On 24.12.2019 13:44, Roger Pau Monne wrote:
> >>>> Further a question on lock nesting: Since the commit message
> >>>> doesn't say anything in this regard, did you check there are no
> >>>> TLB flush invocations with the get_cpu_maps() lock held?
> >>>
> >>> The CPU maps lock is a recursive one, so it should be fine to attempt
> >>> a TLB flush with the lock already held.
> >>
> >> When already held by the same CPU - sure. It being a recursive
> >> one (which I paid attention to when writing my earlier reply)
> >> doesn't make it (together with any other one) immune against
> >> ABBA deadlocks, though.
> > 
> > There's no possibility of a deadlock here because get_cpu_maps does a
> > trylock, so if another cpu is holding the lock the flush will just
> > fallback to not using the shorthand.
> 
> Well, with the _exact_ arrangements (flush_lock used only in one
> place, and cpu_add_remove_lock only used in ways similar to how it
> is used now), there's no such risk, I agree. But there's nothing
> at all making sure this doesn't change. Hence, as said, at the very
> least this needs reasoning about in the description (or a code
> comment).

I'm afraid you will have to bear with me, but I'm still not sure how
the addition of get_cpu_maps in flush_area_mask can lead to deadlocks.
As said above get_cpu_maps does a trylock, which means that it will
never deadlock, and that's the only way to lock cpu_add_remove_lock.

Thanks, Roger.

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