Re: [PATCH v2] fs/buffer: serialize set_buffer_uptodate against concurrent clears
Chris S <[email protected]> Fri, 31 Jul 2026 11:12:27 -0400
| Newsgroups | org.kernel.vger.linux-fsdevel,org.kernel.vger.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 >