Re: [PATCH 5/8] reiserfs: Convert do_journal_end() to use kmap_local_folio()

[Matthew Wilcox <willy@infradead.org>]

On Sat, Dec 17, 2022 at 03:52:50PM -0800, Ira Weiny wrote:
> > +			addr = kmap_local_folio(cn->bh->b_folio, offset);
> > +			memcpy(tmp_bh->b_data, addr, cn->bh->b_size);
> > +			kunmap_local(addr);
> 
> I think we should have a memcpy_{to|from}_folio() like we do for the pages.
> 
> Did I miss this in the earlier patch?

I've been going back-and-forth on a memcpy_(to|from)_folio().
I'm generally in favour of higher-level abstractions; I just don't
know that this is the right one.
On the one hand, we have a memcpy_(to|from)_page() already and
the folio version leads to an obvious conversion.

On the other hand, it's massively overkill for all the places I've been
looking at in reiserfs, which generally just want to touch the contents of
a single buffer_head, which is block-sized.  Obviously I have a sampling
bias since I'm looking for bh->b_page usages.  But I was thinking about
a kmap_local_buffer() for these cases.

But that doesn't solve the "What about fs block size > PAGE_SIZE"
problem that we also want to solve.  Here's a concrete example:

 static __u32 jbd2_checksum_data(__u32 crc32_sum, struct buffer_head *bh)
 {
-       struct page *page = bh->b_page;
+       struct folio *folio = bh->b_folio;
        char *addr;
        __u32 checksum;
 
-       addr = kmap_atomic(page);
-       checksum = crc32_be(crc32_sum,
-               (void *)(addr + offset_in_page(bh->b_data)), bh->b_size);
-       kunmap_atomic(addr);
+       BUG_ON(IS_ENABLED(CONFIG_HIGHMEM) && bh->b_size > PAGE_SIZE);
+
+       addr = kmap_local_folio(folio, offset_in_folio(folio, bh->b_data));
+       checksum = crc32_be(crc32_sum, addr, bh->b_size);
+       kunmap_local(addr);
 
        return checksum;
 }

I don't want to add a lot of complexity to handle the case of b_size >
PAGE_SIZE on a HIGHMEM machine since that's not going to benefit terribly
many people.  I'd rather have the assertion that we don't support it.
But if there's a good higher-level abstraction I'm missing here ...