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

Claudio Imbrenda
2025-09-16T16:47:47Z

On Tue, 16 Sep 2025 18:26:53 +0200
Heiko Carstens <hca@linux.ibm.com> wrote:

> On Wed, Sep 10, 2025 at 08:07:34PM +0200, Claudio Imbrenda wrote:
> > Add page table management functions to be used for KVM guest (gmap)
> > page tables.
> >
> > This patch adds the boilerplate and functions for the allocation and
> > deallocation of DAT tables.
> >
> > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> > ---
> > arch/s390/kvm/Makefile | 1 +
> > arch/s390/kvm/dat.c | 91 ++++++++++++++++++++++++++++++++++++++
> > arch/s390/kvm/dat.h | 4 ++
> > arch/s390/mm/page-states.c | 1 +
> > 4 files changed, 97 insertions(+)
> > create mode 100644 arch/s390/kvm/dat.c
>
> ...
>
> > +static inline struct page_table *dat_alloc_pt_noinit(void)
> > +{
> > + struct page *page;
> > + void *virt;
> > +
> > + page = alloc_pages(GFP_ATOMIC, 0);
> > + if (!page)
> > + return NULL;
> > +
> > + virt = page_to_virt(page);
> > + __arch_set_page_dat(virt, 1);
> > + return virt;
> > +}
>
> Is GFP_ATOMIC a typo, and this should have been GFP_KERNEL?
>
> Otherwise I would guess this will cause problems in the future when
> under memory pressure allocating guest page tables fails easily,
> while before this change such allocations never failed.

how so? the documentation in gfp_types.h says:

* %GFP_ATOMIC users can not sleep and need the allocation to succeed. A lower
* watermark is applied to allow access to "atomic reserves".
* The current implementation doesn't support NMI and few other strict
* non-preemptive contexts (e.g. raw_spin_lock). The same applies to %GFP_NOWAIT.
*
* %GFP_KERNEL is typical for kernel-internal allocations. The caller requires
* %ZONE_NORMAL or a lower zone for direct access but can direct reclaim.

I think GFP_ATOMIC actually gives more guarantees?