You (yamahata) said:
>>>> +
>>>> 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);
Does `page->count_info' have to be operated with atomic ?
>> 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?
I understand it.
Thanks,
- Tsunehisa Doi
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|