Re: [PATCH for-4.16] Revert "domctl: improve locking during domain destruction"

From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Ian Jackson <iwj@xenproject.org>
Cc: <xen-devel@lists.xenproject.org>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Jan Beulich <jbeulich@suse.com>, Julien Grall <julien@xen.org>,
	Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
	Hongyan Xia <hongyxia@amazon.com>,
	Dmitry Isaikin <isaikin-dmitry@yandex.ru>
Subject: Re: [PATCH for-4.16] Revert "domctl: improve locking during domain destruction"
Date: Tue, 9 Nov 2021 16:15:24 +0100	[thread overview]
Message-ID: <YYqQjMuARXwG4/yW@Air-de-Roger> (raw)
In-Reply-To: <24970.36376.658415.956968@mariner.uk.xensource.com>

On Tue, Nov 09, 2021 at 03:04:56PM +0000, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH for-4.16] Revert "domctl: improve locking during domain destruction""):
> > This reverts commit 228ab9992ffb1d8f9d2475f2581e68b2913acb88.
> > 
> > Performance analysis has shown that dropping the domctl lock during
> > domain destruction greatly increases the contention in the heap_lock,
> > thus making parallel destruction of domains slower.
> ...
> > Given the current point in the release, revert the commit and
> > reinstate holding the domctl lock during domain destruction. Further
> > work should be done in order to re-add more fine grained locking to
> > the domain destruction path once a proper solution to avoid the
> > heap_lock contention is found.
> > ---
> ...
> > Since this is a revert and not new code I think the risk is lower.
> > There's however some risk, as the original commit was from 2017, and
> > hence the surrounding code has changed a bit. It's also a possibility
> > that some other parts of the domain destruction code now rely on this
> > more fine grained locking. Local tests however haven't shown issues.
> 
> From a release management point of view I don't regard this as the
> kind of "revert" that ought to get any kind of special consideration.
> The tree has been like this since 2017 and Xen 4.11 and many changes
> have been happened since.
> 
> So I am going to treat this as an effectively new change.
> 
> AIUI it is a proposal to improve performance, not a bugfix.  Was this
> change posted (or, proposed on-list) before the Xen 4.16 Last Posting
> Date (24th of September) ?  Even if it was, it would need a freeze
> exception.

It was posted here:

https://lore.kernel.org/xen-devel/de46590ad566d9be55b26eaca0bc4dc7fbbada59.1585063311.git.hongyxia@amazon.com/

Which was missing a spin_barrier, and in a different form here:

https://lore.kernel.org/xen-devel/2e7044de3cd8a6768a20250e61fe262f3a018724.1631790362.git.isaikin-dmitry@yandex.ru/

Thanks, Roger.