Re: [PATCH v2 02/57] mm: Add the first tail page to struct folio

[Matthew Wilcox <willy@infradead.org>]

On Fri, Sep 02, 2022 at 04:28:30PM -0700, Andrew Morton wrote:
> On Fri,  2 Sep 2022 20:45:58 +0100 "Matthew Wilcox (Oracle)" <willy@infradead.org> wrote:
> 
> > Some of the static checkers get confused by extracting the page from
> > the folio and referring to fields in the first tail page.  Adding these
> > fields to struct folio lets us avoid doing that.  It has the risk that
> > people will refer to those fields without checking that the folio is
> > actually a large folio, so prefix them with underscores and document
> > the preferred function to use instead.
> > 
> 
> Huh.  Silly checkers.
> 
> Which one(s)?

I have a report from Coverity rendered as an image because apparently
you have to have an authorised account.  I applied for one back in April
and that disappeared into a void.

https://www.infradead.org/~willy/linux/image.png is what I was sent
by someone who did get an account.

I don't know that it's all that silly.  Given:

struct folio {
	union {
		...
		struct page;
	}
};

then referring to folio->page[1] seems like a pretty good thing to warn
about.

> > +	unsigned long _flags_1;
> > +	unsigned long __head;
> > +	unsigned char _folio_dtor;
> > +	unsigned char _folio_order;
> > +	atomic_t _total_mapcount;
> > +	atomic_t _pincount;
> > +#ifdef CONFIG_64BIT
> > +	unsigned int _folio_nr_pages;
> > +#endif
> >  };
> 
> Do we really want to muck things up like this?  Can it at least be
> declared temporary until the checker(s?) are fixed?

I'm more inclined to go the other way -- remove these elements from struct
page.  I can do some followup patches in that direction if you like?