On 12/20/2017 09:34 AM, Jan Beulich wrote:
> p2m_pod_decrease_reservation() at the moment only returns a boolean
> value: true for "nothing more to do", false for "something more to do".
> If it returns false, decrease_reservation() will loop over the entire
> range, calling guest_remove_page() for each page.
> 
> Unfortunately, in the case p2m_pod_decrease_reservation() succeeds
> partially, some of the memory in the range will be not-present; at which
> point guest_remove_page() will return an error, and the entire operation
> will fail.
> 
> Fix this by:
> 1. Having p2m_pod_decrease_reservation() return exactly the number of
>    gpfn pages it has handled (i.e., replaced with 'not present').
> 2. Making guest_remove_page() return -ENOENT in the case that the gpfn
>    in question was already empty (and in no other cases).
> 3. When looping over guest_remove_page(), expect the number of -ENOENT
>    failures to be no larger than the number of pages
>    p2m_pod_decrease_reservation() removed.
> 
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
> Signed-off-by: George Dunlap <george.dun...@citrix.com>
> ---
> v2: Re-written description (by George). Add comments (as suggested
>     by George). Formatting.

One thing to double-check...

> @@ -335,10 +336,8 @@ int guest_remove_page(struct domain *d,
>          rc = mem_sharing_unshare_page(d, gmfn, 0);
>          if ( rc )
>          {
> -            put_gfn(d, gmfn);
>              (void)mem_sharing_notify_enomem(d, gmfn, 0);
> -
> -            return rc;
> +            goto out_put_gfn;

I take it you've checked to make sure that moving this put_gfn() over
the notify call is OK?

I took a brief look and it seems OK to me; so if you're happy with that
then:

Reviewed-by: George Dunlap <george.dun...@citrix.com>

Thanks, and sorry for the delay.

 -George

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

Reply via email to