Re: [PATCH 0/8] Convert reiserfs from b_page to b_folio

On Sat, Dec 17, 2022 at 09:43:11PM +0100, Fabio M. De Francesco wrote:
> On venerdì 16 dicembre 2022 21:53:39 CET Matthew Wilcox (Oracle) wrote:
> > These patches apply on top of
> > https://lore.kernel.org/linux-fsdevel/20221215214402.3522366-1-willy@infradead
> > .org/
> > 
> > The non-trivial ones mostly revolve around uses of kmap()/kmap_atomic(),
> > so review from the experts on those would be welcome.
> 
> I took a quick look at your conversions and they made me recall that months 
> ago you converted to kmap_local_folio() a previous conversion from kmap() to 
> kmap_local_page() in ext2_get_page(): commit 37ce0b319b287 ("ext2: Use a folio 
> in ext2_get_page()").
> 
> So I just saw kmap_local_folio again. Unfortunately, because of my 
> inexperience,  I'm not able to see why we should prefer the use of this 
> function instead of kmap_local_page().
> 
> Can you please tell me why and when we should prefer kmap_local_folio() in 
> those cases too where kmap_local_page() can work properly? I'm asking because 
> these days I'm converting other *_get_page() from kmap() (including the series 
> to fs/ufs that I sent today).

Fabio kmap_local_folio() works on folios and handles determining which page in
the folio is the correct one to map.

AFAICT (from a quick grep) fs/ufs does not have folio support.  I am sure
Mathew would appreciate converting fs/ufs to folios if you have the time and
want to figure it out.

Ira

> 
> > If these all look
> > good to people, I can pass them off to Andrew for the 6.3 merge window.
> > 
> > Running xfstests against reiserfs gives me 313/701 failures before and
> > after this set of patches.
> 
> It has happened several times to me too. Some patches of mine have failures 
> from xfstests whose amounts and types don't change with or without my changes.
> 
> Several of them have already been merged. I guess that if they don't add 
> further failures everything is alright.
> 
> However, something is broken for sure... xfstests or the filesystems? :-/ 
> 
> Thanks,
> 
> Fabio
> 
> > I don't have a huge amount of confidence
> > that we're really getting good coverage from that test run!
> > 
> > Matthew Wilcox (Oracle) (8):
> >   reiserfs: use b_folio instead of b_page in some obvious cases
> >   reiserfs: use kmap_local_folio() in _get_block_create_0()
> >   reiserfs: Convert direct2indirect() to call folio_zero_range()
> >   reiserfs: Convert reiserfs_delete_item() to use kmap_local_folio()
> >   reiserfs: Convert do_journal_end() to use kmap_local_folio()
> >   reiserfs: Convert map_block_for_writepage() to use kmap_local_folio()
> >   reiserfs: Convert convert_tail_for_hole() to use folios
> >   reiserfs: Use flush_dcache_folio() in reiserfs_quota_write()
> > 
> >  fs/reiserfs/inode.c           | 73 +++++++++++++++++------------------
> >  fs/reiserfs/journal.c         | 12 +++---
> >  fs/reiserfs/prints.c          |  4 +-
> >  fs/reiserfs/stree.c           |  9 +++--
> >  fs/reiserfs/super.c           |  2 +-
> >  fs/reiserfs/tail_conversion.c | 19 ++++-----
> >  6 files changed, 59 insertions(+), 60 deletions(-)
> > 
> > --
> > 2.35.1
> 
> 
> 
>