Re: [PATCH v2 31/33] mm/sl*b: Differentiate struct slab fields by sl*b implementations

[Hyeonggon Yoo <42.hyeyoo@gmail.com>]

On Sat, Dec 11, 2021 at 04:52:47PM +0000, Matthew Wilcox wrote:
> On Sat, Dec 11, 2021 at 11:55:27AM +0000, Hyeonggon Yoo wrote:
> > On Fri, Dec 10, 2021 at 07:26:11PM +0100, Vlastimil Babka wrote:
> > > On 12/10/21 17:37, Hyeonggon Yoo wrote:
> > > > On Wed, Dec 01, 2021 at 07:15:08PM +0100, Vlastimil Babka wrote:
> > > >> With a struct slab definition separate from struct page, we can go further and
> > > >> define only fields that the chosen sl*b implementation uses. This means
> > > >> everything between __page_flags and __page_refcount placeholders now depends on
> > > >> the chosen CONFIG_SL*B.
> > > > 
> > > > When I read this patch series first, I thought struct slab is allocated
> > > > separately from struct page.
> > > > 
> > > > But after reading it again, It uses same allocated space of struct page.
> > > 
> > > Yes. Allocating it elsewhere is something that can be discussed later. It's
> > > not a simple clear win - more memory used, more overhead, complicated code...
> > >
> > 
> > Right. That is a something that can be discussed,
> > But I don't think there will be much win.
> 
> Oh no, there's a substantial win.  If we can reduce struct page to a
> single pointer, that shrinks it from 64 bytes/4k to 8 bytes/4k.  Set
> against that, you have to allocate the struct folio / struct slab / ...
> but then it's one _per allocation_ rather than one per page.  So for
> an order-2 allocation, it takes 32 bytes + 64 bytes (= 96 bytes)
> rather than 4*64 = 256 bytes.  It's an even bigger win for larger
> allocations, and it lets us grow the memory descriptors independently
> of each other.

Oh I thought there won't be much win because I thought it was
just allocating additional memory for struct slab and still allocating
memory for struct page as we do now.

It will be more efficient if we can allocate descriptor of slab/page/...etc
per *allocation*, which may have order > 1. And currently we're
duplicating memory descriptor (struct page) even on high order
allocation.

Even if we do not allocate high order page at all, it's
still efficient if we can reduce struct page into double word.
And we can allocate something like struct slab only when we need it. 

One challenge here is that we should allocate the descriptors
dynamically... I'm going to read the link you sent.

> Everything currently using struct page needs to
> be converted to use another type, and that's just the pre-requisite
> step.
> 

Oh, you're planning to separate *everything* from
struct page, not only struct slab!

So your intention of this patch series is preparing for
physical separation. It's fascinating...

> But it's also a substantial amount of work, so don't expect us to get
> there any time soon.

Yeah, that will require much work. But I'll wait for your good work.
It's so interesting.

> Some more thoughts on this here:
> https://lore.kernel.org/linux-mm/YXcLqcFhDq3uUwIj@casper.infradead.org/
> 

Thank you for the link.

> > > Yeah. Also whatever aliases with compound_head must not have bit zero set as
> > > that means a tail page.
> > > 
> > 
> > Oh I was missing that. Thank you.
> > 
> > Hmm then in struct slab, page->compound_head and slab->list_head (or
> > slab->rcu_head) has same offset. And list_head / rcu_head both store pointers.
> > 
> > then it has a alignment requirement. (address saved in list_head/rcu_head
> > should be multiple of 2)
> > 
> > Anyway, it was required long time before this patch,
> > so it is not a problem for this patch.
> 
> Yes, that's why there's an assert that the list_heads all line up.  This
> requirement will go away if we do get separately allocated memory
> descriptors (because that bottom bit is no longer PageTail).

Yeah, we don't need to care that if we separately allocate memory for
struct slab.

Thanks,
Hyeonggon.