Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc()
@ 2026-09-02  6:15 Mike Rapoport (Microsoft)
  2026-09-02  6:15 ` [PATCH 1/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STHYI buffer Mike Rapoport (Microsoft)
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Mike Rapoport (Microsoft) @ 2026-09-02  6:15 UTC (permalink / raw)
  To: Christian Borntraeger, Claudio Imbrenda, Janosch Frank
  Cc: Alexander Gordeev, David Hildenbrand, Heiko Carstens,
	Mike Rapoport, Sven Schnelle, Vasily Gorbik, Vlastimil Babka, kvm,
	linux-kernel, linux-mm, linux-s390

This is a (small) part of larger work of replacing page allocator calls
with kmalloc.

My initial intention a few month ago was to remove ugly casts [1], but then
willy pointed out that Linus objected to something like this [2] and it
looks like more than a decade old technical debt.

Largely, anything that doesn't need struct page (or a memdesc in the
future) should just use kmalloc() or kvmalloc() to allocate memory.
kmalloc() guarantees alignment, physical contiguity and working
virt_to_phys() and beside nicer API that returns void * on alloc and
doesn't require to know the allocation size on free, kmalloc() provides
better debugging capabilities than page allocator.

Another thing is that touching these allocation sites gives the reviewers
opportunity to see if a PAGE_SIZE buffer is actually needed or maybe
another size is appropriate.

For larger allocations that don't need physically contiguous memory
kvmalloc() can be a better option that __get_free_pages() because under
memory pressure it's is easier to allocate several order-0 pages than a
physically contiguous chunk with the same number of pages.

And last, but not least, removing needless calls to page allocator should
help with memdesc (aka project folio) conversion. There will be way less
places to audit to see if the user was actually using struct page.

Also in git:
https://git.kernel.org/pub/scm/linux/kernel/git/rppt/linux.git gfp-to-kmalloc/s390-kvm

[1] https://lore.kernel.org/all/20251018093002.3660549-1-rppt@kernel.org/
[2] https://lore.kernel.org/all/CA+55aFwp4iy4rtX2gE2WjBGFL=NxMVnoFeHqYa2j1dYOMMGqxg@mail.gmail.com/

---
Mike Rapoport (Microsoft) (4):
      KVM: s390: Replace get_zeroed_page() with kzalloc() for the STHYI buffer
      KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer
      KVM: s390: Replace get_zeroed_page() with kzalloc() for the GIB
      KVM: s390: Replace get_zeroed_page() with kzalloc() for sie_page2 and CMMA

 arch/s390/kvm/s390/intercept.c |  9 +++++----
 arch/s390/kvm/s390/interrupt.c |  8 ++++----
 arch/s390/kvm/s390/priv.c      | 19 ++++++++++---------
 arch/s390/kvm/s390/s390.c      | 12 ++++++------
 4 files changed, 25 insertions(+), 23 deletions(-)
---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20260828-s390-kvm-2015b0d777dc

--
Sincerely yours,
Mike.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STHYI buffer
  2026-09-02  6:15 [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc() Mike Rapoport (Microsoft)
@ 2026-09-02  6:15 ` Mike Rapoport (Microsoft)
  2026-09-02  6:15 ` [PATCH 2/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer Mike Rapoport (Microsoft)
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Mike Rapoport (Microsoft) @ 2026-09-02  6:15 UTC (permalink / raw)
  To: Christian Borntraeger, Claudio Imbrenda, Janosch Frank
  Cc: Alexander Gordeev, David Hildenbrand, Heiko Carstens,
	Mike Rapoport, Sven Schnelle, Vasily Gorbik, Vlastimil Babka, kvm,
	linux-kernel, linux-mm, linux-s390

handle_sthyi() allocates the buffer that receives the STHYI response
block before it is copied to the guest.

This buffer can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/s390/kvm/s390/intercept.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/s390/kvm/s390/intercept.c b/arch/s390/kvm/s390/intercept.c
index ca1205dfac8b0..aaf9828d1f44d 100644
--- a/arch/s390/kvm/s390/intercept.c
+++ b/arch/s390/kvm/s390/intercept.c
@@ -11,6 +11,7 @@
 #include <linux/kvm_host.h>
 #include <linux/errno.h>
 #include <linux/pagemap.h>
+#include <linux/slab.h>
 
 #include <asm/asm-offsets.h>
 #include <asm/irq.h>
@@ -438,13 +439,13 @@ int handle_sthyi(struct kvm_vcpu *vcpu)
 	if (!kvm_s390_pv_cpu_is_protected(vcpu) && (addr & ~PAGE_MASK))
 		return kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION);
 
-	sctns = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
+	sctns = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);
 	if (!sctns)
 		return -ENOMEM;
 
 	cc = sthyi_fill(sctns, &rc);
 	if (cc < 0) {
-		free_page((unsigned long)sctns);
+		kfree(sctns);
 		return cc;
 	}
 out:
@@ -454,13 +455,13 @@ int handle_sthyi(struct kvm_vcpu *vcpu)
 		} else {
 			r = write_guest(vcpu, addr, reg2, sctns, PAGE_SIZE);
 			if (r) {
-				free_page((unsigned long)sctns);
+				kfree(sctns);
 				return kvm_s390_inject_prog_cond(vcpu, r);
 			}
 		}
 	}
 
-	free_page((unsigned long)sctns);
+	kfree(sctns);
 	vcpu->run->s.regs.gprs[reg2 + 1] = rc;
 	kvm_s390_set_psw_cc(vcpu, cc);
 	return r;

-- 
2.53.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer
  2026-09-02  6:15 [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc() Mike Rapoport (Microsoft)
  2026-09-02  6:15 ` [PATCH 1/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STHYI buffer Mike Rapoport (Microsoft)
@ 2026-09-02  6:15 ` Mike Rapoport (Microsoft)
  2026-09-02  6:15 ` [PATCH 3/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the GIB Mike Rapoport (Microsoft)
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Mike Rapoport (Microsoft) @ 2026-09-02  6:15 UTC (permalink / raw)
  To: Christian Borntraeger, Claudio Imbrenda, Janosch Frank
  Cc: Alexander Gordeev, David Hildenbrand, Heiko Carstens,
	Mike Rapoport, Sven Schnelle, Vasily Gorbik, Vlastimil Babka, kvm,
	linux-kernel, linux-mm, linux-s390

handle_stsi() allocates the buffer that receives the STSI response block
before it is copied to the guest.

This buffer can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

While on it, make the buffer a void pointer to get rid of the casts.

Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/s390/kvm/s390/priv.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/arch/s390/kvm/s390/priv.c b/arch/s390/kvm/s390/priv.c
index b3cb2c2c3aa71..856939acf012f 100644
--- a/arch/s390/kvm/s390/priv.c
+++ b/arch/s390/kvm/s390/priv.c
@@ -14,6 +14,7 @@
 #include <linux/mm_types.h>
 #include <linux/pgtable.h>
 #include <linux/io.h>
+#include <linux/slab.h>
 #include <asm/asm-offsets.h>
 #include <asm/facility.h>
 #include <asm/current.h>
@@ -869,7 +870,7 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
 	int fc = (vcpu->run->s.regs.gprs[0] & 0xf0000000) >> 28;
 	int sel1 = vcpu->run->s.regs.gprs[0] & 0xff;
 	int sel2 = vcpu->run->s.regs.gprs[1] & 0xffff;
-	unsigned long mem = 0;
+	void *mem = NULL;
 	u64 operand2;
 	int rc = 0;
 	u8 ar;
@@ -911,19 +912,19 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
 	switch (fc) {
 	case 1: /* same handling for 1 and 2 */
 	case 2:
-		mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
+		mem = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);
 		if (!mem)
 			goto out_no_data;
-		if (stsi((void *) mem, fc, sel1, sel2))
+		if (stsi(mem, fc, sel1, sel2))
 			goto out_no_data;
 		break;
 	case 3:
 		if (sel1 != 2 || sel2 != 2)
 			goto out_no_data;
-		mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
+		mem = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);
 		if (!mem)
 			goto out_no_data;
-		handle_stsi_3_2_2(vcpu, (void *) mem);
+		handle_stsi_3_2_2(vcpu, mem);
 		break;
 	case 15: /* fc 15 is fully handled in userspace */
 		insert_stsi_usr_data(vcpu, operand2, ar, fc, sel1, sel2);
@@ -931,10 +932,10 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
 		return -EREMOTE;
 	}
 	if (kvm_s390_pv_cpu_is_protected(vcpu)) {
-		memcpy(sida_addr(vcpu->arch.sie_block), (void *)mem, PAGE_SIZE);
+		memcpy(sida_addr(vcpu->arch.sie_block), mem, PAGE_SIZE);
 		rc = 0;
 	} else {
-		rc = write_guest(vcpu, operand2, ar, (void *)mem, PAGE_SIZE);
+		rc = write_guest(vcpu, operand2, ar, mem, PAGE_SIZE);
 	}
 	if (rc) {
 		rc = kvm_s390_inject_prog_cond(vcpu, rc);
@@ -945,14 +946,14 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
 		rc = -EREMOTE;
 	}
 	trace_kvm_s390_handle_stsi(vcpu, fc, sel1, sel2, operand2);
