From: John Hubbard <jhubbard@nvidia.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: Nicholas Piggin <npiggin@gmail.com>,
Hugh Dickins <hughd@google.com>, <akpm@linux-foundation.org>,
<linux-fsdevel@vger.kernel.org>, <linux-mm@kvack.org>,
Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH v8.1 00/31] Memory Folios
Date: Sat, 1 May 2021 17:42:21 -0700 [thread overview]
Message-ID: <f8c968d9-e66a-3588-3811-3efe704650ae@nvidia.com> (raw)
In-Reply-To: <20210502001705.GW1847222@casper.infradead.org>
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
next prev parent reply other threads:[~2021-05-02 0:42 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-30 18:07 [PATCH v8.1 00/31] Memory Folios Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 01/31] mm: Introduce struct folio Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 02/31] mm: Add folio_pgdat and folio_zone Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 03/31] mm/vmstat: Add functions to account folio statistics Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 04/31] mm/debug: Add VM_BUG_ON_FOLIO and VM_WARN_ON_ONCE_FOLIO Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 05/31] mm: Add folio reference count functions Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 06/31] mm: Add folio_put Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 07/31] mm: Add folio_get Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 08/31] mm: Add folio flag manipulation functions Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 09/31] mm: Add folio_young() and folio_idle() Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 10/31] mm: Handle per-folio private data Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 11/31] mm/filemap: Add folio_index, folio_file_page and folio_contains Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 12/31] mm/filemap: Add folio_next_index Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 13/31] mm/filemap: Add folio_offset and folio_file_offset Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 14/31] mm/util: Add folio_mapping and folio_file_mapping Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 15/31] mm: Add folio_mapcount Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 16/31] mm/memcg: Add folio wrappers for various functions Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 17/31] mm/filemap: Add folio_unlock Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 18/31] mm/filemap: Add folio_lock Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 19/31] mm/filemap: Add folio_lock_killable Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 20/31] mm/filemap: Add __folio_lock_async Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 21/31] mm/filemap: Add __folio_lock_or_retry Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 22/31] mm/filemap: Add folio_wait_locked Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 23/31] mm/swap: Add folio_rotate_reclaimable Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 24/31] mm/filemap: Add folio_end_writeback Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 25/31] mm/writeback: Add folio_wait_writeback Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 26/31] mm/writeback: Add folio_wait_stable Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 27/31] mm/filemap: Add folio_wait_bit Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 28/31] mm/filemap: Add folio_wake_bit Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 29/31] mm/filemap: Convert page wait queues to be folios Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 30/31] mm/filemap: Add folio private_2 functions Matthew Wilcox (Oracle)
2021-04-30 18:07 ` [PATCH v8 31/31] fs/netfs: Add folio fscache functions Matthew Wilcox (Oracle)
2021-04-30 18:47 ` [PATCH v8.1 00/31] Memory Folios Hugh Dickins
2021-05-01 1:32 ` Nicholas Piggin
2021-05-01 2:37 ` Matthew Wilcox
2021-05-01 14:31 ` Matthew Wilcox
2021-05-01 21:38 ` John Hubbard
2021-05-02 0:17 ` Matthew Wilcox
2021-05-02 0:42 ` John Hubbard [this message]
2021-05-02 0:45 ` John Hubbard
2021-05-02 2:31 ` Matthew Wilcox
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f8c968d9-e66a-3588-3811-3efe704650ae@nvidia.com \
--to=jhubbard@nvidia.com \
--cc=akpm@linux-foundation.org \
--cc=hughd@google.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=npiggin@gmail.com \
--cc=torvalds@linux-foundation.org \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).