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

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Mon, Apr 15, 2019 at 9:21 AM George Dunlap <george.dunlap@xxxxxxxxxx> wrote:
>
> On 4/12/19 8:47 PM, Tamas K Lengyel 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.
>
> Why?  Is cd not the owner of the page?

It's not, dom_cow is.

> > @@ -1002,7 +994,10 @@ 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);
> > +
> > +    ASSERT(put_count);
> > +    while ( put_count-- )
> > +        put_page_and_type(cpage);
>
> I don't think this is in the spirit of the fix in 0502e0adae2; the idea
> there seems to have been to make sure that cpage belongs to the right
> domain, and that the ownership doesn't change.  If such a race / mistake
> were possible before that change, such a race / mistake would be
> possible after this change, wouldn't it?

I don't have an answer to your question. AFAIU this is just to ensure
that the page isn't dropped before test_and_clear_bit is used on the
page.

Tamas

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