Linux-Fsdevel Archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Sean Christopherson <seanjc@google.com>
Cc: "Xiaoyao Li" <xiaoyao.li@intel.com>,
	"Marc Zyngier" <maz@kernel.org>,
	"Oliver Upton" <oliver.upton@linux.dev>,
	"Huacai Chen" <chenhuacai@kernel.org>,
	"Michael Ellerman" <mpe@ellerman.id.au>,
	"Anup Patel" <anup@brainfault.org>,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"Alexander Viro" <viro@zeniv.linux.org.uk>,
	"Christian Brauner" <brauner@kernel.org>,
	"Matthew Wilcox (Oracle)" <willy@infradead.org>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.linux.dev, linux-mips@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, kvm-riscv@lists.infradead.org,
	linux-riscv@lists.infradead.org, linux-fsdevel@vger.kernel.org,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	"Xu Yilun" <yilun.xu@intel.com>,
	"Chao Peng" <chao.p.peng@linux.intel.com>,
	"Fuad Tabba" <tabba@google.com>,
	"Jarkko Sakkinen" <jarkko@kernel.org>,
	"Anish Moorthy" <amoorthy@google.com>,
	"David Matlack" <dmatlack@google.com>,
	"Yu Zhang" <yu.c.zhang@linux.intel.com>,
	"Isaku Yamahata" <isaku.yamahata@intel.com>,
	"Mickaël Salaün" <mic@digikod.net>,
	"Vlastimil Babka" <vbabka@suse.cz>,
	"Vishal Annapurve" <vannapurve@google.com>,
	"Ackerley Tng" <ackerleytng@google.com>,
	"Maciej Szmigiero" <mail@maciej.szmigiero.name>,
	"David Hildenbrand" <david@redhat.com>,
	"Quentin Perret" <qperret@google.com>,
	"Michael Roth" <michael.roth@amd.com>,
	Wang <wei.w.wang@intel.com>,
	"Liam Merwick" <liam.merwick@oracle.com>,
	"Isaku Yamahata" <isaku.yamahata@gmail.com>,
	"Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>
Subject: Re: [PATCH v13 17/35] KVM: Add transparent hugepage support for dedicated guest memory
Date: Wed, 1 Nov 2023 14:49:40 +0100	[thread overview]
Message-ID: <CABgObfaw4Byuzj5J3k48jdwT0HCKXLJNiuaA9H8Dtg+GOq==Sw@mail.gmail.com> (raw)
In-Reply-To: <ZUJVfCkIYYFp5VwG@google.com>

On Wed, Nov 1, 2023 at 2:41 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Wed, Nov 01, 2023, Xiaoyao Li wrote:
> > On 10/31/2023 10:16 PM, Sean Christopherson wrote:
> > > On Tue, Oct 31, 2023, Xiaoyao Li wrote:
> > > > On 10/28/2023 2:21 AM, Sean Christopherson wrote:
> > > > > Extended guest_memfd to allow backing guest memory with transparent
> > > > > hugepages. Require userspace to opt-in via a flag even though there's no
> > > > > known/anticipated use case for forcing small pages as THP is optional,
> > > > > i.e. to avoid ending up in a situation where userspace is unaware that
> > > > > KVM can't provide hugepages.
> > > >
> > > > Personally, it seems not so "transparent" if requiring userspace to opt-in.
> > > >
> > > > People need to 1) check if the kernel built with TRANSPARENT_HUGEPAGE
> > > > support, or check is the sysfs of transparent hugepage exists; 2)get the
> > > > maximum support hugepage size 3) ensure the size satisfies the alignment;
> > > > before opt-in it.
> > > >
> > > > Even simpler, userspace can blindly try to create guest memfd with
> > > > transparent hugapage flag. If getting error, fallback to create without the
> > > > transparent hugepage flag.
> > > >
> > > > However, it doesn't look transparent to me.
> > >
> > > The "transparent" part is referring to the underlying kernel mechanism, it's not
> > > saying anything about the API.  The "transparent" part of THP is that the kernel
> > > doesn't guarantee hugepages, i.e. whether or not hugepages are actually used is
> > > (mostly) transparent to userspace.
> > >
> > > Paolo also isn't the biggest fan[*], but there are also downsides to always
> > > allowing hugepages, e.g. silent failure due to lack of THP or unaligned size,
> > > and there's precedent in the form of MADV_HUGEPAGE.
> > >
> > > [*] https://lore.kernel.org/all/84a908ae-04c7-51c7-c9a8-119e1933a189@redhat.com
> >
> > But it's different than MADV_HUGEPAGE, in a way. Per my understanding, the
> > failure of MADV_HUGEPAGE is not fatal, user space can ignore it and
> > continue.
> >
> > However, the failure of KVM_GUEST_MEMFD_ALLOW_HUGEPAGE is fatal, which leads
> > to failure of guest memfd creation.
>
> Failing KVM_CREATE_GUEST_MEMFD isn't truly fatal, it just requires different
> action from userspace, i.e. instead of ignoring the error, userspace could redo
> KVM_CREATE_GUEST_MEMFD with KVM_GUEST_MEMFD_ALLOW_HUGEPAGE=0.
>
> We could make the behavior more like MADV_HUGEPAGE, e.g. theoretically we could
> extend fadvise() with FADV_HUGEPAGE, or add a guest_memfd knob/ioctl() to let
> userspace provide advice/hints after creating a guest_memfd.  But I suspect that
> guest_memfd would be the only user of FADV_HUGEPAGE, and IMO a post-creation hint
> is actually less desirable.
>
> KVM_GUEST_MEMFD_ALLOW_HUGEPAGE will fail only if userspace didn't provide a
> compatible size or the kernel doesn't support THP.  An incompatible size is likely
> a userspace bug, and for most setups that want to utilize guest_memfd, lack of THP
> support is likely a configuration bug.  I.e. many/most uses *want* failures due to
> KVM_GUEST_MEMFD_ALLOW_HUGEPAGE to be fatal.
>
> > For current implementation, I think maybe KVM_GUEST_MEMFD_DESIRE_HUGEPAGE
> > fits better than KVM_GUEST_MEMFD_ALLOW_HUGEPAGE? or maybe *PREFER*?
>
> Why?  Verbs like "prefer" and "desire" aren't a good fit IMO because they suggest
> the flag is a hint, and hints are usually best effort only, i.e. are ignored if
> there is a fundamental incompatibility.
>
> "Allow" isn't perfect, e.g. I would much prefer a straight KVM_GUEST_MEMFD_USE_HUGEPAGES
> or KVM_GUEST_MEMFD_HUGEPAGES flag, but I wanted the name to convey that KVM doesn't
> (yet) guarantee hugepages.  I.e. KVM_GUEST_MEMFD_ALLOW_HUGEPAGE is stronger than
> a hint, but weaker than a requirement.  And if/when KVM supports a dedicated memory
> pool of some kind, then we can add KVM_GUEST_MEMFD_REQUIRE_HUGEPAGE.

I think that the current patch is fine, but I will adjust it to always
allow the flag,
and to make the size check even if !CONFIG_TRANSPARENT_HUGEPAGE.
If hugepages are not guaranteed, and (theoretically) you could have no
hugepage at all in the result, it's okay to get this result even if THP is not
available in the kernel.

Paolo


  reply	other threads:[~2023-11-01 13:49 UTC|newest]

