On 04/09/2019 08:55, Jan Beulich wrote:
> Commit 2634b997af ("x86/shadow: don't enable shadow mode with too small
> a shadow allocation") was incomplete: The adjustment done there to
> shadow_enable() is also needed in shadow_one_bit_enable(). The (new)
> problem report was (apparently) a failed PV guest migration followed by
> another migration attempt for that same guest. Disabling log-dirty mode
> after the first one had left a couple of shadow pages allocated (perhaps
> something that also wants fixing), and hence the second enabling of
> log-dirty mode wouldn't have allocated anything further.
>
> Reported-by: James Wang <jnw...@suse.com>
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
>
> --- a/xen/arch/x86/mm/shadow/common.c
> +++ b/xen/arch/x86/mm/shadow/common.c
> @@ -2864,7 +2864,8 @@ static int shadow_one_bit_enable(struct
>  
>      mode |= PG_SH_enable;
>  
> -    if ( d->arch.paging.shadow.total_pages == 0 )
> +    if ( d->arch.paging.shadow.total_pages <
> +         sh_min_allocation(d) + d->arch.paging.shadow.p2m_pages )

This logic looks suspect.  The change from == 0 to < min looks fine, and
feels like the right thing to do.

However,  sh_min_allocation() should by definition be the minimum
quantity of pages necessary for things to function, which makes the + on
the end look wrong.

~Andrew

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

Reply via email to