-	free_page(mem);
+	kfree(mem);
 	kvm_s390_set_psw_cc(vcpu, 0);
 	vcpu->run->s.regs.gprs[0] = 0;
 	return rc;
 out_no_data:
 	kvm_s390_set_psw_cc(vcpu, 3);
 out:
-	free_page(mem);
+	kfree(mem);
 	return rc;
 }
 

-- 
2.53.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the GIB
  2026-09-02  6:15 [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc() Mike Rapoport (Microsoft)
  2026-09-02  6:15 ` [PATCH 1/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STHYI buffer Mike Rapoport (Microsoft)
  2026-09-02  6:15 ` [PATCH 2/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer Mike Rapoport (Microsoft)
@ 2026-09-02  6:15 ` Mike Rapoport (Microsoft)
  2026-09-02  6:15 ` [PATCH 4/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for sie_page2 and CMMA Mike Rapoport (Microsoft)
  2026-09-02 11:06 ` [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc() Claudio Imbrenda
  4 siblings, 0 replies; 11+ messages in thread
From: Mike Rapoport (Microsoft) @ 2026-09-02  6:15 UTC (permalink / raw)
  To: Christian Borntraeger, Claudio Imbrenda, Janosch Frank
  Cc: Alexander Gordeev, David Hildenbrand, Heiko Carstens,
	Mike Rapoport, Sven Schnelle, Vasily Gorbik, Vlastimil Babka, kvm,
	linux-kernel, linux-mm, linux-s390

kvm_s390_gib_init() allocates the guest information block (GIB). The GIB
is passed to the hardware as a physical address and must be page aligned.

kmalloc() guarantees that a power of two sized allocation is aligned to
its size, so a PAGE_SIZE allocation is page aligned as well.

This buffer can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/s390/kvm/s390/interrupt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/s390/kvm/s390/interrupt.c b/arch/s390/kvm/s390/interrupt.c
index 0381ae9817035..02065fb8d053e 100644
--- a/arch/s390/kvm/s390/interrupt.c
+++ b/arch/s390/kvm/s390/interrupt.c
@@ -3689,7 +3689,7 @@ void kvm_s390_gib_destroy(void)
 	}
 	chsc_sgib(0);
 	unregister_adapter_interrupt(&gib_alert_irq);
-	free_page((unsigned long)gib);
+	kfree(gib);
 	gib = NULL;
 }
 
@@ -3703,7 +3703,7 @@ int __init kvm_s390_gib_init(u8 nisc)
 		goto out;
 	}
 
-	gib = (struct kvm_s390_gib *)get_zeroed_page(GFP_KERNEL_ACCOUNT | GFP_DMA);
+	gib = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT | GFP_DMA);
 	if (!gib) {
 		rc = -ENOMEM;
 		goto out;
@@ -3722,7 +3722,7 @@ int __init kvm_s390_gib_init(u8 nisc)
 	gib_origin = virt_to_phys(gib);
 	if (chsc_sgib(gib_origin)) {
 		pr_err("Associating the GIB with the AIV facility failed\n");
-		free_page((unsigned long)gib);
+		kfree(gib);
 		gib = NULL;
 		rc = -EIO;
 		goto out_unreg_gal;
@@ -3742,7 +3742,7 @@ int __init kvm_s390_gib_init(u8 nisc)
 out_unreg_gal:
 	unregister_adapter_interrupt(&gib_alert_irq);
 out_free_gib:
-	free_page((unsigned long)gib);
+	kfree(gib);
 	gib = NULL;
 out:
 	return rc;

-- 
2.53.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for sie_page2 and CMMA
  2026-09-02  6:15 [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc() Mike Rapoport (Microsoft)
                   ` (2 preceding siblings ...)
  2026-09-02  6:15 ` [PATCH 3/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the GIB Mike Rapoport (Microsoft)
@ 2026-09-02  6:15 ` Mike Rapoport (Microsoft)
  2026-09-02 11:06 ` [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc() Claudio Imbrenda
  4 siblings, 0 replies; 11+ messages in thread
From: Mike Rapoport (Microsoft) @ 2026-09-02  6:15 UTC (permalink / raw)
  To: Christian Borntraeger, Claudio Imbrenda, Janosch Frank
  Cc: Alexander Gordeev, David Hildenbrand, Heiko Carstens,
	Mike Rapoport, Sven Schnelle, Vasily Gorbik, Vlastimil Babka, kvm,
	linux-kernel, linux-mm, linux-s390

kvm_arch_init_vm() allocates struct sie_page2 that holds the facility
list and the crypto control block of a VM and kvm_s390_vcpu_setup_cmma()
allocates the CMMA collaborative memory management block referenced by
the SIE control block. Both are passed to the hardware as physical
addresses and must be page aligned.

kmalloc() guarantees that a power of two sized allocation is aligned to
its size, so a PAGE_SIZE allocation is page aligned as well.

These buffers can be allocated with kmalloc() as there's nothing special
about them to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

The sie_page allocated in kvm_arch_vcpu_create() is left alone because it
is mapped to userspace with virt_to_page() in kvm_arch_vcpu_fault() and
therefore really needs a struct page.

Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/s390/kvm/s390/s390.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/s390/kvm/s390/s390.c b/arch/s390/kvm/s390/s390.c
index b0839e887221e..8ec505060f8c0 100644
--- a/arch/s390/kvm/s390/s390.c
+++ b/arch/s390/kvm/s390/s390.c
@@ -3273,8 +3273,8 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 		goto out_err;
 
 	BUILD_BUG_ON(sizeof(struct sie_page2) != 4096);
-	kvm->arch.sie_page2 =
-	     (struct sie_page2 *) get_zeroed_page(GFP_KERNEL_ACCOUNT | GFP_DMA);
+	kvm->arch.sie_page2 = kzalloc_obj(*kvm->arch.sie_page2,
+					  GFP_KERNEL_ACCOUNT | GFP_DMA);
 	if (!kvm->arch.sie_page2)
 		goto out_err;
 
@@ -3369,7 +3369,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 
 	return 0;
 out_err:
-	free_page((unsigned long)kvm->arch.sie_page2);
+	kfree(kvm->arch.sie_page2);
 	debug_unregister(kvm->arch.dbf);
 	sca_dispose(kvm);
 	KVM_EVENT(3, "creation of vm failed: %d", rc);
@@ -3427,7 +3427,7 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 		mmu_notifier_unregister(&kvm->arch.pv.mmu_notifier, kvm->mm);
 
 	debug_unregister(kvm->arch.dbf);
-	free_page((unsigned long)kvm->arch.sie_page2);
+	kfree(kvm->arch.sie_page2);
 	kvm_s390_destroy_adapters(kvm);
 	kvm_s390_clear_float_irqs(kvm);
 	kvm_s390_vsie_destroy(kvm);
@@ -3657,13 +3657,13 @@ static void kvm_s390_vcpu_crypto_setup(struct kvm_vcpu *vcpu)
 void kvm_s390_vcpu_unsetup_cmma(struct kvm_vcpu *vcpu)
 {
 	if (vcpu->arch.sie_block->cbrlo)
-		free_page((unsigned long)phys_to_virt(vcpu->arch.sie_block->cbrlo));
+		kfree(phys_to_virt(vcpu->arch.sie_block->cbrlo));
 	vcpu->arch.sie_block->cbrlo = 0;
 }
 
 int kvm_s390_vcpu_setup_cmma(struct kvm_vcpu *vcpu)
 {
-	void *cbrlo_page = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
+	void *cbrlo_page = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);
 
 	if (!cbrlo_page)
 		return -ENOMEM;

-- 
2.53.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc()
  2026-09-02  6:15 [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc() Mike Rapoport (Microsoft)
                   ` (3 preceding siblings ...)
  2026-09-02  6:15 ` [PATCH 4/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for sie_page2 and CMMA Mike Rapoport (Microsoft)
@ 2026-09-02 11:06 ` Claudio Imbrenda
  2026-09-02 13:17   ` Mike Rapoport
  4 siblings, 1 reply; 11+ messages in thread
From: Claudio Imbrenda @ 2026-09-02 11:06 UTC (permalink / raw)
  To: Mike Rapoport (Microsoft)
  Cc: Christian Borntraeger, Janosch Frank, Alexander Gordeev,
	David Hildenbrand, Heiko Carstens, Sven Schnelle, Vasily Gorbik,
	Vlastimil Babka, kvm, linux-kernel, linux-mm, linux-s390

On Wed, 02 Sep 2026 09:15:12 +0300
"Mike Rapoport (Microsoft)" <rppt@kernel.org> wrote:

> This is a (small) part of larger work of replacing page allocator calls
> with kmalloc.
> 
> My initial intention a few month ago was to remove ugly casts [1], but then
> willy pointed out that Linus objected to something like this [2] and it
> looks like more than a decade old technical debt.
> 
> Largely, anything that doesn't need struct page (or a memdesc in the
> future) should just use kmalloc() or kvmalloc() to allocate memory.
> kmalloc() guarantees alignment, physical contiguity and working
> virt_to_phys() and beside nicer API that returns void * on alloc and
> doesn't require to know the allocation size on free, kmalloc() provides
> better debugging capabilities than page allocator.
> 
> Another thing is that touching these allocation sites gives the reviewers
> opportunity to see if a PAGE_SIZE buffer is actually needed or maybe
> another size is appropriate.
> 
> For larger allocations that don't need physically contiguous memory
> kvmalloc() can be a better option that __get_free_pages() because under
> memory pressure it's is easier to allocate several order-0 pages than a
> physically contiguous chunk with the same number of pages.
> 
> And last, but not least, removing needless calls to page allocator should
> help with memdesc (aka project folio) conversion. There will be way less
> places to audit to see if the user was actually using struct page.

I have some objections to this series, but not because of what you are
trying to do (which is actually nice).

I understand that you probably wanted to touch as little code as
possible, but now since you're rewriting the allocations to use
kmalloc.... I'd like them to be converted to use the __free(kvmalloc)
system. It will make the code smaller, easier to read and understand,
less prone to future errors, etc.

In some places the whole code flow can be simplified a lot.
 
> Also in git:
> https://git.kernel.org/pub/scm/linux/kernel/git/rppt/linux.git gfp-to-kmalloc/s390-kvm
> 
> [1] https://lore.kernel.org/all/20251018093002.3660549-1-rppt@kernel.org/
> [2] https://lore.kernel.org/all/CA+55aFwp4iy4rtX2gE2WjBGFL=NxMVnoFeHqYa2j1dYOMMGqxg@mail.gmail.com/
> 
> ---
> Mike Rapoport (Microsoft) (4):
>       KVM: s390: Replace get_zeroed_page() with kzalloc() for the STHYI buffer
>       KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer
>       KVM: s390: Replace get_zeroed_page() with kzalloc() for the GIB
>       KVM: s390: Replace get_zeroed_page() with kzalloc() for sie_page2 and CMMA
> 
>  arch/s390/kvm/s390/intercept.c |  9 +++++----
>  arch/s390/kvm/s390/interrupt.c |  8 ++++----
>  arch/s390/kvm/s390/priv.c      | 19 ++++++++++---------
>  arch/s390/kvm/s390/s390.c      | 12 ++++++------
>  4 files changed, 25 insertions(+), 23 deletions(-)
> ---
> base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
> change-id: 20260828-s390-kvm-2015b0d777dc
> 
> --
> Sincerely yours,
> Mike.
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc()
  2026-09-02 11:06 ` [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc() Claudio Imbrenda
@ 2026-09-02 13:17   ` Mike Rapoport
  2026-09-02 14:53     ` Claudio Imbrenda
  0 siblings, 1 reply; 11+ messages in thread
From: Mike Rapoport @ 2026-09-02 13:17 UTC (permalink / raw)
  To: Claudio Imbrenda
  Cc: Christian Borntraeger, Janosch Frank, Alexander Gordeev,
	David Hildenbrand, Heiko Carstens, Sven Schnelle, Vasily Gorbik,
	Vlastimil Babka, kvm, linux-kernel, linux-mm, linux-s390

On Wed, Sep 02, 2026 at 01:06:12PM +0200, Claudio Imbrenda wrote:
> On Wed, 02 Sep 2026 09:15:12 +0300
> "Mike Rapoport (Microsoft)" <rppt@kernel.org> wrote:
> 
> > This is a (small) part of larger work of replacing page allocator calls
> > with kmalloc.
> > 
> > My initial intention a few month ago was to remove ugly casts [1], but then
> > willy pointed out that Linus objected to something like this [2] and it
> > looks like more than a decade old technical debt.
> > 
> > Largely, anything that doesn't need struct page (or a memdesc in the
> > future) should just use kmalloc() or kvmalloc() to allocate memory.
> > kmalloc() guarantees alignment, physical contiguity and working
> > virt_to_phys() and beside nicer API that returns void * on alloc and
> > doesn't require to know the allocation size on free, kmalloc() provides
> > better debugging capabilities than page allocator.
> > 
> > Another thing is that touching these allocation sites gives the reviewers
> > opportunity to see if a PAGE_SIZE buffer is actually needed or maybe
> > another size is appropriate.
> > 
> > For larger allocations that don't need physically contiguous memory
> > kvmalloc() can be a better option that __get_free_pages() because under
> > memory pressure it's is easier to allocate several order-0 pages than a
> > physically contiguous chunk with the same number of pages.
> > 
> > And last, but not least, removing needless calls to page allocator should
> > help with memdesc (aka project folio) conversion. There will be way less
> > places to audit to see if the user was actually using struct page.
> 
> I have some objections to this series, but not because of what you are
> trying to do (which is actually nice).
> 
> I understand that you probably wanted to touch as little code as
> possible,

Yep :)

> but now since you're rewriting the allocations to use
> kmalloc.... I'd like them to be converted to use the __free(kvmalloc)

You mean __free(kfree)? Sure, I can look into it.

> system. It will make the code smaller, easier to read and understand,
> less prone to future errors, etc.
> 
> In some places the whole code flow can be simplified a lot.

-- 
Sincerely yours,
Mike.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc()
  2026-09-02 13:17   ` Mike Rapoport
@ 2026-09-02 14:53     ` Claudio Imbrenda
  2026-09-03  8:04       ` Mike Rapoport
  0 siblings, 1 reply; 11+ messages in thread
From: Claudio Imbrenda @ 2026-09-02 14:53 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: Christian Borntraeger, Janosch Frank, Alexander Gordeev,
	David Hildenbrand, Heiko Carstens, Sven Schnelle, Vasily Gorbik,
	Vlastimil Babka, kvm, linux-kernel, linux-mm, linux-s390

On Wed, 2 Sep 2026 16:17:03 +0300
Mike Rapoport <rppt@kernel.org> wrote:

> On Wed, Sep 02, 2026 at 01:06:12PM +0200, Claudio Imbrenda wrote:
> > On Wed, 02 Sep 2026 09:15:12 +0300
> > "Mike Rapoport (Microsoft)" <rppt@kernel.org> wrote:
> >   
> > > This is a (small) part of larger work of replacing page allocator calls
> > > with kmalloc.
> > > 
> > > My initial intention a few month ago was to remove ugly casts [1], but then
> > > willy pointed out that Linus objected to something like this [2] and it
> > > looks like more than a decade old technical debt.
> > > 
> > > Largely, anything that doesn't need struct page (or a memdesc in the
> > > future) should just use kmalloc() or kvmalloc() to allocate memory.
> > > kmalloc() guarantees alignment, physical contiguity and working
> > > virt_to_phys() and beside nicer API that returns void * on alloc and
> > > doesn't require to know the allocation size on free, kmalloc() provides
> > > better debugging capabilities than page allocator.
> > > 
> > > Another thing is that touching these allocation sites gives the reviewers
> > > opportunity to see if a PAGE_SIZE buffer is actually needed or maybe
> > > another size is appropriate.
> > > 
> > > For larger allocations that don't need physically contiguous memory
> > > kvmalloc() can be a better option that __get_free_pages() because under
> > > memory pressure it's is easier to allocate several order-0 pages than a
> > > physically contiguous chunk with the same number of pages.
> > > 
> > > And last, but not least, removing needless calls to page allocator should
> > > help with memdesc (aka project folio) conversion. There will be way less
> > > places to audit to see if the user was actually using struct page.  
> > 
> > I have some objections to this series, but not because of what you are
> > trying to do (which is actually nice).
> > 
> > I understand that you probably wanted to touch as little code as
> > possible,  
> 
> Yep :)
> 
> > but now since you're rewriting the allocations to use
> > kmalloc.... I'd like them to be converted to use the __free(kvmalloc)  
> 
> You mean __free(kfree)? Sure, I can look into it.

yep :)

> 
> > system. It will make the code smaller, easier to read and understand,
> > less prone to future errors, etc.
> > 
> > In some places the whole code flow can be simplified a lot.  
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc()
  2026-09-02 14:53     ` Claudio Imbrenda
@ 2026-09-03  8:04       ` Mike Rapoport
  2026-09-03 13:08         ` Claudio Imbrenda
  0 siblings, 1 reply; 11+ messages in thread
From: Mike Rapoport @ 2026-09-03  8:04 UTC (permalink / raw)
  To: Claudio Imbrenda
  Cc: Christian Borntraeger, Janosch Frank, Alexander Gordeev,
	David Hildenbrand, Heiko Carstens, Sven Schnelle, Vasily Gorbik,
	Vlastimil Babka, kvm, linux-kernel, linux-mm, linux-s390

Hi Claudio,

On Wed, Sep 02, 2026 at 04:53:15PM +0200, Claudio Imbrenda wrote:
> On Wed, 2 Sep 2026 16:17:03 +0300
> Mike Rapoport <rppt@kernel.org> wrote:
> 
> > On Wed, Sep 02, 2026 at 01:06:12PM +0200, Claudio Imbrenda wrote:
> > > On Wed, 02 Sep 2026 09:15:12 +0300
> > > "Mike Rapoport (Microsoft)" <rppt@kernel.org> wrote:
> > >   
> > > > This is a (small) part of larger work of replacing page allocator calls
> > > > with kmalloc.
> > > 
> > > I have some objections to this series, but not because of what you are
> > > trying to do (which is actually nice).
> > > 
> > > I understand that you probably wanted to touch as little code as
> > > possible,  
> > 
> > Yep :)
> > 
> > > but now since you're rewriting the allocations to use
> > > kmalloc.... I'd like them to be converted to use the __free(kvmalloc)  
> > 
> > You mean __free(kfree)? Sure, I can look into it.
> 
> yep :)

Well, I looked :) 

The only place where __free(kfree) is completely safe is handle_sthyi().

In all the other functions using __free(kfree) creates a mix of of goto
and cleanup helpers and cleanup docs advise against mixing them:

https://docs.kernel.org/core-api/cleanup.html

The cleanup paths are quite involved in these functions so using
__free(kfree) there is really not trivial.

-- 
Sincerely yours,
Mike.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc()
  2026-09-03  8:04       ` Mike Rapoport
@ 2026-09-03 13:08         ` Claudio Imbrenda
  2026-09-03 15:22           ` Mike Rapoport
  0 siblings, 1 reply; 11+ messages in thread
From: Claudio Imbrenda @ 2026-09-03 13:08 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: Christian Borntraeger, Janosch Frank, Alexander Gordeev,
	David Hildenbrand, Heiko Carstens, Sven Schnelle, Vasily Gorbik,
	Vlastimil Babka, kvm, linux-kernel, linux-mm, linux-s390

On Thu, 3 Sep 2026 11:04:44 +0300
Mike Rapoport <rppt@kernel.org> wrote:

> Hi Claudio,
> 
> On Wed, Sep 02, 2026 at 04:53:15PM +0200, Claudio Imbrenda wrote:
> > On Wed, 2 Sep 2026 16:17:03 +0300
> > Mike Rapoport <rppt@kernel.org> wrote:
> >   
> > > On Wed, Sep 02, 2026 at 01:06:12PM +0200, Claudio Imbrenda wrote:  
> > > > On Wed, 02 Sep 2026 09:15:12 +0300
> > > > "Mike Rapoport (Microsoft)" <rppt@kernel.org> wrote:
> > > >     
> > > > > This is a (small) part of larger work of replacing page allocator calls
> > > > > with kmalloc.  
> > > > 
> > > > I have some objections to this series, but not because of what you are
> > > > trying to do (which is actually nice).
> > > > 
> > > > I understand that you probably wanted to touch as little code as
> > > > possible,    
> > > 
> > > Yep :)
> > >   
> > > > but now since you're rewriting the allocations to use
> > > > kmalloc.... I'd like them to be converted to use the __free(kvmalloc)    
> > > 
> > > You mean __free(kfree)? Sure, I can look into it.  
> > 
> > yep :)  
> 
> Well, I looked :) 
> 
> The only place where __free(kfree) is completely safe is handle_sthyi().
> 
> In all the other functions using __free(kfree) creates a mix of of goto
> and cleanup helpers and cleanup docs advise against mixing them:
> 
> https://docs.kernel.org/core-api/cleanup.html
> 
> The cleanup paths are quite involved in these functions so using

it looked trivial when I gave it a cursory glance, hence the reason of
my original request, but... 

> __free(kfree) there is really not trivial.

... yeah, it's actually not trivial at all.


maybe refactor the first patch, and leave the rest as it is :)



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc()
  2026-09-03 13:08         ` Claudio Imbrenda
