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

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Fri, May 3, 2019 at 7:56 AM Jan Beulich <JBeulich@xxxxxxxx> wrote:
>
> >>> On 03.05.19 at 15:48, <tamas@xxxxxxxxxxxxx> wrote:
> > On Fri, May 3, 2019 at 2:12 AM Jan Beulich <JBeulich@xxxxxxxx> wrote:
> >>
> >> >>> On 03.05.19 at 00:13, <tamas@xxxxxxxxxxxxx> wrote:
> >> > @@ -1002,7 +989,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);
>
> This should be after ...
>
> >> > -    put_page(cpage);
> >> > +
> >> > +    BUG_ON(!put_count);
>
> ... this, because ...
>
> >> > +    while ( put_count-- )
> >> > +        put_page_and_type(cpage);
> >>
> >> Strictly speaking I think the BUG_ON() should be moved ahead of the
> >> if() in context, so that a problematic put_page() would not get
> >> executed in the first place (even if the system is to die soon after).
> >
> > I don't follow - where is the problematic put_page()? And why is it
> > problematic?
>
> ... if indeed the BUG_ON() triggers, then it should do so before
> potentially putting the last ref of a page which shouldn't be put
> that way.

I see, thanks.

Tamas

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