Re: [Xen-devel] [PATCH 2/3] x86/mem_sharing: replace use of page_lock/unlock with our own lock

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Thu, Apr 25, 2019 at 2:01 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>
> On 25/04/2019 20:57, Tamas K Lengyel wrote:
> > On Thu, Apr 25, 2019 at 12:58 PM Andrew Cooper
> > <andrew.cooper3@xxxxxxxxxx> wrote:
> >> On 25/04/2019 16:32, Tamas K Lengyel wrote:
> >>> diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
> >>> index 6faa563167..594de6148f 100644
> >>> --- a/xen/include/asm-x86/mm.h
> >>> +++ b/xen/include/asm-x86/mm.h
> >>> @@ -133,7 +133,10 @@ struct page_info
> >>>           * of sharing share the version they expect to.
> >>>           * This list is allocated and freed when a page is 
> >>> shared/unshared.
> >>>           */
> >>> -        struct page_sharing_info *sharing;
> >>> +        struct {
> >>> +            struct page_sharing_info *info;
> >>> +            rwlock_t lock;
> >>> +        } sharing;
> >>>      };
> >>>
> >>>      /* Reference count and various PGC_xxx flags and fields. */
> >> I'm afraid this is a no-go, but for some reasons which are rather more
> >> subtle that they appear here.
> >>
> >> There is one struct page_info per 4k frame, and you've added an extra 16
> >> bytes, taking it from 32 bytes to 48 bytes.
> >>
> >> Other than making it a non-power-of-2 (net diffstat grow/shrink: 256/27
> >> up/down: 7750/-5696 (2054) due to less efficient arithmetic[1]), the
> >> framtable has a fixed virtual size (128G by default), so you've ended up
> >> dropping Xen's memory limit (16TB by default) by 1/3.
> > Interesting, I'm not sure I completely follow but does this mean that
> > this structure is now not allowed to grow, like ever, or that I just
> > need to add padding to fix up its alignment?
>
> Basically, it needs exceedingly good reasons to grow.  Even "supporting
> more than 16T of RAM" wasn't a good enough reason to grow it from 32 to
> 64 bytes, which is why CONFIG_BIGMEM exists and is off by default.
>
> At the moment, I don't have a good solution for your problem to suggest.
>

I would be OK with putting the whole thing behind
CONFIG_HAS_MEM_SHARING and having that be off by default. Is that a
feasible route from your POV?

Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel