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

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

 



On 5/1/21 5:17 PM, Matthew Wilcox wrote:
...
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.

Yes, the page dirty stuff is definitely not simple, so it's very good to
move away from the auto-generated names there. Looks like you are down to
just a couple of generated names now, if I'm reading this correctly.


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

Yes, that's a good guideline: no special cases in the auto-generated function
names. Because, either it is a simple, standard auto-generated thing, or
it is something that one needs to read, in order to see exactly what it does.


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

If "kernel-doc" is effectively a proxy for "file names are directly visible
in the source code, then I'm a lot happier than I was, yes. :)

to undocumented folio_test_set_foo_flag(), but it's all regular and
autogenerated.


Sounds pretty good.


thanks,
--
John Hubbard
NVIDIA



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux