Re: [PATCH 03/19] jbd2: point the shadow buffer at the frozen data directly

Matthew Wilcox <[email protected]> Tue, 4 Aug 2026 04:11:59 +0100
Newsgroups dev.linux.lists.ocfs2-devel,dev.linux.lists.gfs2,org.kernel.vger.linux-ext4,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On Mon, Aug 03, 2026 at 07:11:49PM +0100, Matthew Wilcox wrote:
> On Sat, Aug 01, 2026 at 06:00:47PM -0400, Chao Shi wrote:
> > @@ -330,6 +330,8 @@ static __u32 jbd2_checksum_data(__u32 crc32_sum, struct buffer_head *bh)
> >  	char *addr;
> >  	__u32 checksum;
> >  
> > +	if (!bh->b_folio)
> > +		return crc32_be(crc32_sum, bh->b_data, bh->b_size);
> >  	addr = kmap_local_folio(bh->b_folio, bh_offset(bh));
> >  	checksum = crc32_be(crc32_sum, addr, bh->b_size);
> >  	kunmap_local(addr);
> 
> This is awkward.  How about ...

... Oh.  It's not just awkward.  The current code is actually wrong.
If one uses ext4 on a bs>PS device and one has
CONFIG_DEBUG_KMAP_LOCAL_FORCE_MAP enabled, it'll only map one
page of the buffer and we should get a crash when trying to checksum
the entire block.

Supporting mapping multiple pages from a folio simultaneously is
something we haven't figured out how to support.  Since HIGHMEM is
dying (see Arnd's recent work), I'm not inclined to spend effort
on it.

We probably just need to make ext4 depend on !DEBUG_KMAP_LOCAL_FORCE_MAP
or something?

> static inline void *kmap_local_bh(const struct buffer_head *bh)
> {
> 	if (bh->b_folio)
> 		return kmap_local_folio(bh->b_folio, bh_offset(bh));
> 	return kmap_local_page(virt_to_page(bh->b_data);
> }
> 
> (this is also somewhat awkward because it feels like we could just
> return bh->b_data, but kunmap_local_indexed() does some ... stuff)
> 
> Anyway, it all gets optimised away on non-HIGHMEM.  Or if it doesn't,
> you can force it to ;-)
> 
>