[PATCH] block: partitions: replace __get_free_page() with kmalloc()

[PATCH] block: partitions: replace __get_free_page() with kmalloc()

[Mike Rapoport (Microsoft)]

check_partition() allocates a buffer to use as backing buffer for
seq_buf.

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

Replace use of __get_free_page() with kmalloc() and free_page() with
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
This is a (tiny) part of larger work of replacing page allocator calls
with kmalloc:

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

Signed-off-by: Mike Rapoport <rppt@kernel.org>
---
 block/partitions/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/partitions/core.c b/block/partitions/core.c
index 5d5332ce586b..b5c59b79ca7c 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -124,7 +124,7 @@ static struct parsed_partitions *check_partition(struct gendisk *hd)
 	state = allocate_partitions(hd);
 	if (!state)
 		return NULL;
-	state->pp_buf.buffer = (char *)__get_free_page(GFP_KERNEL);
+	state->pp_buf.buffer = kmalloc(PAGE_SIZE, GFP_KERNEL);
 	if (!state->pp_buf.buffer) {
 		free_partitions(state);
 		return NULL;
@@ -154,7 +154,7 @@ static struct parsed_partitions *check_partition(struct gendisk *hd)
 	if (res > 0) {
 		printk(KERN_INFO "%s", seq_buf_str(&state->pp_buf));
 
-		free_page((unsigned long)state->pp_buf.buffer);
+		kfree(state->pp_buf.buffer);
 		return state;
 	}
 	if (state->access_beyond_eod)
@@ -170,7 +170,7 @@ static struct parsed_partitions *check_partition(struct gendisk *hd)
 		printk(KERN_INFO "%s", seq_buf_str(&state->pp_buf));
 	}
 
-	free_page((unsigned long)state->pp_buf.buffer);
+	kfree(state->pp_buf.buffer);
 	free_partitions(state);
 	return ERR_PTR(res);
 }

---
base-commit: 5d6919055dec134de3c40167a490f33c74c12581
change-id: 20260520-block-25582753fd38

Best regards,
--  
Sincerely yours,
Mike.

Re: [PATCH] block: partitions: replace __get_free_page() with kmalloc()

[Christoph Hellwig]

On Wed, May 20, 2026 at 11:15:52AM +0300, Mike Rapoport (Microsoft) wrote:
> check_partition() allocates a buffer to use as backing buffer for
> seq_buf.
> 
> This buffer can be allocated with kmalloc() as there's nothing special
> about it to go directly to the page allocator.
> 
> Replace use of __get_free_page() with kmalloc() and free_page() with
> kfree().

So I heard various vague references that we should replace
__get_free_page with kmalloc, but nothing definitive.  Can you please
point to a good resource for that?

Re: [PATCH] block: partitions: replace __get_free_page() with kmalloc()

[Mike Rapoport]

On Sun, May 24, 2026 at 11:08:31PM -0700, Christoph Hellwig wrote:
> On Wed, May 20, 2026 at 11:15:52AM +0300, Mike Rapoport (Microsoft) wrote:
> > check_partition() allocates a buffer to use as backing buffer for
> > seq_buf.
> > 
> > This buffer can be allocated with kmalloc() as there's nothing special
> > about it to go directly to the page allocator.
> > 
> > Replace use of __get_free_page() with kmalloc() and free_page() with
> > kfree().
> 
> So I heard various vague references that we should replace
> __get_free_page with kmalloc, but nothing definitive.  Can you please
> point to a good resource for that?

There was quite recent discussion when I posted patches that change
__get_free_page to return void *:

https://lore.kernel.org/all/20251018093002.3660549-1-rppt@kernel.org/

And an old thread when Al posted similar patches:

https://lore.kernel.org/all/CA+55aFwp4iy4rtX2gE2WjBGFL=NxMVnoFeHqYa2j1dYOMMGqxg@mail.gmail.com/T/#u<S-Del>

-- 
Sincerely yours,
Mike.

Re: [PATCH] block: partitions: replace __get_free_page() with kmalloc()

[Christoph Hellwig]