@ 2026-09-03 15:22           ` Mike Rapoport
  0 siblings, 0 replies; 11+ messages in thread
From: Mike Rapoport @ 2026-09-03 15:22 UTC (permalink / raw)
  To: Claudio Imbrenda
  Cc: Christian Borntraeger, Janosch Frank, Alexander Gordeev,
	David Hildenbrand, Heiko Carstens, Sven Schnelle, Vasily Gorbik,
	Vlastimil Babka, kvm, linux-kernel, linux-mm, linux-s390

On Thu, Sep 03, 2026 at 03:08:52PM +0200, Claudio Imbrenda wrote:
> > 
> > The only place where __free(kfree) is completely safe is handle_sthyi().
> > 
> > In all the other functions using __free(kfree) creates a mix of of goto
> > and cleanup helpers and cleanup docs advise against mixing them:
> > 
> > https://docs.kernel.org/core-api/cleanup.html
> > 
> > The cleanup paths are quite involved in these functions so using
> 
> it looked trivial when I gave it a cursory glance, hence the reason of
> my original request, but... 
> 
> > __free(kfree) there is really not trivial.
> 
> ... yeah, it's actually not trivial at all.
> 
> 
> maybe refactor the first patch, and leave the rest as it is :)

Yeah, that's what I've been thinking too.
And leave the rest to s390 kvm experts :)

-- 
Sincerely yours,
Mike.


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-09-03 15:22 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-02  6:15 [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc() Mike Rapoport (Microsoft)
2026-09-02  6:15 ` [PATCH 1/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STHYI buffer Mike Rapoport (Microsoft)
2026-09-02  6:15 ` [PATCH 2/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer Mike Rapoport (Microsoft)
2026-09-02  6:15 ` [PATCH 3/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the GIB Mike Rapoport (Microsoft)
2026-09-02  6:15 ` [PATCH 4/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for sie_page2 and CMMA Mike Rapoport (Microsoft)
2026-09-02 11:06 ` [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc() Claudio Imbrenda
2026-09-02 13:17   ` Mike Rapoport
2026-09-02 14:53     ` Claudio Imbrenda
2026-09-03  8:04       ` Mike Rapoport
2026-09-03 13:08         ` Claudio Imbrenda
2026-09-03 15:22           ` Mike Rapoport

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox