On Thu, May 26, 2022 at 03:28:25PM +0100, Matthew Wilcox wrote: > On Thu, May 26, 2022 at 11:48:32AM +0300, Dan Carpenter wrote: > > On Thu, May 26, 2022 at 02:16:34AM +0100, Matthew Wilcox wrote: > > > Bizarre this started showing up now. The recent patch was: > > > > > > - info->alloced += compound_nr(page); > > > - inode->i_blocks += BLOCKS_PER_PAGE << compound_order(page); > > > + info->alloced += folio_nr_pages(folio); > > > + inode->i_blocks += BLOCKS_PER_PAGE << folio_order(folio); > > > > > > so it could tell that compound_order() was small, but folio_order() > > > might be large? > > > > The old code also generates a warning on my test system. Smatch thinks > > both compound_order() and folio_order() are 0-255. I guess because of > > the "unsigned char compound_order;" in the struct page. > > It'd be nice if we could annotate that as "contains a value between > 1 and BITS_PER_LONG - PAGE_SHIFT". Then be able to optionally enable > a checker that ensures that's true on loads/stores. Maybe we need a > language that isn't C :-P Ada can do this ... I don't think Rust can. Machine Parsable Comments. It's a matter of figuring out the best format and writing the code. In Smatch, I have table of hard coded return values in the format: <function> <old return> <new hard coded return> https://github.com/error27/smatch/blob/master/smatch_data/db/kernel.return_fixes I don't have code to handle something like BITS_PER_LONG or PAGE_SHIFT. To be honest, Smatch code always assumes that PAGE_SIZE is 4096 but I should actually look it up... It's not impossible to do. The GFP_KERNEL values changed enough so that I eventually made that look up the actual defines. I also have a table in the database where I could edit the values of (struct page)->compound_order. regards, dan carpenter
