Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210325032202.GS1719932@casper.infradead.org>
In-Reply-To: <20210325032202.GS1719932@casper.infradead.org>
From:   Amir Goldstein <amir73il@gmail.com>
Date:   Fri, 2 Apr 2021 09:19:36 +0300
Message-ID: <CAOQ4uxikP_GFNYzgatON2dRQyiHvTBP5iO4Xk091ruLUBDMt-w@mail.gmail.com>
Subject: Re: [RFC] Convert sysv filesystem to use folios exclusively
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>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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.