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@xxxxxxxxxxxxx> 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?
