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:   Thu, 18 Mar 2021 17:57:51 +0000
From:   Matthew Wilcox <willy@infradead.org>
To:     Christoph Hellwig <hch@infradead.org>
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v4 08/25] mm: Handle per-folio private data
Message-ID: <20210318175751.GS3420@casper.infradead.org>
References: <20210305041901.2396498-1-willy@infradead.org>
 <20210305041901.2396498-9-willy@infradead.org>
 <YFI6YOe2uU/n2vR6@infradead.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YFI6YOe2uU/n2vR6@infradead.org>
Precedence: bulk

On Wed, Mar 17, 2021 at 06:20:32PM +0100, Christoph Hellwig wrote:
> > +static inline void attach_page_private(struct page *page, void *data)
> > +{
> > +	attach_folio_private((struct folio *)page, data);
> > +}
> > +
> > +static inline void *detach_page_private(struct page *page)
> > +{
> > +	return detach_folio_private((struct folio *)page);
> > +}
> 
> I hate these open code casts.  Can't we have a single central
> page_to_folio helper, which could also grow a debug check (maybe
> under a new config option) to check that it really is called on a
> head page?

Some of that is already there.  We have page_folio() which is the
page_to_folio() helper you're asking for.  And folio_flags() (which is
called *all the time*) contains
        VM_BUG_ON_PGFLAGS(PageTail(page), page);
Someone passing around a tail pointer cast to a folio is not going to
get very far, assuming CONFIG_DEBUG_VM_PGFLAGS is enabled (most distros
don't, but I do when I'm testing anything THPish).

These helpers aren't going to live for very long ... I expect to have
all filesystems which use attach/detach page private converted to folios
pretty soon.  Certainly before any of them _use_ multi-page folios.

Anyway, the simple thing to do is just to use page_folio() here and eat
the cost of calling compound_head() on something we're certain is an
order-0 page.  It only defers the win of removing the compound_head()
call; it doesn't preclude it.  And it means we're not setting a bad
example here (there really shouldn't be any casts from pages to folios,
except in the folio allocator, which uses the page allocator and then
casts what _must be_ a non-tail page to a folio).