Re: [PATCH v8.1 00/31] Memory Folios
From: John Hubbard <jhubbard@nvidia.com>
Date: 2021-05-02 00:42:28
Also in:
linux-fsdevel
On 5/1/21 5:17 PM, Matthew Wilcox wrote: ...
quoted
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.
quoted
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().quoted
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