Commit 5f9b4b0d authored by Dave Chinner's avatar Dave Chinner Committed by Darrick J. Wong
Browse files

xfs: xfs_log_force_lsn isn't passed a LSN 



In doing an investigation into AIL push stalls, I was looking at the
log force code to see if an async CIL push could be done instead.
This lead me to xfs_log_force_lsn() and looking at how it works.

xfs_log_force_lsn() is only called from inode synchronisation
contexts such as fsync(), and it takes the ip->i_itemp->ili_last_lsn
value as the LSN to sync the log to. This gets passed to
xlog_cil_force_lsn() via xfs_log_force_lsn() to flush the CIL to the
journal, and then used by xfs_log_force_lsn() to flush the iclogs to
the journal.

The problem is that ip->i_itemp->ili_last_lsn does not store a
log sequence number. What it stores is passed to it from the
->iop_committing method, which is called by xfs_log_commit_cil().
The value this passes to the iop_committing method is the CIL
context sequence number that the item was committed to.

As it turns out, xlog_cil_force_lsn() converts the sequence to an
actual commit LSN for the related context and returns that to
xfs_log_force_lsn(). xfs_log_force_lsn() overwrites it's "lsn"
variable that contained a sequence with an actual LSN and then uses
that to sync the iclogs.

This caused me some confusion for a while, even though I originally
wrote all this code a decade ago. ->iop_committing is only used by
a couple of log item types, and only inode items use the sequence
number it is passed.

Let's clean up the API, CIL structures and inode log item to call it
a sequence number, and make it clear that the high level code is
using CIL sequence numbers and not on-disk LSNs for integrity
synchronisation purposes.

Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
Reviewed-by: default avatarDarrick J. Wong <djwong@kernel.org>
Reviewed-by: default avatarAllison Henderson <allison.henderson@oracle.com>
Signed-off-by: default avatarDarrick J. Wong <djwong@kernel.org>
parent 19f4e7cc

fs/xfs/libxfs/xfs_types.h

+1 −0
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ typedef int32_t xfs_suminfo_t; /* type of bitmap summary info */ 
typedef uint32_t	xfs_rtword_t;	/* word type for bitmap manipulations */



typedef int64_t		xfs_lsn_t;	/* log sequence number */

typedef int64_t		xfs_csn_t;	/* CIL sequence number */



typedef uint32_t	xfs_dablk_t;	/* dir/attr block number (in file) */

typedef uint32_t	xfs_dahash_t;	/* dir/attr hash value */

fs/xfs/xfs_buf_item.c

+1 −1
Original line number Diff line number Diff line
@@ -713,7 +713,7 @@ xfs_buf_item_release( 
STATIC void

xfs_buf_item_committing(

	struct xfs_log_item	*lip,

	xfs_lsn_t		commit_lsn)

	xfs_csn_t		seq)

{

	return xfs_buf_item_release(lip);

}

fs/xfs/xfs_dquot_item.c

+1 −1
Original line number Diff line number Diff line
@@ -188,7 +188,7 @@ xfs_qm_dquot_logitem_release( 
STATIC void

xfs_qm_dquot_logitem_committing(

	struct xfs_log_item	*lip,

	xfs_lsn_t		commit_lsn)

	xfs_csn_t		seq)

{

	return xfs_qm_dquot_logitem_release(lip);

}

fs/xfs/xfs_file.c

+7 −7
Original line number Diff line number Diff line
@@ -119,8 +119,8 @@ xfs_dir_fsync( 
	return xfs_log_force_inode(ip);

}



static xfs_lsn_t

xfs_fsync_lsn(

static xfs_csn_t

xfs_fsync_seq(

	struct xfs_inode	*ip,

	bool			datasync)

{
@@ -128,7 +128,7 @@ xfs_fsync_lsn( 
		return 0;

	if (datasync && !(ip->i_itemp->ili_fsync_fields & ~XFS_ILOG_TIMESTAMP))

		return 0;

	return ip->i_itemp->ili_last_lsn;

	return ip->i_itemp->ili_commit_seq;

}



/*
@@ -151,12 +151,12 @@ xfs_fsync_flush_log( 
	int			*log_flushed)

{

	int			error = 0;

	xfs_lsn_t		lsn;

	xfs_csn_t		seq;



	xfs_ilock(ip, XFS_ILOCK_SHARED);

	lsn = xfs_fsync_lsn(ip, datasync);

	if (lsn) {

		error = xfs_log_force_lsn(ip->i_mount, lsn, XFS_LOG_SYNC,

	seq = xfs_fsync_seq(ip, datasync);

	if (seq) {

		error = xfs_log_force_seq(ip->i_mount, seq, XFS_LOG_SYNC,

					  log_flushed);



		spin_lock(&ip->i_itemp->ili_lock);

fs/xfs/xfs_inode.c

+5 −5
Original line number Diff line number Diff line
@@ -2633,7 +2633,7 @@ xfs_iunpin( 
	trace_xfs_inode_unpin_nowait(ip, _RET_IP_);



	/* Give the log a push to start the unpinning I/O */

	xfs_log_force_lsn(ip->i_mount, ip->i_itemp->ili_last_lsn, 0, NULL);

	xfs_log_force_seq(ip->i_mount, ip->i_itemp->ili_commit_seq, 0, NULL);



}
@@ -3647,16 +3647,16 @@ int 
xfs_log_force_inode(

	struct xfs_inode	*ip)

{

	xfs_lsn_t		lsn = 0;

	xfs_csn_t		seq = 0;



	xfs_ilock(ip, XFS_ILOCK_SHARED);

	if (xfs_ipincount(ip))

		lsn = ip->i_itemp->ili_last_lsn;

		seq = ip->i_itemp->ili_commit_seq;

	xfs_iunlock(ip, XFS_ILOCK_SHARED);



	if (!lsn)

	if (!seq)

		return 0;

	return xfs_log_force_lsn(ip->i_mount, lsn, XFS_LOG_SYNC, NULL);

	return xfs_log_force_seq(ip->i_mount, seq, XFS_LOG_SYNC, NULL);

}



/*