Re: [PATCH v2 08/20] KVM: s390: KVM page table management functions: allocation

Claudio Imbrenda
2025-09-17T13:11:32Z

On Wed, 17 Sep 2025 14:30:06 +0200
Heiko Carstens <hca@linux.ibm.com> wrote:

> On Wed, Sep 17, 2025 at 01:25:56PM +0200, Claudio Imbrenda wrote:
> > On Wed, 17 Sep 2025 09:27:33 +0200
> > Heiko Carstens <hca@linux.ibm.com> wrote:
> >
> > > On Tue, Sep 16, 2025 at 07:36:44PM +0200, Heiko Carstens wrote:
> > > > On Tue, Sep 16, 2025 at 07:06:06PM +0200, Christian Borntraeger wrote:
> > > > >
> > > > > Am 16.09.25 um 19:05 schrieb Claudio Imbrenda:
> > > > >
> > > > > > > > I think GFP_ATOMIC actually gives more guarantees?
> > > > > > >
> > > > > > > In real life GFP_ATOMIC can fail, GFP_KERNEL does not.All gfp allocation failures
> > > > > > > are usually the atomic ones.
> > > > > >
> > > > > > interesting... then I guess I need GFP_KERNEL | GFP_ATOMIC ?
> > > > >
> > > > > No. ATOMIC always means: can fail.
> >
> > my issue is that GFP_KERNEL can sleep, and this allocation is sometimes
> > called from atomic contexts (e.g. while holding spinlocks)
> >
> > the right way to do this would be with mempools, to allocate memory
> > (and potentially sleep) when we are not in atomic context, and use it
> > whenever needed. this is on my to-do list for the future, but right now
> > I'd like to avoid having to refactor a ton of code.
>
> I doubt this is accetable even for an intermediate solution. As soon
> as the host is under memory pressure and starts doing I/O to free up
> memory, you will end up in -ENOMEM situations for simple guest page
> allocations.
>
> What happens with a guest in such a situation? Is this gracefully
> handled without that the guest is terminated?

well, we return -ENOMEM to userspace (and qemu will probably kill the
guest)

but if we can't even allocate 16kB, probably we're already in a pretty
bad situation

if you think this is not acceptable, I guess I'll have to implement
mempools

>
> > > Another correction: it should be GFP_KERNEL_ACCOUNT instead, like it
> > > used to be before in gmap_alloc_crst() since those page tables should
> > > be accounted to the process which is allocating them.
> >
> > gfp_types.h says:
> >
> > * %GFP_KERNEL_ACCOUNT is the same as GFP_KERNEL, except the allocation is
> > * accounted to kmemcg.
> >
> > I thought that without GFP_KERNEL_ACCOUNT, it would be accounted toward
> > the process?
> > The documentation is a little confusing here
>
> Documentation/core-api/memory-allocation.rst:
>
> * Untrusted allocations triggered from userspace should be a subject
> of kmem accounting and must have ``__GFP_ACCOUNT`` bit set. There
> is the handy ``GFP_KERNEL_ACCOUNT`` shortcut for ``GFP_KERNEL``
> allocations that should be accounted.
>
> This is done for all page table allocations, except if mm == init_mm
> (and unfortunately also not for the s390 specific crste alloc
> functions - I'll send a patch for that).

alright, then GFP_KERNEL_ACCOUNT it is