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

folio_vmap() sounds like the right shape, and nothing in this series is
waiting on it, so there is no hurry from my side.

One thing worth settling before anyone writes it, because it decides what
the callers have to look like: kmap_local_folio() documents "Context: Can
be invoked from any context" and has no failure return, while vmap() does
might_sleep() and returns NULL on failure.  So folio_vmap() would inherit
the weaker contract from the one case that needs it, and every caller would
have to be written for may-sleep and may-fail even though on !HIGHMEM
neither can ever happen.  That costs the jbd2 commit path nothing - both
checksum callers run under jbd2_journal_commit_transaction() in kjournald2,
which sleeps freely - but it is not free everywhere, so this may want to be
two entry points rather than one.

Glad to talk it through, and happy to write it if that would help.  I
already owe you a look at the bs>PAGE_SIZE callers once this series
settles, and this is the same errand, so it would be no extra work for me
to start there.  Entirely your call.

Best,
Chao

On Wed, Aug 5, 2026 at 4:41 PM Matthew Wilcox <[email protected]> wrote:
>
> On Wed, Aug 05, 2026 at 03:02:05PM -0400, Chris S wrote:
> > Thank you so much.
> > Taken, with a change: the helper needs a matching unmap, and must not map
> > the folio-less case at all.
> >
> > static inline void *kmap_local_bh(const struct buffer_head *bh)
> > {
> > if (!bh->b_folio)
> > return bh->b_data;
> > return kmap_local_folio(bh->b_folio, bh_offset(bh));
> > }
> >
> > static inline void kunmap_local_bh(const struct buffer_head *bh, void *addr)
> > {
> > if (bh->b_folio)
> > kunmap_local(addr);
> > }
>
> Ah, I hadn't considered that option.  Yes, I think this is the right
> approach.  Thanks!
>
> > Two reasons.  kmap_local_page(virt_to_page(bh->b_data)) loses the offset
> > within the page, and b_frozen_data is kmalloc(b_size), which is only
> > guaranteed page aligned when the block size is at least PAGE_SIZE.
>
> Yeah; my bad.  I should have added in PAGE_MASK(bh->b_data) to that,
> but what you have is better.
>
> > The second one is your other point.  With CONFIG_DEBUG_KMAP_LOCAL_FORCE_MAP,
> > __kmap_local_page_prot() maps even a lowmem page, one page at a time.  So
> > mapping memory that is already permanently mapped would walk this path
> > straight into the bs>PS problem you just described, which it is otherwise
> > immune to - b_data covers the whole block whatever its size.  Returning
> > b_data and calling kunmap_local() on it anyway is not an option either:
> > kunmap_local_indexed() WARNs on an address outside the fixmap range under
> > that config.  Hence the pair.
> >
> > On the bs>PS folio path itself: agreed it is real and predates this series.
> > I would rather not fold a fix into this one, but I will take it - I'll look
> > at it separately once this has settled.
>
> I've been considering adding a folio_vmap() which would be a nop for
> !highmem, kmap_local() for order-0 highmem folios and vmap() for
> large highmem folios.  It's not reached the top of my todo list yet.