On Mon, May 25, 2026 at 09:52:09AM +0300, Mike Rapoport wrote:
> On Sun, May 24, 2026 at 11:08:31PM -0700, Christoph Hellwig wrote:
> > On Wed, May 20, 2026 at 11:15:52AM +0300, Mike Rapoport (Microsoft) wrote:
> > > check_partition() allocates a buffer to use as backing buffer for
> > > seq_buf.
> > > 
> > > This buffer can be allocated with kmalloc() as there's nothing special
> > > about it to go directly to the page allocator.
> > > 
> > > Replace use of __get_free_page() with kmalloc() and free_page() with
> > > kfree().
> > 
> > So I heard various vague references that we should replace
> > __get_free_page with kmalloc, but nothing definitive.  Can you please
> > point to a good resource for that?
> 
> There was quite recent discussion when I posted patches that change
> __get_free_page to return void *:
> 
> https://lore.kernel.org/all/20251018093002.3660549-1-rppt@kernel.org/

This doesn't tell much more.

> And an old thread when Al posted similar patches:
> 
> https://lore.kernel.org/all/CA+55aFwp4iy4rtX2gE2WjBGFL=NxMVnoFeHqYa2j1dYOMMGqxg@mail.gmail.com/T/#u<S-Del>

This does, but it still fails to explain why kmalloc performs just as
well as __get_free_page(s) these days.

And such an explanation or link to it should go into every patch doing
this switch.

Re: [PATCH] block: partitions: replace __get_free_page() with kmalloc()

[Mike Rapoport]

On Mon, May 25, 2026 at 12:16:23AM -0700, Christoph Hellwig wrote:
> On Mon, May 25, 2026 at 09:52:09AM +0300, Mike Rapoport wrote:
> > On Sun, May 24, 2026 at 11:08:31PM -0700, Christoph Hellwig wrote:
> > > On Wed, May 20, 2026 at 11:15:52AM +0300, Mike Rapoport (Microsoft) wrote:
> > > > check_partition() allocates a buffer to use as backing buffer for
> > > > seq_buf.
> > > > 
> > > > This buffer can be allocated with kmalloc() as there's nothing special
> > > > about it to go directly to the page allocator.
> > > > 
> > > > Replace use of __get_free_page() with kmalloc() and free_page() with
> > > > kfree().
> > > 
> > > So I heard various vague references that we should replace
> > > __get_free_page with kmalloc, but nothing definitive.  Can you please
> > > point to a good resource for that?
> > 
> > There was quite recent discussion when I posted patches that change
> > __get_free_page to return void *:
> > 
> > https://lore.kernel.org/all/20251018093002.3660549-1-rppt@kernel.org/
> 
> This doesn't tell much more.
> 
> > And an old thread when Al posted similar patches:
> > 
> > https://lore.kernel.org/all/CA+55aFwp4iy4rtX2gE2WjBGFL=NxMVnoFeHqYa2j1dYOMMGqxg@mail.gmail.com/T/#u<S-Del>
> 
> This does, but it still fails to explain why kmalloc performs just as
> well as __get_free_page(s) these days.

I don't think that in this case - a single allocation on the cold path -
the performance difference is even measurable.

Nevertheless allocations from slab caches are way faster than
__get_free_page() (i.e.  alloc_pages()) as it's essentially lockless
cmpxchg. Allocations that need to refill the cache do alloc_pages() with a
little of slab bookkeeping overhead.

-- 
Sincerely yours,
Mike.

Re: [PATCH] block: partitions: replace __get_free_page() with kmalloc()

[Christoph Hellwig]

On Mon, May 25, 2026 at 12:35:56PM +0300, Mike Rapoport wrote:
> > This does, but it still fails to explain why kmalloc performs just as
> > well as __get_free_page(s) these days.
> 
> I don't think that in this case - a single allocation on the cold path -
> the performance difference is even measurable.

Well, please state that.

> Nevertheless allocations from slab caches are way faster than
> __get_free_page() (i.e.  alloc_pages()) as it's essentially lockless
> cmpxchg. Allocations that need to refill the cache do alloc_pages() with a
> little of slab bookkeeping overhead.

Please state that too.