Re: [PATCH v2] fs/buffer: serialize set_buffer_uptodate against concurrent clears

[Chris S]

My pleasure. I did a small check.  bh_offset() is indeed the only
b_page user left, and
it applies as-is: my patch guards the four callers that can see a
folio-less bh (__bh_submit(), buffer_set_crypto_ctx(), and the two jbd2
checksum helpers), so bh_offset() itself never sees one and stays the
one-liner you made it.

Built and ran the replay test with both applied - ext4 data=journal,
blocks starting with the JBD2 magic to force copy-out, sysrq-b without
unmounting, then mount to replay.  Recovery completed, md5sums matched,
e2fsck clean, and an instrumented build confirmed the b_folio == NULL path
was taken.

Series head:

  1  buffer_head: Remove b_page                           (yours)
  2  jbd2: point the temp bh at the frozen data directly  (b_folio = NULL)
  3  ... the buffer_write_io_error() conversion Jan asked for

I'll note in 2's changelog that a folio-less bh must not be passed to
bh_offset(), so the constraint is written down rather than implied.

Chao

On Thu, Jul 30, 2026 at 8:45 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> On Thu, Jul 30, 2026 at 06:09:24PM -0400, Chris S wrote:
> > Yes, that works. And appreciate your reply.
> >   I prototyped it, and it survives a journal replay.
> >
> > Six sites, with bio_add_virt_nofail() covering the submit side:
> >
> >   bh_offset()                     b_page may be NULL
> >   buffer_set_crypto_ctx()         NULL guard
> >   __bh_submit()                   bio_add_virt_nofail() when !b_folio, and
> >                                   skip wbc_account_cgroup_owner()
> >   jbd2_journal_write_metadata_buffer()
> >                                   b_folio = NULL, b_data =
> >                                   jh->b_frozen_data, not folio_set_bh()
> >   jbd2_checksum_data(), jbd2_block_tag_csum_set()   both kmap the temp bh
> >                                   (fs/jbd2/commit.c:705 and :745)
> >
> > 4 files, +38/-12.  I'll send it as the first patch of the series.
>
> Fantastic.  Would you care to also include this patch as part of your
> series?
>
> From f7cd3419ce0518ba429ac854282888833bf60147 Mon Sep 17 00:00:00 2001
> From: "Matthew Wilcox (Oracle)" <willy@xxxxxxxxxxxxx>
> Date: Thu, 30 Jul 2026 20:43:22 -0400
> Subject: [PATCH] buffer_head: Remove b_page
>
> All users except bh_offset() have been converted to use b_folio instead.
> Convert bh_offset() and remove b_page.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> ---
>  include/linux/buffer_head.h | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
> index 8b23bc9a244c..3881141a3ff1 100644
> --- a/include/linux/buffer_head.h
> +++ b/include/linux/buffer_head.h
> @@ -59,10 +59,7 @@ struct address_space;
>  struct buffer_head {
>         unsigned long b_state;          /* buffer state bitmap (see above) */
>         struct buffer_head *b_this_page;/* circular list of page's buffers */
> -       union {
> -               struct page *b_page;    /* the page this bh is mapped to */
> -               struct folio *b_folio;  /* the folio this bh is mapped to */
> -       };
> +       struct folio *b_folio;          /* the folio this bh is mapped to */
>
>         sector_t b_blocknr;             /* start block number */
>         size_t b_size;                  /* size of mapping */
> @@ -172,7 +169,7 @@ static __always_inline int buffer_uptodate(const struct buffer_head *bh)
>
>  static inline unsigned long bh_offset(const struct buffer_head *bh)
>  {
> -       return (unsigned long)(bh)->b_data & (page_size(bh->b_page) - 1);
> +       return (unsigned long)(bh)->b_data & (folio_size(bh->b_folio) - 1);
>  }
>
>  /* If we *know* page->private refers to buffer_heads */
> --
> 2.47.3
>