Commit 34061f5c authored by Dave Chinner's avatar Dave Chinner Committed by Ben Myers
Browse files

xfs: xfs_sync_fsdata is redundant 



Why do we need to write the superblock to disk once we've written
all the data?  We don't actually - the reasons for doing this are
lost in the mists of time, and go back to the way Irix used to drive
VFS flushing.

On linux, this code is only called from two contexts: remount and
.sync_fs. In the remount case, the call is followed by a metadata
sync, which unpins and writes the superblock.  In the sync_fs case,
we only need to force the log to disk to ensure that the superblock
is correctly on disk, so we don't actually need to write it. Hence
the functionality is either redundant or superfluous and thus can be
removed.

Seeing as xfs_quiesce_data is essentially now just a log force,
remove it as well and fold the code back into the two callers.
Neither of them need the log covering check, either, as that is
redundant for the remount case, and unnecessary for the .sync_fs
case.

Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
Signed-off-by: default avatarBen Myers <bpm@sgi.com>
parent 5889608d

fs/xfs/xfs_super.c

+6 −13
Original line number Diff line number Diff line
@@ -1057,7 +1057,6 @@ xfs_fs_sync_fs( 
	int			wait)

{

	struct xfs_mount	*mp = XFS_M(sb);

	int			error;



	/*

	 * Doing anything during the async pass would be counterproductive.
@@ -1065,10 +1064,7 @@ xfs_fs_sync_fs( 
	if (!wait)

		return 0;



	error = xfs_quiesce_data(mp);

	if (error)

		return -error;



	xfs_log_force(mp, XFS_LOG_SYNC);

	if (laptop_mode) {

		/*

		 * The disk must be active because we're syncing.
@@ -1238,15 +1234,12 @@ xfs_fs_remount( 
	/* rw -> ro */

	if (!(mp->m_flags & XFS_MOUNT_RDONLY) && (*flags & MS_RDONLY)) {

		/*

		 * After we have synced the data but before we sync the

		 * metadata, we need to free up the reserve block pool so that

		 * the used block count in the superblock on disk is correct at

		 * the end of the remount. Stash the current reserve pool size

		 * so that if we get remounted rw, we can return it to the same

		 * size.

		 * Before we sync the metadata, we need to free up the reserve

		 * block pool so that the used block count in the superblock on

		 * disk is correct at the end of the remount. Stash the current

		 * reserve pool size so that if we get remounted rw, we can

		 * return it to the same size.

		 */



		xfs_quiesce_data(mp);

		xfs_save_resvblks(mp);

		xfs_quiesce_attr(mp);

		mp->m_flags |= XFS_MOUNT_RDONLY;

fs/xfs/xfs_sync.c

+8 −59
Original line number Diff line number Diff line
@@ -214,69 +214,15 @@ xfs_inode_ag_iterator( 
	return XFS_ERROR(last_error);

}



STATIC int

xfs_sync_fsdata(

	struct xfs_mount	*mp)

{

	struct xfs_buf		*bp;

	int			error;



/*

	 * If the buffer is pinned then push on the log so we won't get stuck

	 * waiting in the write for someone, maybe ourselves, to flush the log.

 * Second stage of a quiesce. The data is already synced, now we have to take

 * care of the metadata. New transactions are already blocked, so we need to

 * wait for any remaining transactions to drain out before proceeding.

 *

	 * Even though we just pushed the log above, we did not have the

	 * superblock buffer locked at that point so it can become pinned in

	 * between there and here.

	 */

	bp = xfs_getsb(mp, 0);

	if (xfs_buf_ispinned(bp))

		xfs_log_force(mp, 0);

	error = xfs_bwrite(bp);

	xfs_buf_relse(bp);

	return error;

}



/*

 * When remounting a filesystem read-only or freezing the filesystem, we have

 * two phases to execute. This first phase is syncing the data before we

 * quiesce the filesystem, and the second is flushing all the inodes out after

 * we've waited for all the transactions created by the first phase to

 * complete. The second phase ensures that the inodes are written to their

 * The second phase ensures that the inodes are written to their

 * location on disk rather than just existing in transactions in the log. This

 * means after a quiesce there is no log replay required to write the inodes to

 * disk (this is the main difference between a sync and a quiesce).

 */

/*

 * First stage of freeze - no writers will make progress now we are here,

 * so we flush delwri and delalloc buffers here, then wait for all I/O to

 * complete.  Data is frozen at that point. Metadata is not frozen,

 * transactions can still occur here so don't bother emptying the AIL

 * because it'll just get dirty again.

 */

int

xfs_quiesce_data(

	struct xfs_mount	*mp)

{

	int			error, error2 = 0;



	/* force out the log */

	xfs_log_force(mp, XFS_LOG_SYNC);



	/* write superblock and hoover up shutdown errors */

	error = xfs_sync_fsdata(mp);



	/* mark the log as covered if needed */

	if (xfs_log_need_covered(mp))

		error2 = xfs_fs_log_dummy(mp);



	return error ? error : error2;

}



/*

 * Second stage of a quiesce. The data is already synced, now we have to take

 * care of the metadata. New transactions are already blocked, so we need to

 * wait for any remaining transactions to drain out before proceeding.

 *

 * Note: this stops background sync work - the callers must ensure it is started

 * again when appropriate.
@@ -291,6 +237,9 @@ xfs_quiesce_attr( 
	while (atomic_read(&mp->m_active_trans) > 0)

		delay(100);



	/* force the log to unpin objects from the now complete transactions */

	xfs_log_force(mp, XFS_LOG_SYNC);



	/* reclaim inodes to do any IO before the freeze completes */

	xfs_reclaim_inodes(mp, 0);

	xfs_reclaim_inodes(mp, SYNC_WAIT);

CRA Git | Maintained and supported by SUSTech CRA and CCSE