Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 24 May 2023 12:43:40 +0200
From:   Jan Kara <jack@suse.cz>
To:     Eric Whitney <enwlinux@gmail.com>
Cc:     jack@suse.cz, linux-ext4@vger.kernel.org
Subject: Re: 6.4-rc1 xfstests-bld adv regressions
Message-ID: <20230524104340.5ypioctla3s676gm@quack3>
References: <ZFqO3xVnmhL7zv1x@debian-BULLSEYE-live-builder-AMD64>
 <20230509190930.wyblxwohejmd43fw@quack3>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230509190930.wyblxwohejmd43fw@quack3>
Precedence: bulk

Hello!

Due to conferences this took a bit long. I'm sorry for that.

On Tue 09-05-23 21:09:30, Jan Kara wrote:
> On Tue 09-05-23 14:20:15, Eric Whitney wrote:
> > I'm seeing two test regressions on 6.4-rc1 while running the adv test case
> > with kvm-xfstests.  Both tests fail with 100% reliability in 100 trial runs,
> > and the failures appear to depend solely upon the fast commit mount option.
> > 
> > The first is generic/065, where the relevant info from 065.full is:
> > 
> > _check_generic_filesystem: filesystem on /dev/vdc is inconsistent
> > *** fsck.ext4 output ***
> > fsck from util-linux 2.36.1
> > e2fsck 1.47.0 (5-Feb-2023)
> > Pass 1: Checking inodes, blocks, and sizes
> > Pass 2: Checking directory structure
> > Pass 3: Checking directory connectivity
> > Pass 4: Checking reference counts
> > Pass 5: Checking group summary information
> > Directories count wrong for group #16 (4294967293, counted=0).
> > 
> > 
> > The second is generic/535, where the test output is:
> > 
> >      QA output created by 535
> >      Silence is golden
> >     +Before: 755
> >     +After : 777
> > 
> > Both test failures bisect to:  e360c6ed7274 ("ext4: Drop special handling of
> > journalled data from ext4_sync_file()").  Reverting this patch eliminates the
> > test failures.  So, I thought I'd bring these to your attention.
> 
> Thanks for report! Yeah, when doing commit e360c6ed7274 I forgot about
> directories which can be also fsynced and which need special treatment. I
> have to think a bit what's the best way to fix this.

After digging a bit in the code I understand now what has confused me. The
thing is that fastcommit does not track metadata changes on directories but
neither does it mark the filesystem as ineligible when they happen. So
ext4_fc_commit() implicitely relies on the fact that it never gets called
in any other case than fsync(2) on a regular file.

I believe we should improve fastcommit code to better handle directories
or at least not have these implicit assumptions but for now the easiest fix
is to return back the explicit full commit for non-regular files. I'll send
a patch.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR