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

[Tim Deegan <Tim.Deegan@xxxxxxxxxx>]

Hi, 

At 02:56 +0000 on 03 Dec (1291344990), Jui-Hao Chiang wrote:
> This small patch fixes 2 problems of memory sharing for xen-4.0-testing.hg
> (I haven't submitted patch here, if it violates any conventional
> rules, I'm glad to have advices)

Thanks for your patch!

Patches should be based on the tip of xen-unstable; we apply them there
and backport to the stable branches.

Also, you need to add a "Signed-off-by" line to the patch description to 
declare that the code is appropriately owned/licensed.  
See: http://elinux.org/Developer_Certificate_Of_Origin for what that means.

> (1) When nominating a shared page, the page_make_sharable() does not
>     recover the type_info count if it fails to nominate the page.

It looks to me as if it works already -- the cmpxchg loop in that
function always changes from (type = none, count = 0) to (type = shared,
count = 1), so the put_page_and_type() in the failure case does the
right thing, putting the count back to 0.

I don't understand why this function requires type == none; CC'ing the
author for an explanation.

> (2) When building xen with debug=n, the code in ASSERT() won't get
>     executed. Change to BUG_ON.

This part is clearly correct; I've made the equivalent change in
xen-unstable as changeset 22467:89116f28083f

> Besides, I don't understand why the page_make_sharable() force checking the 
> count_info with the following way?
> /* Check if the ref count is 2. The first from PGT_allocated, and the second
>      * from get_page at the top of this function */
>     if(page->count_info != (PGC_allocated | (2 + expected_refcnt)))
> 
> This seems to imply that the following kind of page can never be nominated 
> for shared pages because ci (count_info) is greater than 2 after get_page. 
> Here, domain 3 is a 64-bit HVM with hap=1, pae=1 on 64bit Xen.
> (XEN) Debug for domain=3, gfn=10, Debug page: MFN=c210ad is 
> ci=8000000000000002, ti=0, owner_id=3
> 
> Can someone gives a hint that
> (1) in what kind of scenario that ci = 2 and ti=0?
> (2) or why not allow ci >=2 to be nominated?

count = 2 and type = 0 happens in exactly the situation that the comment
describes: the page has no mappings from anywhere, just the one refcount
from being allocated and one taken at the start of the current function.

It's not possible to share a page with typecount > 0 because we need to
change its type.  I'm not sure why the refcount can't be greater than
two though, but I think it's to do with how shared pages have their
refcounts tracked differently to other pages.  Again, maybe Grzegorz can
clarify.

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

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