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

From: Matthew Wilcox <willy@infradead.org>
To: John Hubbard <jhubbard@nvidia.com>
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: Sun, 2 May 2021 03:31:13 +0100	[thread overview]
Message-ID: <20210502023113.GX1847222@casper.infradead.org> (raw)
In-Reply-To: <f8c968d9-e66a-3588-3811-3efe704650ae@nvidia.com>

On Sat, May 01, 2021 at 05:42:21PM -0700, John Hubbard wrote:
> On 5/1/21 5:17 PM, Matthew Wilcox wrote:
> > 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.

Six -- test_set, test_clear, __set, __clear, set, clear.

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

I'm thinking about this kind of thing for each flag (uptodate was the
easiest one to start with because it's already not autogenerated):

/**
 * folio_uptodate - Is this folio up to date?
 * @folio: The folio.
 *
 * The uptodate flag is set on a folio when every byte in the folio is at
 * least as new as the corresponding bytes on storage.  Anonymous folios
 * are always uptodate.  If the folio is not uptodate, some of the bytes
 * in it may be; see the is_partially_uptodate() address_space operation.
 */
static inline bool folio_uptodate(struct folio *folio)
{
...

(um, this is going to increase the patch series significantly.  i may
not do this until later.  there's more important things to get in that
are already done and waiting on this initial patch series.)