Re: [PATCH v2] x86: Perform mem_sharing teardown before paging teardown

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Tue, Feb 21, 2023 at 8:54 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 15.02.2023 18:07, Tamas K Lengyel wrote:
> > An assert failure has been observed in p2m_teardown when performing vm
> > forking and then destroying the forked VM (p2m-basic.c:173). The assert
> > checks whether the domain's shared pages counter is 0. According to the
> > patch that originally added the assert (7bedbbb5c31) the p2m_teardown
> > should only happen after mem_sharing already relinquished all shared pages.
> >
> > In this patch we flip the order in which relinquish ops are called to avoid
> > tripping the assert.
> >
> > Signed-off-by: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> > Fixes: e7aa55c0aab3 ("x86/p2m: free the paging memory pool preemptively")
>
> Especially considering the backporting requirement I'm disappointed to
> find that you haven't extended the description to explain why this
> heavier code churn is to be preferred over an adjustment to the offending
> assertion. As said in reply to v1 - I'm willing to accept arguments

> towards this being better, but I need to know those arguments.

The assert change as you proposed is hard to understand and will be thus harder to maintain. Conceptually sharing being torn down makes sense to happen before paging is torn down. Considering that's how it has been before the unfortunate regression I'm fixing here I don't think more justification is needed. The code-churn is minimal anyway.

Tamas