Thread overview: 148+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-27 18:21 [PATCH v13 00/35] KVM: guest_memfd() and per-page attributes Sean Christopherson
2023-10-27 18:21 ` [PATCH v13 01/35] KVM: Tweak kvm_hva_range and hva_handler_t to allow reusing for gfn ranges Sean Christopherson
2023-11-01 12:46   ` Fuad Tabba
2023-10-27 18:21 ` [PATCH v13 02/35] KVM: Assert that mmu_invalidate_in_progress *never* goes negative Sean Christopherson
2023-10-30 16:27   ` Paolo Bonzini
2023-11-01 12:46   ` Fuad Tabba
2023-10-27 18:21 ` [PATCH v13 03/35] KVM: Use gfn instead of hva for mmu_notifier_retry Sean Christopherson
2023-10-30 16:30   ` Paolo Bonzini
2023-10-30 16:53   ` David Matlack
2023-10-30 17:00     ` Paolo Bonzini
2023-10-30 18:21       ` David Matlack
2023-10-30 18:19     ` David Matlack
2023-11-01 15:31   ` Xu Yilun
2023-10-27 18:21 ` [PATCH v13 04/35] KVM: WARN if there are dangling MMU invalidations at VM destruction Sean Christopherson
2023-10-30 16:32   ` Paolo Bonzini
2023-11-01 12:50   ` Fuad Tabba
2023-10-27 18:21 ` [PATCH v13 05/35] KVM: PPC: Drop dead code related to KVM_ARCH_WANT_MMU_NOTIFIER Sean Christopherson
2023-10-30 16:34   ` Paolo Bonzini
2023-11-01 12:51   ` Fuad Tabba
2023-10-27 18:21 ` [PATCH v13 06/35] KVM: PPC: Return '1' unconditionally for KVM_CAP_SYNC_MMU Sean Christopherson
2023-10-27 18:21 ` [PATCH v13 07/35] KVM: Convert KVM_ARCH_WANT_MMU_NOTIFIER to CONFIG_KVM_GENERIC_MMU_NOTIFIER Sean Christopherson
2023-10-30 16:37   ` Paolo Bonzini
2023-11-01 12:54   ` Fuad Tabba
2023-10-27 18:21 ` [PATCH v13 08/35] KVM: Introduce KVM_SET_USER_MEMORY_REGION2 Sean Christopherson
2023-10-30 16:41   ` Paolo Bonzini
2023-10-30 20:25     ` Sean Christopherson
2023-10-30 22:12       ` Sean Christopherson
2023-10-30 23:22       ` Paolo Bonzini
2023-10-31  0:18         ` Sean Christopherson
2023-10-31  2:26   ` Xiaoyao Li
2023-10-31 14:04     ` Sean Christopherson
2023-11-01 14:19   ` Fuad Tabba
2023-10-27 18:21 ` [PATCH v13 09/35] KVM: Add KVM_EXIT_MEMORY_FAULT exit to report faults to userspace Sean Christopherson
2023-10-30 17:22   ` Paolo Bonzini
2023-11-01  7:30   ` Binbin Wu
2023-11-01 10:52   ` Huang, Kai
2023-11-01 17:36     ` Sean Christopherson
2023-11-02  2:19       ` Xiaoyao Li
2023-11-02 15:51         ` Sean Christopherson
2023-11-02  3:17       ` Huang, Kai
2023-11-02  9:35         ` Huang, Kai
2023-11-02 11:03           ` Paolo Bonzini
2023-11-02 15:44             ` Sean Christopherson
2023-11-02 18:35               ` Huang, Kai
2023-11-02 15:56         ` Sean Christopherson
2023-11-02 11:01       ` Paolo Bonzini
2023-11-03  4:09   ` Xu Yilun
2023-10-27 18:21 ` [PATCH v13 10/35] KVM: Add a dedicated mmu_notifier flag for reclaiming freed memory Sean Christopherson
2023-10-30 17:11   ` Paolo Bonzini
2023-11-02 13:55   ` Fuad Tabba
2023-10-27 18:21 ` [PATCH v13 11/35] KVM: Drop .on_unlock() mmu_notifier hook Sean Christopherson
2023-10-30 17:18   ` Paolo Bonzini
2023-11-02 13:55   ` Fuad Tabba
2023-10-27 18:21 ` [PATCH v13 12/35] KVM: Prepare for handling only shared mappings in mmu_notifier events Sean Christopherson
2023-10-30 17:21   ` Paolo Bonzini
2023-10-30 22:07     ` Sean Christopherson
2023-11-02  5:59   ` Binbin Wu
2023-11-02 11:14     ` Paolo Bonzini
2023-11-02 14:01   ` Fuad Tabba
2023-11-02 14:41     ` Sean Christopherson
2023-11-02 14:57       ` Fuad Tabba
2023-10-27 18:21 ` [PATCH v13 13/35] KVM: Introduce per-page memory attributes Sean Christopherson
2023-10-30  8:11   ` Chao Gao
2023-10-30 16:10     ` Sean Christopherson
2023-10-30 22:05       ` Sean Christopherson
2023-10-31 16:43   ` David Matlack
2023-11-02  3:01   ` Huang, Kai
2023-11-02 10:32     ` Paolo Bonzini
2023-11-02 10:55       ` Huang, Kai
2023-10-27 18:21 ` [PATCH v13 14/35] mm: Add AS_UNMOVABLE to mark mapping as completely unmovable Sean Christopherson
2023-10-30 17:24   ` Paolo Bonzini
2023-10-27 18:21 ` [PATCH v13 15/35] fs: Export anon_inode_getfile_secure() for use by KVM Sean Christopherson
2023-10-30 17:30   ` Paolo Bonzini
2023-11-02 16:24   ` Christian Brauner
2023-11-03 10:40     ` Paolo Bonzini
2023-10-27 18:21 ` [PATCH v13 16/35] KVM: Add KVM_CREATE_GUEST_MEMFD ioctl() for guest-specific backing memory Sean Christopherson
2023-10-31  2:27   ` Xiaoyao Li
2023-10-31  6:30   ` Chao Gao
2023-10-31 14:10     ` Sean Christopherson
2023-10-31 15:05   ` Fuad Tabba
2023-10-31 22:13     ` Sean Christopherson
2023-10-31 22:18       ` Paolo Bonzini
2023-11-01 10:51       ` Fuad Tabba
2023-11-01 21:55         ` Sean Christopherson
2023-11-02 13:52           ` Fuad Tabba
2023-11-03 23:17             ` Sean Christopherson
2023-10-31 18:24   ` David Matlack
2023-10-31 21:36     ` Sean Christopherson
2023-10-31 22:39       ` David Matlack
2023-11-02 15:48         ` Paolo Bonzini
2023-11-02 16:03           ` Sean Christopherson
2023-11-02 16:28             ` David Matlack
2023-11-02 17:37               ` Sean Christopherson
2023-11-03  9:42   ` Fuad Tabba
2023-11-04 10:26   ` Xu Yilun
2023-11-06 15:43     ` Sean Christopherson
2023-10-27 18:21 ` [PATCH v13 17/35] KVM: Add transparent hugepage support for dedicated guest memory Sean Christopherson
2023-10-31  8:35   ` Xiaoyao Li
2023-10-31 14:16     ` Sean Christopherson
2023-11-01  7:25       ` Xiaoyao Li
2023-11-01 13:41         ` Sean Christopherson
2023-11-01 13:49           ` Paolo Bonzini [this message]
2023-11-01 16:36             ` Sean Christopherson
2023-11-01 22:28               ` Paolo Bonzini
2023-11-01 22:34                 ` Sean Christopherson
2023-11-01 23:17                   ` Paolo Bonzini
2023-11-02 15:38                     ` Sean Christopherson
2023-11-02 15:46                       ` Paolo Bonzini
2023-11-27 11:13                         ` Vlastimil Babka
2023-11-29 22:40                           ` Sean Christopherson
2023-10-27 18:22 ` [PATCH v13 18/35] KVM: x86: "Reset" vcpu->run->exit_reason early in KVM_RUN Sean Christopherson
2023-10-30 17:31   ` Paolo Bonzini
2023-11-02 14:16   ` Fuad Tabba
2023-10-27 18:22 ` [PATCH v13 19/35] KVM: x86: Disallow hugepages when memory attributes are mixed Sean Christopherson
2023-10-27 18:22 ` [PATCH v13 20/35] KVM: x86/mmu: Handle page fault for private memory Sean Christopherson
2023-11-02 14:34   ` Fuad Tabba
2023-11-05 13:02   ` Xu Yilun
2023-11-05 16:19     ` Paolo Bonzini
2023-11-06 13:29       ` Xu Yilun
2023-11-06 15:56         ` Sean Christopherson
2023-10-27 18:22 ` [PATCH v13 21/35] KVM: Drop superfluous __KVM_VCPU_MULTIPLE_ADDRESS_SPACE macro Sean Christopherson
2023-11-02 14:35   ` Fuad Tabba
2023-10-27 18:22 ` [PATCH v13 22/35] KVM: Allow arch code to track number of memslot address spaces per VM Sean Christopherson
2023-10-30 17:34   ` Paolo Bonzini
2023-11-02 14:52   ` Fuad Tabba
2023-10-27 18:22 ` [PATCH v13 23/35] KVM: x86: Add support for "protected VMs" that can utilize private memory Sean Christopherson
2023-10-30 17:36   ` Paolo Bonzini
2023-11-06 11:00   ` Fuad Tabba
2023-11-06 11:03     ` Paolo Bonzini
2023-10-27 18:22 ` [PATCH v13 24/35] KVM: selftests: Drop unused kvm_userspace_memory_region_find() helper Sean Christopherson
2023-10-27 18:22 ` [PATCH v13 25/35] KVM: selftests: Convert lib's mem regions to KVM_SET_USER_MEMORY_REGION2 Sean Christopherson
2024-04-25 14:12   ` Dan Carpenter
2024-04-25 14:45     ` Shuah Khan
2024-04-25 15:09       ` Sean Christopherson
2024-04-25 16:22         ` Shuah Khan
2024-04-26  7:33         ` Jarkko Sakkinen
2023-10-27 18:22 ` [PATCH v13 26/35] KVM: selftests: Add support for creating private memslots Sean Christopherson
2023-10-27 18:22 ` [PATCH v13 27/35] KVM: selftests: Add helpers to convert guest memory b/w private and shared Sean Christopherson
2023-11-06 11:26   ` Fuad Tabba
2023-10-27 18:22 ` [PATCH v13 28/35] KVM: selftests: Add helpers to do KVM_HC_MAP_GPA_RANGE hypercalls (x86) Sean Christopherson
2023-10-27 18:22 ` [PATCH v13 29/35] KVM: selftests: Introduce VM "shape" to allow tests to specify the VM type Sean Christopherson
2023-10-27 18:22 ` [PATCH v13 30/35] KVM: selftests: Add GUEST_SYNC[1-6] macros for synchronizing more data Sean Christopherson
2023-10-27 18:22 ` [PATCH v13 31/35] KVM: selftests: Add x86-only selftest for private memory conversions Sean Christopherson
2023-10-27 18:22 ` [PATCH v13 32/35] KVM: selftests: Add KVM_SET_USER_MEMORY_REGION2 helper Sean Christopherson
2023-10-27 18:22 ` [PATCH v13 33/35] KVM: selftests: Expand set_memory_region_test to validate guest_memfd() Sean Christopherson
2023-10-27 18:22 ` [PATCH v13 34/35] KVM: selftests: Add basic selftest for guest_memfd() Sean Christopherson
2023-10-27 18:22 ` [PATCH v13 35/35] KVM: selftests: Test KVM exit behavior for private memory/access Sean Christopherson
2023-10-30 17:39 ` [PATCH v13 00/35] KVM: guest_memfd() and per-page attributes Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CABgObfaw4Byuzj5J3k48jdwT0HCKXLJNiuaA9H8Dtg+GOq==Sw@mail.gmail.com' \
    --to=pbonzini@redhat.com \
    --cc=ackerleytng@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=amoorthy@google.com \
    --cc=anup@brainfault.org \
    --cc=aou@eecs.berkeley.edu \
    --cc=brauner@kernel.org \
    --cc=chao.p.peng@linux.intel.com \
    --cc=chenhuacai@kernel.org \
    --cc=david@redhat.com \
    --cc=dmatlack@google.com \
    --cc=isaku.yamahata@gmail.com \
    --cc=isaku.yamahata@intel.com \
    --cc=jarkko@kernel.org \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=kvm-riscv@lists.infradead.org \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=liam.merwick@oracle.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mail@maciej.szmigiero.name \
    --cc=maz@kernel.org \
    --cc=mic@digikod.net \
    --cc=michael.roth@amd.com \
    --cc=mpe@ellerman.id.au \
    --cc=oliver.upton@linux.dev \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=qperret@google.com \
    --cc=seanjc@google.com \
    --cc=tabba@google.com \
    --cc=vannapurve@google.com \
    --cc=vbabka@suse.cz \
    --cc=viro@zeniv.linux.org.uk \
    --cc=wei.w.wang@intel.com \
    --cc=willy@infradead.org \
    --cc=xiaoyao.li@intel.com \
    --cc=yilun.xu@intel.com \
    --cc=yu.c.zhang@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).