Re: [RFC v2 02/11] fs/buffer: add a for_each_bh() for block_read_full_folio()

Luis Chamberlain
2024-12-16T21:46:33Z

On Mon, Dec 16, 2024 at 12:05:54PM -0800, Luis Chamberlain wrote:
> On Mon, Dec 16, 2024 at 10:56:44AM -0800, Luis Chamberlain wrote:
> > On Sat, Dec 14, 2024 at 04:02:53AM +0000, Matthew Wilcox wrote:
> > > On Fri, Dec 13, 2024 at 07:10:40PM -0800, Luis Chamberlain wrote:
> > > > - do {
> > > > + for_each_bh(bh, head) {
> > > > if (buffer_uptodate(bh))
> > > > continue;
> > > >
> > > > @@ -2454,7 +2464,9 @@ int block_read_full_folio(struct folio *folio, get_block_t *get_block)
> > > > continue;
> > > > }
> > > > arr[nr++] = bh;
> > > > - } while (i++, iblock++, (bh = bh->b_this_page) != head);
> > > > + i++;
> > > > + iblock++;
> > > > + }
> > >
> > > This is non-equivalent. That 'continue' you can see would increment i
> > > and iblock. Now it doesn't.
> >
> > Thanks, not sure how I missed that! With that fix in place I ran a full
> > baseline against ext4 and all XFS profiles.
> >
> > For ext4 the new failures I see are just:
> >
> > * generic/044
> > * generic/045
> > * generic/046
>
> Oh my, these all fail on vanilla v6.12-rc2 so its not the code which is
> at fault.

I looked inside my bag of "tribal knowedlge" and found that these are
known to fail because by default ext4 uses mount -o data=ordered mode
in favor of performance instead of the mount -o data=journal mode.
And I confirm using mount -o data=journal fixes this for both the
baselines v6.13-rc2 and with these patches. In fstets you do that with:

MOUNT_OPTIONS='-o data=journal'

And so these failure are part of the baseline, and so, so far no
regressions are found with ext4 with this patch series.

Luis