All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
Cc: Ira Weiny <ira.weiny@intel.com>,
	reiserfs-devel@vger.kernel.org, Jan Kara <jack@suse.cz>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 0/8] Convert reiserfs from b_page to b_folio
Date: Sun, 18 Dec 2022 17:59:37 +0000	[thread overview]
Message-ID: <Y59VCfAXmg1jow2o@casper.infradead.org> (raw)
In-Reply-To: <3515948.LM0AJKV5NW@suse>

On Sun, Dec 18, 2022 at 09:09:56AM +0100, Fabio M. De Francesco wrote:
> It all started when months ago I saw a patch from Matthew about the conversion 
> from kmap_local_page() to kmap_local_folio() in ext2_get_page().
> 
> Here I wanted to comment on the xfstests failures but, when I read patch 2/8 
> of this series and saw kmap() converted to kmap_local_folio(), I thought to 
> also use this opportunity to ask about why and when kmap_local_folio() should 
> be preferred over kmap_local_page().
> 
> Obviously, I have nothing against these conversions. I would only like to 
> understand what are the reasons behind the preference for the folio function.

I should probably update this, but here's a good start on folio vs page:
https://lore.kernel.org/linux-fsdevel/YvV1KTyzZ+Jrtj9x@casper.infradead.org/

> Mine is a general question about design, necessity, opportunity: what were the 
> reasons why, in the above-mentioned cases, the use of kmap_local_folio() has 
> been preferred over kmap_local_page()? 
> 
> I saw that this series is about converting from b_page to b_folio, therefore 
> kmap_local_folio() is the obvious choice here.
> 
> But my mind went back again to ext2_get_page :-)
> 
> It looks to me that ext2_get_page() was working properly with 
> kmap_local_page() (since you made the conversion from kmap()). Therefore I 
> could not understand why it is preferred to call read_mapping_folio() to get a 
> folio and then map a page of that folio with kmap_local_folio(). 
> 
> I used to think that read_mapping_page() + kmap_local_page() was all we 
> needed. ATM I have not enough knowledge of VFS/filesystems to understand on my 
> own what we gain from the other way to local map pages.    

read_mapping_page() is scheduled for removal.  All callers need to be
converted to read_mapping_folio() (or another variant if preferable).
I don't mind following along behind your conversions to kmap_local_page()
and converting them to kmap_local_folio(), but if I can go straight from
kmap() / kmap_atomic() to kmap_local_folio(), then I'll do that.

Eventually, kmap_local_page() will _probably_ disappear.  ext2_get_page()
is really only partially converted, and you can see that by the way it
calls ext2_check_page() instead of ext2_check_folio().  I have a design
in place for ext2_check_folio() that handles mapping large folios,
but it isn't on the top of my pile right now.

> I'd really like to work on converting fs/ufs to folios but you know that I'll 
> have enough time to work on other projects only starting by the end of 
> January. 
> 
> AFAIK this task has mainly got to do with the conversions of the address space 
> operations (correct?). I know too little to be able to estimate how much time 
> it takes but I'm pretty sure it needs more than I currently can set aside.
> 
> Instead I could easily devolve the time it is needed for making the  
> memcpy_{to|from}_folio() helpers you talked about in a patch of this series, 
> unless you or Matthew prefer to do yourselves. Please let me know.

I've been wondering about a memcpy_(to|from)_folio() helper too, but I
haven't read Ira's message about it yet.  I'll comment over there.

      reply	other threads:[~2022-12-18 17:59 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-16 20:53 [PATCH 0/8] Convert reiserfs from b_page to b_folio Matthew Wilcox (Oracle)
2022-12-16 20:53 ` [PATCH 1/8] reiserfs: use b_folio instead of b_page in some obvious cases Matthew Wilcox (Oracle)
2022-12-16 20:53 ` [PATCH 2/8] reiserfs: use kmap_local_folio() in _get_block_create_0() Matthew Wilcox (Oracle)
2022-12-17 17:14   ` Ira Weiny
2022-12-17 17:14     ` Ira Weiny
2022-12-17 19:07     ` Matthew Wilcox
2022-12-17 23:33       ` Ira Weiny
2022-12-17 23:33         ` Ira Weiny
2022-12-19 10:42       ` Jan Kara
2022-12-16 20:53 ` [PATCH 3/8] reiserfs: Convert direct2indirect() to call folio_zero_range() Matthew Wilcox (Oracle)
2022-12-17 21:08   ` Ira Weiny
2022-12-17 21:08     ` Ira Weiny
2022-12-16 20:53 ` [PATCH 4/8] reiserfs: Convert reiserfs_delete_item() to use kmap_local_folio() Matthew Wilcox (Oracle)
2022-12-17 23:44   ` Ira Weiny
2022-12-17 23:44     ` Ira Weiny
2022-12-16 20:53 ` [PATCH 5/8] reiserfs: Convert do_journal_end() " Matthew Wilcox (Oracle)
2022-12-17 23:52   ` Ira Weiny
2022-12-17 23:52     ` Ira Weiny
2022-12-20  9:35     ` Matthew Wilcox
2022-12-20 11:18       ` Jan Kara
2022-12-20 16:58         ` Ira Weiny
2022-12-20 16:58           ` Ira Weiny
2022-12-20 18:34           ` Matthew Wilcox
2022-12-20 23:59             ` Ira Weiny
2022-12-20 23:59               ` Ira Weiny
2022-12-21 19:04               ` Matthew Wilcox
2022-12-22 10:37                 ` Jan Kara
2022-12-16 20:53 ` [PATCH 6/8] reiserfs: Convert map_block_for_writepage() " Matthew Wilcox (Oracle)
2022-12-18  0:02   ` Ira Weiny
2022-12-18  0:02     ` Ira Weiny
2022-12-16 20:53 ` [PATCH 7/8] reiserfs: Convert convert_tail_for_hole() to use folios Matthew Wilcox (Oracle)
2022-12-16 20:53 ` [PATCH 8/8] reiserfs: Use flush_dcache_folio() in reiserfs_quota_write() Matthew Wilcox (Oracle)
2022-12-17 20:43 ` [PATCH 0/8] Convert reiserfs from b_page to b_folio Fabio M. De Francesco
2022-12-17 23:39   ` Ira Weiny
2022-12-17 23:39     ` Ira Weiny
2022-12-18  8:09     ` Fabio M. De Francesco
2022-12-18 17:59       ` Matthew Wilcox [this message]

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=Y59VCfAXmg1jow2o@casper.infradead.org \
    --to=willy@infradead.org \
    --cc=fmdefrancesco@gmail.com \
    --cc=ira.weiny@intel.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=reiserfs-devel@vger.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.