Re: [Xen-devel] [PATCH][Xen 4.0-testing.hg] fix small bugs of memory sharing

[Jui-Hao Chiang <juihaochiang@xxxxxxxxx>]

Hi, Tim:

On Thu, Dec 9, 2010 at 6:01 PM, Tim Deegan <Tim.Deegan@xxxxxxxxxx> wrote:

At 08:14 +0000 on 09 Dec (1291882483), Jui-Hao Chiang wrote:
> It seems the candidate page for nomination usually has (type=none, count=1), and it's ok for page_make_sharable() to make it (type=none, count=2) afterwards.
> However, when we have a page (type=none, count=2), the page_make_sharable() will make it wrong as the following steps:
> (step1) get_page() increases count (type=none, count=3)
> (step2) cmpxchg loops changes type (type=8400000000000001, count=3, actually the real value of count_info is 0x8000000000000002)
> (step3) Checking count is greater than 2? Oops!.... abort without recovering type back to none

OK, I think that's the problem.  Pages with type-count 0 basically have
no type so page_make_sharable() is wrong to insist on its having
type=none; it should only check that the typecount is zero.

Can you try changing the test in the cmpxchg loop only to check
for ((x & PGT_count_mask) != 0)?

I just checked the value of PGT_none, and it is actually equal to 0, so there seems nothing wrong with the original checking.
The reason that I swap step3 and step2 is because step3 doesn't modify any value while step2 does.
If checking count_info fails, then just abort. Else if count_info is correct, then go checking and modifying type_info.

> (2) is there a way or any place to unmap the memory and make the page count back to 1?

IIRC it's possible to ask qemu to unmap the guest's memory but not to
synchronously wait for that to happen (because that would be a priority
inversion at best and a deadlock at worst).  So that doesn't really
help.

If you're trying to do page deduplication you might try putting some of
the logic into qemu, where it can make sure it unmaps the page before
asking to share it.

I found one patch of xenpaging as the following, but not sure it's exactly the same as my question.
Cuts from their article http://thread.gmane.org/gmane.comp.emulators.xen.devel/91768/focus=91770
"qemu will just keep mapping pages and not release them, which causes problems for the memory pager (since the page is mapped, it won't get paged out)"

AFAIK, the qemu can not always map the entire address space of HVM guest, so Map Cache feature tends to map HVM's memory on-demand/partially. The flush-cache command will trigger qemu_invalidate_map_cache(), which in turn calls munmap() on all mapped virtual addresses. Am I on the right track?
 

> Assume my previous guess for stub domain is right.  Then if a page
> from the previous scenario is made sharable and its mapped mfn is
> freed (when sharing two pages, the later one's mfn will be discarded),
> will the stub domain refer to the old discarded mfn if no unmapping is
> performed?

Yes, and that's definitely not good.  AFAICS, qemu has to drop all its
mappings for the sharing to be safe, and the page needs to be unshared
again if qemu tries to map it again.  Otherwise I/O from one VM could
pollute another VM (or in your case, I/O to one page could overwrite
another page).

Thanks for your remind "the page needs to be unshared again if qemu tries to map it again".
There exists several hypercalls to perform the memory mapping, e.g. mmu_update, update_va_mapping, and I will check on them.
Of course, only the RW mapping should be taken care, right?
 
Bests,
Jui-Hao

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