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

Chris S <[email protected]>
Newsgroups gmane.linux.file-systems,gmane.linux.kernel
Message-ID <CACd_6n0FW=9H9iesZ3p3+=6yXnvvH5Wz3svS1kpa2qiPuKZKvw@mail.gmail.com>
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 <[email protected]> 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)" <[email protected]>
> 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) <[email protected]>
> ---
>  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
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.