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

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Wed, 17 Sep 2025 15:31:42 +0200
Heiko Carstens <hca@linux.ibm.com> wrote:

> On Wed, Sep 17, 2025 at 03:11:24PM +0200, Claudio Imbrenda wrote:
> > On Wed, 17 Sep 2025 14:30:06 +0200
> > Heiko Carstens <hca@linux.ibm.com> wrote:  
> > > > > > > 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  
> 
> Not necessarily...
> 
> > if you think this is not acceptable, I guess I'll have to implement
> > mempools  
> 
> ...since the kernel _might_ be easily capable of freeing memory, e.g. by
> writing to swap or writing file contents to disk. But this wouldn't happen
> with GFP_ATOMIC allocations.
> 
> I'm just stating that with GFP_ATOMIC for guest page table allocations it is
> much more likely that guests might be killed compared to what we have now.
> 
> Also I'm wondering why such code paths where page tables in atomic context
> need to be allocated even exist. Is that s390 specific (aka the new code), or
> is this due to common code requirements?

this is in the gmap rewrite, so new code in this series

mempools it is, then