Re: [PATCH 17/19] jbd2: assert on a failed write, not on a buffer that is not up to date

Taken - v2 keeps the assertion on buffer_uptodate() and only changes the
message to "Buffer not uptodate!\n".

You are right that a write error is fine here.  I had converted it on the
grounds that BH_Uptodate stops meaning "the last write worked", but that was
answering the wrong question: what this assert is guarding is that there is
valid data to copy at all, which is still exactly what BH_Uptodate says.

Best,
Chao

On Tue, Aug 4, 2026 at 5:05 AM Jan Kara <[email protected]> wrote:
>
> On Sat 01-08-26 18:01:01, Chao Shi wrote:
> > jbd2_freeze_jh_data() asserts that the buffer it is about to copy out is up
> > to date.  Once this series stops clearing BH_Uptodate on write error, that
> > assertion stops firing for the case it was written for, because a buffer
> > whose write failed stays up to date - which is the point: the in-memory
> > copy being frozen is still the data the filesystem wants written.
> >
> > Assert on the condition that still means something went wrong.  The message
> > loses its "Possible", because BH_Write_EIO is not a guess.
> >
> > Suggested-by: Jan Kara <[email protected]>
> > Signed-off-by: Chao Shi <[email protected]>
> ...
> > diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> > index 5cc7d097b2ac..fdabe34a7e22 100644
> > --- a/fs/jbd2/transaction.c
> > +++ b/fs/jbd2/transaction.c
> > @@ -920,7 +920,7 @@ static void jbd2_freeze_jh_data(struct journal_head *jh)
> >       char *source;
> >       struct buffer_head *bh = jh2bh(jh);
> >
> > -     J_EXPECT_JH(jh, buffer_uptodate(bh), "Possible IO failure.\n");
> > +     J_EXPECT_JH(jh, !buffer_write_io_error(bh), "IO failure.\n");
>
> After some more thought I think getting here a buffer_write_io_error()
> buffer is actually OK and we'll rewrite it. Instead this assert should just
> have a different message:
>
>         J_EXPECT_JH(jh, buffer_uptodate(bh), "Buffer not uptodate!\n");
>
> to verify the buffer has valid data.
>
>                                                                 Honza
>
>
> >       source = kmap_local_folio(bh->b_folio, bh_offset(bh));
> >       /* Fire data frozen trigger just before we copy the data */
> >       jbd2_buffer_frozen_trigger(jh, source, jh->b_triggers);
> > --
> > 2.43.0
> >
> --
> Jan Kara <[email protected]>
> SUSE Labs, CR