Skip to contentSkip to conversation

block: partitions: replace __get_free_page() with kmalloc()

PATCH Superseded Mike Rapoport (Microsoft) 5 participants 12 messages May 20 – May 27

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 block: partitions: replace __get_free_page() with kmalloc()

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().

---
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

---
 block/partitions/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
block/partitions/core.c +3 -3
index 5d5332ce586b..b5c59b79ca7c 100644
@@ -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,

Signature · 2 lines
Sincerely yours,
Mike.
#1 lore raw
Quoted 8 lines · 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().

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?

#2 lore raw
Quoted 7 lines · Christoph Hellwig
Quoted 6 lines · 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.

Quoted 6 lines
Quoted 2 lines

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/[email protected]/

And an old thread when Al posted similar patches:

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

Signature · 2 lines
Sincerely yours,
Mike.
#3 lore raw
Quoted 11 lines · Mike Rapoport
Quoted 10 lines · Christoph Hellwig
Quoted 8 lines · 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().

Quoted 8 lines
Quoted 3 lines

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/[email protected]/

This doesn't tell much more.

Quoted 3 lines

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.

#4 lore raw
Quoted 21 lines · Christoph Hellwig
Quoted 11 lines · Mike Rapoport
Quoted 10 lines · Christoph Hellwig
Quoted 8 lines · 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().

Quoted 8 lines
Quoted 3 lines

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/[email protected]/

Quoted 8 lines

This doesn't tell much more.

Quoted 3 lines

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.

Signature · 2 lines
Sincerely yours,
Mike.
#5 lore raw
Quoted 5 lines · Mike Rapoport
Quoted 2 lines

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.

Quoted 4 lines

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.

#6 lore raw
In reply to Mike Rapoport
Quoted 11 lines · Mike Rapoport
Quoted 3 lines · Christoph Hellwig

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

Probably not "way faster" but the fast path is quite similar - percpu pcplist protected by spin_trylock (pages) vs sheaves with local_trylock (slab), should slightly favour slab because spinlocks are typically not inlined and local_trylock is.

The main reasons for switching AFAIU would be related with the folio/memdesc conversions? If one needs just a kernel memory buffer, kmalloc() it is, even if it happens to be page size. Page allocator should be only used if you need e.g. the refcounting or anything else that struct page provides. But then in some cases the memdesc conversion would need adjustments at some point. With kmalloc() we can forget about this user.

Matthew can probably state it better or even link to something authoritative?

Quoted 2 lines

little of slab bookkeeping overhead.

#7 lore raw
Quoted 7 lines · Vlastimil Babka

The main reasons for switching AFAIU would be related with the folio/memdesc conversions? If one needs just a kernel memory buffer, kmalloc() it is, even if it happens to be page size. Page allocator should be only used if you need e.g. the refcounting or anything else that struct page provides. But then in some cases the memdesc conversion would need adjustments at some point. With kmalloc() we can forget about this user.

No, I think this is unrelated to memdescs.

I've seen a few people say slightly wrong things about folios/pages/memdescs recently, so let me try to clarify the end state.

I do not intend to get rid of the ability to allocate a bare page of memory with something like alloc_pages() or get_free_page(). It's just that the struct page associated with it will contain far less information (because it's smaller).

https://kernelnewbies.org/MatthewWilcox/Memdescs has a bit more information, but to distill it:

You get a u64 worth of data (technically one per page, but if you allocate multiple pages, they're all going to be the same). Bits 0-3 will be type 0 (to indicate that it has no memdesc). Bits 4-10 will be subtype 2 (to indicate no information about owner). Bit 11 will be clear to indicate that this page should not be mappable to userspace.
Bits 12-17 will store the allocation order.
The top few bits will encode zone/node/section like page->flags do today.

That doesn't leave many free bits for the user, but that's OK because most allocations don't actually need any bits in struct page. If you do want something like a refcount or list_head, see the "Managed memory" section on that page. If you actually want a full-fat folio, well, allocate a folio, not a page.

#8 lore raw
Quoted 11 lines · Matthew Wilcox
Quoted 7 lines · Vlastimil Babka

The main reasons for switching AFAIU would be related with the folio/memdesc conversions? If one needs just a kernel memory buffer, kmalloc() it is, even if it happens to be page size. Page allocator should be only used if you need e.g. the refcounting or anything else that struct page provides. But then in some cases the memdesc conversion would need adjustments at some point. With kmalloc() we can forget about this user.

No, I think this is unrelated to memdescs.

Quoted 7 lines

I've seen a few people say slightly wrong things about folios/pages/memdescs recently, so let me try to clarify the end state.

I do not intend to get rid of the ability to allocate a bare page of memory with something like alloc_pages() or get_free_page(). It's just that the struct page associated with it will contain far less information (because it's smaller).

