On Sat, May 01, 2021 at 11:32:20AM +1000, Nicholas Piggin wrote: > Excerpts from Hugh Dickins's message of May 1, 2021 4:47 am: > > On Fri, 30 Apr 2021, Matthew Wilcox (Oracle) wrote: > >> - 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. I'm not entirely comfortable with this. I'd like to stop modules from accessing folio_set_dirty() because it's just going to mess up filesystems. I just haven't thought of a good way to expose some flags and not others. Actually, looking at what filesystems actually use at the moment, it's quite a small subset: ClearPageChecked ClearPageDirty ClearPageError ClearPageFsCache __ClearPageLocked ClearPageMappedToDisk ClearPagePrivate2 ClearPagePrivate ClearPageReferenced ClearPageUptodate TestClearPageError TestClearPageFsCache TestClearPagePrivate2 TestClearPageDirty SetPageError __SetPageLocked SetPageMappedToDisk SetPagePrivate2 SetPagePrivate SetPageUptodate __SetPageUptodate TestSetPageDirty TestSetPageFsCache several of those are ... confused ... but the vast majority of page flags don't need to be exposed to filesystems. Does it make you feel better if folio_set_dirty() doesn't get exposed outside the VFS?
