Re: [PATCH 2/2] xfs: rework datasync tracking and execution

[Christoph Hellwig <hch@infradead.org>]

On Thu, Sep 18, 2025 at 08:12:54AM +1000, Dave Chinner wrote:
> Jan demostrated that reducing the ifdatasync lock hold time can

s/demostrated/demonstrated/

> came to the conclusion that seperately tracking datasync flush

s/seperately/separately/

> seqeunces was the best approach to solving the problem.
 
s/seqeunces/sequences/g

> operations are allo done under ILOCK_EXCL context:

s/allo/all/

>  xfs_fsync_flush_log(
>  	struct xfs_inode	*ip,
>  	bool			datasync,
>  	int			*log_flushed)
>  {
> +	struct xfs_inode_log_item *iip = ip->i_itemp;
> +	xfs_csn_t		seq = 0;
>  
> +	spin_lock(&iip->ili_lock);
> +	if (datasync)
> +		seq = iip->ili_datasync_seq;
> +	else
> +		seq = iip->ili_commit_seq;
> +	spin_unlock(&iip->ili_lock);

If we care about the additional speedup of the READ_ONCE done
in Jan's patch we could make that configurable on CONFIG_64BIT
here.  There's precedence for that in i_size_read for that in the
VFS.  If we have a helper like:

static inline bool
xfs_inode_sync_csn(
	struct xfs_inode	*ip,
 	bool			datasync,
	xfs_csn_t		*seq)
 {
	struct xfs_inode_log_item *iip = ip->i_itemp;

	if (!iip)
		return false;

	spin_lock(&iip->ili_lock);
	if (datasync)
		*seq = iip->ili_datasync_seq;
	else
		*seq = iip->ili_commit_seq;
	spin_unlock(&iip->ili_lock);
	return true;
}

we could isolate that to one single place as well.

But even without that, the patch looks fine:

Reviewed-by: Christoph Hellwig <hch@lst.de>