linux-kernel - Re: [RFC] Convert sysv filesystem to use folios exclusively

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOQ4uxikP_GFNYzgatON2dRQyiHvTBP5iO4Xk091ruLUBDMt-w@mail.gmail.com>
Date:   Fri, 2 Apr 2021 09:19:36 +0300
From:   Amir Goldstein <amir73il@...il.com>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     Linux MM <linux-mm@...ck.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Christoph Hellwig <hch@...radead.org>
Subject: Re: [RFC] Convert sysv filesystem to use folios exclusively

On Thu, Mar 25, 2021 at 5:43 AM Matthew Wilcox <willy@...radead.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.