[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Wed, 8 Jan 2020 14:30:57 +0100
  • Authentication-results: esa5.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=roger.pau@xxxxxxxxxx; spf=Pass smtp.mailfrom=roger.pau@xxxxxxxxxx; spf=None smtp.helo=postmaster@xxxxxxxxxxxxxxx
  • Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx, Wei Liu <wl@xxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Delivery-date: Wed, 08 Jan 2020 13:31:12 +0000
  • Ironport-sdr: zKcUHQviZYxV1gA86x5qtzpFsWd4H+rE9u7GZX4MJhpV0E601KmqA/XMd+JGBfkVyBfeNlO7T9 Ys8DQSurAD4+il7o03inApYOiZLxoy63NKoulri78hcbFtM7nFNAUPyYi7X42NMRkMlqFRH8zZ DXajrPxKs9buzuhOQCp/c1YFhndp0SgQkshPYmjxOnDmgSXS1AVn2/9Hv2R+lbAPSmI9CJMsTv tqkrLwFkkchLbyRYQS+SUDI6w1932t21kJptu0i04eKcnw5ZyuGw2Uf0kNyxuKxCNOPWra0A6U S3I=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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.

> >> Even if
> >> you did and even if there are none, I think the function should
> >> then get a comment attached to the effect of this lock order
> >> inversion risk. (For example, it isn't obvious to me that no user
> >> of stop_machine() would ever want to do any kind of TLB flushing.)
> >>
> >> Overall I wonder whether your goal couldn't be achieved without
> >> the extra locking and without the special conditions.
> > 
> > Hm, this so far has worked fine on all the boxes that I've tried.
> > I'm happy to change it to a simpler approach, but I think the
> > conditions and locking are required for this to work correctly.
> 
> Which might then indicate said pre-existing use of cpu_online_map
> to be a (latent?) problem.

Hm, maybe it could be a problem when offlining a CPU. I assume this is
not an issue so far because there are no global TLB flushes with a
mask contaning a CPU that is being offlined.

Regarding the patch itself, do you think the shorthand logic should be
added to send_IPI_mask, or would you rather only use the shorthand for
the TLB flushes?

Thanks, Roger.

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.