Re: [PATCH v6 04/10] arm64: hyperv: Add memory alloc/free functions for Hyper-V size pages

[Arnd Bergmann]

On Wed, Mar 18, 2020 at 1:15 AM Michael Kelley <mikelley@xxxxxxxxxxxxx> wrote:
> From: Arnd Bergmann <arnd@xxxxxxxx> Sent: Monday, March 16, 2020 1:33 AM
> >
> > On Mon, Mar 16, 2020 at 9:30 AM Marc Zyngier <maz@xxxxxxxxxx> wrote:
> > > On 2020-03-16 08:22, Arnd Bergmann wrote:
> > > > On Sat, Mar 14, 2020 at 4:36 PM Michael Kelley <mikelley@xxxxxxxxxxxxx>
> > > > wrote:
> > > >>  /*
> > > >> + * Functions for allocating and freeing memory with size and
> > > >> + * alignment HV_HYP_PAGE_SIZE. These functions are needed because
> > > >> + * the guest page size may not be the same as the Hyper-V page
> > > >> + * size. We depend upon kmalloc() aligning power-of-two size
> > > >> + * allocations to the allocation size boundary, so that the
> > > >> + * allocated memory appears to Hyper-V as a page of the size
> > > >> + * it expects.
> > > >> + *
> > > >> + * These functions are used by arm64 specific code as well as
> > > >> + * arch independent Hyper-V drivers.
> > > >> + */
> > > >> +
> > > >> +void *hv_alloc_hyperv_page(void)
> > > >> +{
> > > >> +       BUILD_BUG_ON(PAGE_SIZE <  HV_HYP_PAGE_SIZE);
> > > >> +       return kmalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> > > >> +}
> > > >> +EXPORT_SYMBOL_GPL(hv_alloc_hyperv_page);
> > > >
> > > > I don't think there is any guarantee that kmalloc() returns
> > > > page-aligned
> > > > allocations in general.
> > >
> > > I believe that guarantee came with 59bb47985c1db ("mm, sl[aou]b:
> > > guarantee
> > > natural alignment for kmalloc(power-of-two)").
> > >
> > > > How about using get_free_pages() to implement this?
> > >
> > > This would certainly work, at the expense of a lot of wasted memory when
> > > PAGE_SIZE isn't 4k.
> >
> > I'm sure this is the least of your problems when the guest runs with
> > a large base page size, you've already wasted most of your memory
> > otherwise then.
> >
>
> I think there's value in keeping these functions.  There are 8 uses in
> architecture independent code at the moment, which admittedly saves
> only ~1/2 Mbyte of memory with a 64K page size, but we will have
> additional uses with more memory savings as we get all of the
> Hyper-V synthetic drivers to work with 64K page size.  Furthermore,
> there's coming work that will require additional steps to share a page
> between a guest and the Hyper-V host.  These functions are the right
> place to put the code for the additional sharing steps.  Removing them
> now in favor of a bare kmalloc() and then adding them back doesn't
> seem worthwhile.

My point was to keep the functions but use alloc_pages() internally,
so you can deal with the hypervisor having a larger page size than
the guest, which seems to be a more important scenario if I correctly
understand the differences between the way Windows and Linux
deal with page cache.

As far as I understand, using 64kb pages on Windows is generally
a win as the VFS code already deals with units of that size, while
on Linux the 4kb page size is too deeply entrenched within the
file system code to mess with it: Whatever you gain in terms of
TLB pressure on workloads that cannot use huge pages is all lost
again through extra I/O and wasted physical memory.

        Arnd