Hi Yamahata-san,
Thank you for your comment.
You (yamahata) said:
> On Thu, Mar 08, 2007 at 01:06:04PM +0900, DOI Tsunehisa wrote:
> > diff -r 61eb6589e720 -r b602dd142385 xen/arch/ia64/xen/mm.c
> > --- a/xen/arch/ia64/xen/mm.c Tue Mar 06 21:11:37 2007 +0900
> > +++ b/xen/arch/ia64/xen/mm.c Thu Mar 08 11:57:46 2007 +0900
> > @@ -2110,12 +2125,31 @@ arch_memory_op(int op, XEN_GUEST_HANDLE(
> >
> > /* Unmap from old location, if any. */
> > gpfn = get_gpfn_from_mfn(mfn);
> > - if (gpfn != INVALID_M2P_ENTRY)
> > + if (gpfn != INVALID_M2P_ENTRY) {
> > + unsigned long x, nx;
> > + /*
> > + * guest_physmap_remove_page() (for IPF) descrements page
> > + * counter and unset PGC_allocated flag,
> > + * so pre-increment page counter and post-set flag inserte
d
> > + */
> > + /* pre-increment page counter */
> > + get_page(mfn_to_page(mfn), d);
>
> Please check the return value.
I agree. I'll modify it.
>> +
>> 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.
In this code, I assumed that hypervisor might be destructing the
domain. In this case, the page counter might be zero.
Thanks,
- Tsunehisa Doi
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|
Home