Re: [PATCH 2/3] x86/mem_sharing: use dom_cow as placeholder parent until fork is complete

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.03.2022 18:41, Tamas K Lengyel wrote:
> For the duration of the fork memop set dom_cow as a placeholder parent. This
> gets updated to the real parent when the fork operation completes, or to NULL
> in case the fork failed.

I am concerned of this, in particular because the state may last across
perhaps a long series of preemptions. Furthermore ...

> --- a/xen/arch/x86/include/asm/mem_sharing.h
> +++ b/xen/arch/x86/include/asm/mem_sharing.h
> @@ -79,7 +79,7 @@ static inline int mem_sharing_unshare_page(struct domain *d,
>  
>  static inline bool mem_sharing_is_fork(const struct domain *d)
>  {
> -    return d->parent;
> +    return d->parent && d->parent != dom_cow;
>  }

... this now makes the function "lie" (the domain is a fork already
while being constructed). Hence at the very least a comment would want
to appear here explaining why this is wanted despite not really being
correct. This "lying" for example means a partly set up fork would be
skipped by domain_relinquish_resources(), in case the tool stack
decided to kill the domain instead of waiting for its creation to
finish.

> --- a/xen/arch/x86/mm/mem_sharing.c
> +++ b/xen/arch/x86/mm/mem_sharing.c
> @@ -1850,7 +1850,9 @@ static int fork(struct domain *cd, struct domain *d, 
> uint16_t flags)
>          *cd->arch.cpuid = *d->arch.cpuid;
>          *cd->arch.msr = *d->arch.msr;
>          cd->vmtrace_size = d->vmtrace_size;
> -        cd->parent = d;
> +
> +        /* use dom_cow as a placeholder until we are all done */

Nit: As per ./CODING_STYLE you want to at least start the comment with
a capital letter.

Jan