Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Sun, 29 May 2022 04:31:07 +0100
From:   Matthew Wilcox <willy@infradead.org>
To:     Muchun Song <songmuchun@bytedance.com>
Cc:     bh1scw@gmail.com, Christoph Lameter <cl@linux.com>,
        Pekka Enberg <penberg@kernel.org>,
        David Rientjes <rientjes@google.com>,
        Joonsoo Kim <iamjoonsoo.kim@lge.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Vlastimil Babka <vbabka@suse.cz>,
        Roman Gushchin <roman.gushchin@linux.dev>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/slub: replace alloc_pages with folio_alloc
Message-ID: <YpLo+4U8k+OLTmz9@casper.infradead.org>
References: <20220528161157.3934825-1-bh1scw@gmail.com>
 <YpJNX7PN8hAFgVwj@casper.infradead.org>
 <YpLhSokkrPrXjNXP@FVFYT0MHHV2J.googleapis.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YpLhSokkrPrXjNXP@FVFYT0MHHV2J.googleapis.com>
Precedence: bulk

On Sun, May 29, 2022 at 10:58:18AM +0800, Muchun Song wrote:
> On Sat, May 28, 2022 at 05:27:11PM +0100, Matthew Wilcox wrote:
> > On Sun, May 29, 2022 at 12:11:58AM +0800, bh1scw@gmail.com wrote:
> > > From: Fanjun Kong <bh1scw@gmail.com>
> > > 
> > > This patch will use folio allocation functions for allocating pages.
> > 
> > That's not actually a good idea.  folio_alloc() will do the
> > prep_transhuge_page() step which isn't needed for slab.
> 
> You mean folio_alloc() is dedicated for THP allocation?  It is a little
> surprise to me.  I thought folio_alloc() is just a variant of alloc_page(),
> which returns a folio struct instead of a page.  Seems like I was wrong.
> May I ask what made us decide to do this?

Yeah, the naming isn't great here.  The problem didn't really occur
to me until I saw this patch, and I don't have a good solution yet.
We're in the middle of a transition, but the transition is likely to
take years and I don't think we necessarily have the final form of the
transition fully agreed to or understood, so we should come up with
something better for the transition.

Ignoring the naming here, memory allocated to filesystems can be split,
but the split can fail, so they need the page-deferred-list and the
DTOR.  Memory allocated to slab cannot be split, so initialising the
page-deferred-list is a waste of time.

The end-goal is to split apart allocating the memory from allocating
its memory descriptor (which I like to call memdesc).  So for filesystem
folios, we'd call slab to allocate a struct folio and then tell the
buddy allocator "here is the memdesc of type folio, allocate
me 2^n pages and make pfn_to_memdesc return this memdesc for each of
the 2^n pages in it".

In this end-goal, slab would also allocate a struct slab (... there's
a recursion problem here which has a solution ...), and then allocate
2^n pages.  But until we're ready to shrink struct page down to one
or two words, doing this is just a waste of memory and time.

So I still don't have a good solution to receiving patches like this
other than maybe adding a comment like

	/* Do not change this to allocate a folio */

which will be ignored.