Re: [Xen-devel] [PATCH 1/2] x86/mem_sharing: reorder when pages are unlocked and released

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.04.19 at 06:29, <tamas@xxxxxxxxxxxxx> wrote:
> Patch 0502e0adae2 "x86: correct instances of PGC_allocated clearing" 
> introduced
> grabbing extra references for pages that drop references tied to 
> PGC_allocated.
> However, the way these extra references were grabbed were incorrect, resulting
> in both share_pages and unshare_pages failing.

I'm sorry for this.

> @@ -984,7 +976,7 @@ static int share_pages(struct domain *sd, gfn_t sgfn, 
> shr_handle_t sh,
>           * Don't change the type of rmap for the client page. */
>          rmap_del(gfn, cpage, 0);
>          rmap_add(gfn, spage);
> -        put_page_and_type(cpage);
> +        put_count++;

Since this sits in a while(), ...

> @@ -1002,7 +994,9 @@ static int share_pages(struct domain *sd, gfn_t sgfn, 
> shr_handle_t sh,
>      /* Free the client page */
>      if(test_and_clear_bit(_PGC_allocated, &cpage->count_info))
>          put_page(cpage);
> -    put_page(cpage);
> +
> +    while(put_count--)
> +        put_page_and_type(cpage);

... put_count may be zero before this while(). At which point the
earlier put_page() would still be unsafe.

Also, despite the if() in context suggesting the style you used,
the rest of the function looks to use proper Xen style, so would
you please add the three missing blanks to the while() you add?

Jan



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