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 18:12:26 -0400
From:   Eric Whitney <enwlinux@gmail.com>
To:     Jan Kara <jack@suse.cz>
Cc:     Ted Tso <tytso@mit.edu>, linux-ext4@vger.kernel.org,
        Eric Whitney <enwlinux@gmail.com>,
        Linux kernel regressions list <regressions@lists.linux.dev>
Subject: Re: [PATCH] ext4: Fix fsync for non-directories
Message-ID: <ZG6Lyq4iq/HnBvPu@debian-BULLSEYE-live-builder-AMD64>
References: <20230524104453.8734-1-jack@suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230524104453.8734-1-jack@suse.cz>
Precedence: bulk

* Jan Kara <jack@suse.cz>:
> Commit e360c6ed7274 ("ext4: Drop special handling of journalled data
> from ext4_sync_file()") simplified ext4_sync_file() by dropping special
> handling of journalled data mode as it was not needed anymore. However
> that branch was also used for directories and symlinks and since the
> fastcommit code does not track metadata changes to non-regular files, the
> change has caused e.g. fsync(2) on directories to not commit transaction
> as it should. Fix the problem by adding handling for non-regular files.
> 
> Fixes: e360c6ed7274 ("ext4: Drop special handling of journalled data from ext4_sync_file()")
> Reported-by: Eric Whitney <enwlinux@gmail.com>
> Link: https://lore.kernel.org/all/ZFqO3xVnmhL7zv1x@debian-BULLSEYE-live-builder-AMD64
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/ext4/fsync.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/ext4/fsync.c b/fs/ext4/fsync.c
> index f65fdb27ce14..2a143209aa0c 100644
> --- a/fs/ext4/fsync.c
> +++ b/fs/ext4/fsync.c
> @@ -108,6 +108,13 @@ static int ext4_fsync_journal(struct inode *inode, bool datasync,
>  	journal_t *journal = EXT4_SB(inode->i_sb)->s_journal;
>  	tid_t commit_tid = datasync ? ei->i_datasync_tid : ei->i_sync_tid;
>  
> +	/*
> +	 * Fastcommit does not really support fsync on directories or other
> +	 * special files. Force a full commit.
> +	 */
> +	if (!S_ISREG(inode->i_mode))
> +		return ext4_force_commit(inode->i_sb);
> +
>  	if (journal->j_flags & JBD2_BARRIER &&
>  	    !jbd2_trans_will_send_data_barrier(journal, commit_tid))
>  		*needs_barrier = true;
> -- 
> 2.35.3
>

Hi Jan:

100/100 trials of both the original test regressions - generic/065 and
generic/535 - passed when I used kvm-xfstests to run them on a 6.4-rc3 kernel
modified with this patch.  A complete run of the adv test case also passed
without new regressions.

So,
Tested-by: Eric Whitney <enwlinux@gmail.com>

Thanks!
Eric