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

Claudio Imbrenda
2025-09-16T17:05:23Z

On Tue, 16 Sep 2025 19:01:11 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> Am 16.09.25 um 18:47 schrieb Claudio Imbrenda:
> > 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?
>
> 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 ?