Re: [PATCH 11/13] nfsd: Use inode_or_folios_dirty() for dirtiness check in wait_for_concurrent_writes()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] nfsd: data race on static variables in
  wait_for_concurrent_writes() [1]
--

commit e80cc2d41e77d086f3f345c30a86cbe9a96c4044
Author: Jan Kara <jack@suse.cz>

nfsd: Use inode_or_folios_dirty() for dirtiness check in wait_for_concurrent_writes()

This commit replaces the racy I_DIRTY check in wait_for_concurrent_writes()
with inode_or_folios_dirty() so that concurrent writeback clearing
I_DIRTY_PAGES or lazytime timestamp updates do not cause vfs_fsync() to be
skipped.

> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 6905d3285ce10..f271b474e76ce 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1431,7 +1431,7 @@ static int wait_for_concurrent_writes(struct file *file)
>  		dprintk("nfsd: write resume %d\n", task_pid_nr(current));
>  	}
>  
> -	if (inode_state_read_once(inode) & I_DIRTY) {
> +	if (inode_or_folios_dirty(inode)) {
>  		dprintk("nfsd: write sync %d\n", task_pid_nr(current));
>  		err = vfs_fsync(file, 0);
>  	}

[Severity: Medium]
This problem wasn't introduced by this patch, but can concurrent NFSD
threads cause a data race on the static variables last_ino and last_dev in
wait_for_concurrent_writes() [1]?

fs/nfsd/vfs.c:wait_for_concurrent_writes() {
	struct inode *inode = file_inode(file);
	static ino_t last_ino;
	static dev_t last_dev;
	int err = 0;

	if (atomic_read(&inode->i_writecount) > 1
	    || (last_ino == inode->i_ino && last_dev == inode->i_sb->s_dev)) {
		...
		msleep(10);
		...
	}
	...
	last_ino = inode->i_ino;
	last_dev = inode->i_sb->s_dev;
	return err;
}

Because last_ino and last_dev are shared across all NFSD threads and read
and written without synchronization, concurrent write requests can observe
torn accesses or mismatched inode and device pairs, causing spurious
msleep(10) delays or missed write deferrals.

[1] https://sashiko.dev/#/bug/linux-f2ccec51-4c11-45a7-a336-775a80d79f1c

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113083336.15152-1-jack@suse.cz?part=11