On Sat, May 01, 2021 at 02:38:50PM -0700, John Hubbard wrote:

On 4/30/21 6:32 PM, Nicholas Piggin wrote:
...

quoted

quoted

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).

In addition to pointing out that the name was a page flag, the weird
camel case also meant, "if you try to search for this symbol, you will
be defeated", because the darn thing is constructed via macro
concatenation.

I've always hated that, FWIW.  And you can't add kernel-doc for them
because kernel-doc doesn't understand cpp.  So my current plan (quoting
my other email):

folio_dirty() -- defined in page-flags.h
would have kernel-doc, would be greppable

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
would not have kernel-doc, would not be greppable, would only be used
in core vfs and core mm.

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
already have kernel-doc, are greppable, used by filesystems and sometimes
other random code.

Except that over time, it turned out to be not quite that simple, and
people started adding functionality. So now it's "cannot find it, and
it's also got little goodies hiding in there--maybe!".

I also don't like that.  With what I'm thinking, there are no special
cases hidden in the autogenerated names.  Special things like the current
SetPageUptodate would be in folio_mark_uptodate() and filesystems couldn't
even call folio_set_uptodate().

Given all that, I'd argue for either:
    b) changing a bunch of the items to actual written-out names. What's
       the harm? We'd end up with a longer file, but one could grep or
       cscope for the names.

I hope the above makes you happy -- everything a filesystem author needs
gets kernel-doc.  People working inside the VM/VFS still get exposed
to undocumented folio_test_set_foo_flag(), but it's all regular and
autogenerated.