Re: [PATCH v8.1 00/31] Memory Folios

On Sat, May 01, 2021 at 03:37:11AM +0100, Matthew Wilcox wrote:

On Sat, May 01, 2021 at 11:32:20AM +1000, Nicholas Piggin wrote:

quoted

Excerpts from Hugh Dickins's message of May 1, 2021 4:47 am:

quoted

On Fri, 30 Apr 2021, Matthew Wilcox (Oracle) wrote:

quoted

 - Big renaming (thanks to peterz):
   - PageFoo() becomes folio_foo()
   - SetFolioFoo() becomes folio_set_foo()
   - ClearFolioFoo() becomes folio_clear_foo()
   - __SetFolioFoo() becomes __folio_set_foo()
   - __ClearFolioFoo() becomes __folio_clear_foo()
   - TestSetPageFoo() becomes folio_test_set_foo()
   - TestClearPageFoo() becomes folio_test_clear_foo()
   - PageHuge() is now folio_hugetlb()

If you rename these things at the same time, can you make it clear 
they're flags (folio_flag_set_foo())? The weird camel case accessors at 
least make that clear (after you get to know them).

We have a set_page_dirty(), so page_set_dirty() would be annoying.
page_flag_set_dirty() keeps the easy distinction that SetPageDirty()
provides.

Maybe I should have sent more of the patches in this batch ...

mark_page_accessed() becomes folio_mark_accessed()
set_page_dirty() becomes folio_mark_dirty()
set_page_writeback() becomes folio_start_writeback()
test_clear_page_writeback() becomes __folio_end_writeback()
cancel_dirty_page() becomes folio_cancel_dirty()
clear_page_dirty_for_io() becomes folio_clear_dirty_for_io()
lru_cache_add() becomes folio_add_lru()
add_to_page_cache_lru() becomes folio_add_to_page_cache()
write_one_page() becomes folio_write_one()
account_page_redirty() becomes folio_account_redirty()
account_page_cleaned() becomes folio_account_cleaned()

So the general pattern is that folio_set_foo() and folio_clear_foo()
works on the flag directly.  If we do anything fancy to it, it's
folio_verb_foo() where verb depends on foo.

After sleeping on this, I now think "Why not both?"

folio_dirty() -- defined in page-flags.h

folio_test_set_dirty_flag()
folio_test_clear_dirty_flag()
__folio_clear_dirty_flag()
__folio_set_dirty_flag()
folio_clear_dirty_flag()
folio_set_dirty_flag() -- generated in filemap.h under #ifndef MODULE

folio_mark_dirty() -- declared in mm.h (this is rare; turns out all kinds of
			crap wants to mark pages as being dirty)
folio_clear_dirty_for_io() -- declared in filemap.h


the other flags would mostly follow this pattern.  i'd also change
folio_set_uptodate() to folio_mark_uptodate().