Re: [Xen-ia64-devel] [Patch] Fix for re-enabling PV-on-HVM on IPF

[Isaku Yamahata <yamahata@xxxxxxxxxxxxx>]

On Thu, Mar 08, 2007 at 03:14:38PM +0900, Doi.Tsunehisa@xxxxxxxxxxxxxx wrote:

> >> +
> >>              guest_physmap_remove_page(d, gpfn, mfn);
> >> +
> >> +            /* post-set PGC_allocated flag */
> >> +            do {
> >> +                x = mfn_to_page(mfn)->count_info;
> >> +                if ((x & PGC_count_mask) == 0)
> >> +                    goto out;
> >> +                nx = x | PGC_allocated;
> >> +            } while (cmpxchg_acq(&mfn_to_page(mfn)->count_info, x, nx)
> >>  != x);
> >> +        }
> > 
> > checking == 0 is non-sense because we incremented it.
> > Probably you want to 
> >    if (!test_and_set_bit(page->count_info, _PGC_allocated)) {
> >     put_page(page);
> >     goto out;
> >    }
> 
>   guest_physmap_remove_page() unsets PGC_allocated flag, thus
> this test_and_set_bit() returns zero allways, I think.

Sorry, I was somewhat confused.
Probably it should looks like the following.
if (page->count_info != 1) { /* no one but us is using this page */
        put_page(page);
        goto out;
}
__set_bit(&page->count_info, _PGC_allocated);



>   In this code, I assumed that hypervisor might be destructing the
> domain. In this case, the page counter might be zero.

Such case doesn't occur because we called get_page().
It is the reason that the page reference counter exists for, isn't it?

-- 
yamahata

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel