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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, May 01, 2021 at 02:38:50PM -0700, John Hubbard wrote:
> On 4/30/21 6:32 PM, Nicholas Piggin 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).
> 
> 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.




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux