Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 16 Aug 2021 03:35:18 +0100
From:   Matthew Wilcox <willy@infradead.org>
To:     David Howells <dhowells@redhat.com>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        linux-fsdevel@vger.kernel.org, Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH v14 084/138] mm/page_alloc: Add folio allocation functions
Message-ID: <YRnO5gudWRvGjxW4@casper.infradead.org>
References: <20210715033704.692967-85-willy@infradead.org>
 <20210715033704.692967-1-willy@infradead.org>
 <1814546.1628632283@warthog.procyon.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1814546.1628632283@warthog.procyon.org.uk>
Precedence: bulk

On Tue, Aug 10, 2021 at 10:51:23PM +0100, David Howells wrote:
> Matthew Wilcox (Oracle) <willy@infradead.org> wrote:
> 
> > +struct folio *folio_alloc(gfp_t gfp, unsigned order)
> > +{
> > +	struct page *page = alloc_pages(gfp | __GFP_COMP, order);
> > +
> > +	if (page && order > 1)
> > +		prep_transhuge_page(page);
> 
> Ummm...  Shouldn't order==1 pages (two page folios) be prep'd also?

No.  The deferred list is stored in the second tail page, so there's
nowhere to store one if there are only two pages.

The free_transhuge_page() dtor only handles the deferred list, so
it's fine to skip setting the DTOR in the page too.

> Would it be better to just jump to alloc_pages() if order <= 1?  E.g.:
> 
> struct folio *folio_alloc(gfp_t gfp, unsigned order)
> {
> 	struct page *page;
> 
> 	if (order <= 1)
> 		return (struct folio *)alloc_pages(gfp | __GFP_COMP, order);
> 
> 	page = alloc_pages(gfp | __GFP_COMP, order);
> 	if (page)
> 		prep_transhuge_page(page);
> 	return (struct folio *)page;
> }

That doesn't look simpler to me?