linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: Linux MM <linux-mm@kvack.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	 linux-kernel <linux-kernel@vger.kernel.org>,
	Christoph Hellwig <hch@infradead.org>
Subject: Re: [RFC] Convert sysv filesystem to use folios exclusively
Date: Fri, 2 Apr 2021 09:19:36 +0300	[thread overview]
Message-ID: <CAOQ4uxikP_GFNYzgatON2dRQyiHvTBP5iO4Xk091ruLUBDMt-w@mail.gmail.com> (raw)
In-Reply-To: <20210325032202.GS1719932@casper.infradead.org>

On Thu, Mar 25, 2021 at 5:43 AM Matthew Wilcox <willy@infradead.org> wrote:
>
>
> I decided to see what a filesystem free from struct page would look
> like.  I chose sysv more-or-less at random; I wanted a relatively simple
> filesystem, but I didn't want a toy.  The advantage of sysv is that the
> maintainer is quite interested in folios ;-)
>
> $ git grep page fs/sysv
> fs/sysv/dir.c:#include <linux/pagemap.h>
> fs/sysv/dir.c:          if (offset_in_page(diter->pos)) {
> fs/sysv/inode.c:        .get_link       = page_get_link,
> fs/sysv/inode.c:        truncate_inode_pages_final(&inode->i_data);
> fs/sysv/itree.c:        block_truncate_page(inode->i_mapping, inode->i_size, get_block);
> fs/sysv/itree.c:                truncate_pagecache(inode, inode->i_size);
> fs/sysv/itree.c:        .readpage = sysv_read_folio,
> fs/sysv/itree.c:        .writepage = sysv_write_folio,

I would like to address only the social engineering aspect of the s/page/folio
conversion.

Personally, as a filesystem developer, I find the concept of the folio
abstraction very useful and I think that the word is short, clear and witty.

But the example above (writepage = sysv_write_folio) just goes to show
how deeply rooted the term 'page' is throughout the kernel and this is
just the tip of the iceberg. There are documents, comments and decades
of using 'page' in our language - those will be very hard to root out.

My first impression from looking at sample patches is that 90% of the churn
does not serve any good purpose and by that, I am referring to the
conversion of local variable names and struct field names s/page/folio.

Those conversions won't add any clarity to subsystems that only need to
deal with the simple page type (i.e. non-tail pages).
The compiler type checks will have already did that job already and changing
the name of the variables does not help in this case.

I think someone already proposed the "boring" name struct page_head as
a replacement for the "cool" name struct folio.

Whether it's page_head, page_ref or what not, anything that can
be written in a way that these sort of "thin" conversions make sense:

-static int sysv_readpage(struct file *file, struct page *page)
+static int sysv_readpage(struct file *file, struct page_head *page)
 {
       return block_read_full_page(page, get_block);
 }

So when a filesystem developer reviews your conversion patch
he goes: "Whatever, if the compiler says this is fine, it's fine".

Thanks,
Amir.


  reply	other threads:[~2021-04-02  6:19 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-25  3:22 [RFC] Convert sysv filesystem to use folios exclusively Matthew Wilcox
2021-04-02  6:19 ` Amir Goldstein [this message]
2021-04-02 13:50   ` 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=CAOQ4uxikP_GFNYzgatON2dRQyiHvTBP5iO4Xk091ruLUBDMt-w@mail.gmail.com \
    --to=amir73il@gmail.com \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.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).