Re: [PATCH 01/17] mm/sparse: drop power-of-2 size requirement for struct mem_section

[David Laight]

On Wed, 15 Jul 2026 21:15:45 +0800
Muchun Song <muchun.song@xxxxxxxxx> wrote:

> > On Jul 15, 2026, at 17:34, David Laight <david.laight.linux@xxxxxxxxx> wrote:
> > 
> > On Thu,  2 Jul 2026 17:38:05 +0800
> > Muchun Song <songmuchun@xxxxxxxxxxxxx> wrote:
> >   
> >> struct mem_section is currently forced to a power-of-2 size so the
> >> section-to-root lookup can use a mask instead of a modulo.
> >> 
> >> That requirement adds configuration-dependent padding, especially with
> >> CONFIG_PAGE_EXTENSION, just to preserve the lookup scheme.
> >> 
> >> Drop the constraint and use a plain modulo for the lookup instead. The
> >> divisor is constant, so the generated code remains cheap while avoiding
> >> the extra padding. It also removes an unnecessary layout constraint
> >> from the type.  
> > 
> > This has a side effect of changing the size of the 'section' from
> > PAGE_SIZE to something 'a bit smaller' when CONFIG_PAGE_EXTENSION
> > is defined.
> > I don't think it actually matters, the allocation is done by:
> > 
> > static noinline struct mem_section __ref *sparse_index_alloc(int nid)
> > {
> > 	struct mem_section *section = NULL;
> > 	unsigned long array_size = SECTIONS_PER_ROOT *
> > 				   sizeof(struct mem_section);
> > 
> > 	if (slab_is_available()) {
> > 		section = kzalloc_node(array_size, GFP_KERNEL, nid);
> > 	} else {
> > 		section = memblock_alloc_node(array_size, SMP_CACHE_BYTES,
> > 					      nid);
> > 
> > so the size might get rounded up to PAGE_SIZE anyway.  
> 
> I'm not sure I really understand what you mean. You might be asking whether
> the reduction in the size of the `mem_section` structure does not actually result
> in memory savings? If so, please let me explain clearly. As you mentioned, the
> size of memory allocated each time here should be PAGE_SIZE. Before the
> modification, one page could hold 4096/32 = 128 `struct mem_section` instances;
> with the modified code, the number of `struct mem_section` instances that can fit
> is 4096/24 = 170. Therefore, the range of memory sections that a PAGE_SIZE can
> cover has increased 32%.
> 
> Please let me know if I didn’t get your point.

170 * 24 is 4080 - 16 bytes less than PAGE_SIZE.
In principle kmalloc() need not allocate a full page for it which would
lead to the data crossing a page boundary - which may not be intended.

> 
> > 
> > I also suspect that '% 24u' might be enough slower than '% 32u' to
> > generate a measurable performance drop.
> > (It doesn't matter whether you do '& 31' or '% 32u'.)  
> 
> David, I agree that % 24u is slower than % 32u — the latter maps to ‘and', while the
> former requires a multiply-shift sequence. However, since the divisor is a constant,
> the compiler should uses the magic multiplier approach at -O2, which is ~3-5 cycles
> instead of 1. So I think the per-lookup overhead is real but small.

The divide/remainder is by 170 not 24 - but the effect is the same.

Indeed, but it might be a hot enough path to be measurable.
I've just done a pile of test compiles, see https://godbolt.org/z/zq8WjaqGj

The remainder code has to do the divide, gcc might notice it has already
done it - but it would be better to do it explicitly.
However, both sparc64 and looooongarch64 end up executing divides.
Also some of the others aren't as short as you might expect, both
generating the constant for the multiply and avoiding a 'mul' instruction
for the remainder (esp. s390).

	David

> 
> Thanks for your review.
> 
> Muchun
> 
> > 
> > 	David  
>