Alright, but isn't it still the case that if you don't need any of what struct page provides today or will do in the future, it's better if you just use kmalloc()? I thought you said so yourself?

https://lore.kernel.org/all/[email protected]/

So what exactly would your rationale for "Most of them shouldn't be using get_free_pages() at all, they should be using kmalloc()." be?

Quoted 18 lines

https://kernelnewbies.org/MatthewWilcox/Memdescs has a bit more information, but to distill it:

You get a u64 worth of data (technically one per page, but if you allocate multiple pages, they're all going to be the same). Bits 0-3 will be type 0 (to indicate that it has no memdesc). Bits 4-10 will be subtype 2 (to indicate no information about owner). Bit 11 will be clear to indicate that this page should not be mappable to userspace.
Bits 12-17 will store the allocation order.
The top few bits will encode zone/node/section like page->flags do today.

That doesn't leave many free bits for the user, but that's OK because most allocations don't actually need any bits in struct page. If you do want something like a refcount or list_head, see the "Managed memory" section on that page. If you actually want a full-fat folio, well, allocate a folio, not a page.

#9 lore raw
Quoted 20 lines · Vlastimil Babka
Quoted 11 lines · Matthew Wilcox
Quoted 7 lines · Vlastimil Babka

The main reasons for switching AFAIU would be related with the folio/memdesc conversions? If one needs just a kernel memory buffer, kmalloc() it is, even if it happens to be page size. Page allocator should be only used if you need e.g. the refcounting or anything else that struct page provides. But then in some cases the memdesc conversion would need adjustments at some point. With kmalloc() we can forget about this user.

No, I think this is unrelated to memdescs.

Quoted 7 lines

I've seen a few people say slightly wrong things about folios/pages/memdescs recently, so let me try to clarify the end state.

I do not intend to get rid of the ability to allocate a bare page of memory with something like alloc_pages() or get_free_page(). It's just that the struct page associated with it will contain far less information (because it's smaller).

Quoted 6 lines

Alright, but isn't it still the case that if you don't need any of what struct page provides today or will do in the future, it's better if you just use kmalloc()? I thought you said so yourself?

https://lore.kernel.org/all/[email protected]/

Precisely my reasoning. In most cases, __get_free_page() is just a
lazy way of saying "I need some memory and the allocation should not fail". And typically these callers don't really care about the page mapping, too.
Additionally, these applications get in the way when using large block
sizes on the system, as they needlessly increase the pressure on
compaction.
So switching them over to kmalloc() is a good thing IMO.
If nothing else it allows us to differentiate which places _actually_
need struct page, and which callers just want to do a memory allocation.

Cheers,

Hannes

Signature · 4 lines
Dr. Hannes Reinecke                  Kernel Storage Architect
[email protected]                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
#10 lore raw
Quoted 2 lines · Hannes Reinecke

Precisely my reasoning. In most cases, __get_free_page() is just a lazy way of saying "I need some memory and the allocation should not fail".

Huh? __get_free_page and kmalloc can and should fail.

#11 lore raw
Quoted 6 lines · Christoph Hellwig
Quoted 2 lines · Hannes Reinecke

Precisely my reasoning. In most cases, __get_free_page() is just a lazy way of saying "I need some memory and the allocation should not fail".

Huh? __get_free_page and kmalloc can and should fail.

Even better, one argument less why it should not be converted :-)

Cheers,

Hannes

Signature · 4 lines
Dr. Hannes Reinecke                  Kernel Storage Architect
[email protected]                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich