[PATCH 7/7] xfs: split xfs_sync_inodes
Eric Sandeen
sandeen at sandeen.net
Fri Jun 5 15:32:25 CDT 2009
Christoph Hellwig wrote:
> xfs_sync_inodes is used to write back either file data or inode metadata.
> In generally we always do these separately, except for one fishy case in
^^^ "In general"
> xfs_fs_put_super that does both. So separate xfs_sync_inodes into
> separate xfs_sync_data and xfs_sync_attr functions. In xfs_fs_put_super
> we first call the data sync and then the attr sync as that was the previous
> order. The moved log force in that path doesn't make a different because
> we will force the log again as part of the real unmount process.
>
> The filesystem readonly checks are not performed by the new function but
> instead moved into the callers, given that most callers alredy have it
> further up in the stack. Also add debug checks that we do not pass in
> incorrect flags in the new xfs_sync_data and xfs_sync_attr function and
> fix the one place that did pass in a wrong flag.
>
> Also remove a comment mentioning xfs_sync_inodes that has been incorrect
> for a while because we always take either the iolock or ilock in the
> sync path these days.
>
>
> Signed-off-by: Christoph Hellwig <hch at lst.de>
Reviewed-by: Eric Sandeen <sandeen at sandeen.net>
> Index: xfs/fs/xfs/linux-2.6/xfs_super.c
> ===================================================================
> --- xfs.orig/fs/xfs/linux-2.6/xfs_super.c 2009-05-14 19:09:00.178792110 +0200
> +++ xfs/fs/xfs/linux-2.6/xfs_super.c 2009-05-14 19:09:05.278808755 +0200
> @@ -1070,7 +1070,18 @@ xfs_fs_put_super(
> int unmount_event_flags = 0;
>
> xfs_syncd_stop(mp);
> - xfs_sync_inodes(mp, SYNC_ATTR|SYNC_DELWRI);
> +
> + if (!(sb->s_flags & MS_RDONLY)) {
> + /*
> + * XXX(hch): this should be SYNC_WAIT.
> + *
> + * Or more likely no needed at all because the VFS is already
> + * calling ->sync_fs after shutting down all filestem
> + * operations and just before calling ->put_super.
> + */
> + xfs_sync_data(mp, 0);
> + xfs_sync_attr(mp, 0);
> + }
>
> #ifdef HAVE_DMAPI
> if (mp->m_flags & XFS_MOUNT_DMAPI) {
> Index: xfs/fs/xfs/linux-2.6/xfs_sync.c
> ===================================================================
> --- xfs.orig/fs/xfs/linux-2.6/xfs_sync.c 2009-05-14 19:09:04.687659175 +0200
> +++ xfs/fs/xfs/linux-2.6/xfs_sync.c 2009-05-14 19:09:05.279808603 +0200
> @@ -265,29 +265,40 @@ xfs_sync_inode_attr(
> return error;
> }
>
> +/*
> + * Write out pagecache data for the whole filesystem.
> + */
> int
> -xfs_sync_inodes(
> - xfs_mount_t *mp,
> - int flags)
> +xfs_sync_data(
> + struct xfs_mount *mp,
> + int flags)
> {
> - int error = 0;
> - int lflags = XFS_LOG_FORCE;
> + int error;
>
> - if (mp->m_flags & XFS_MOUNT_RDONLY)
> - return 0;
> + ASSERT((flags & ~(SYNC_TRYLOCK|SYNC_WAIT|SYNC_IOWAIT)) == 0);
>
> - if (flags & SYNC_WAIT)
> - lflags |= XFS_LOG_SYNC;
> + error = xfs_inode_ag_iterator(mp, xfs_sync_inode_data, flags, -1);
> + if (error)
> + return XFS_ERROR(error);
>
> - if (flags & SYNC_DELWRI)
> - error = xfs_inode_ag_iterator(mp, xfs_sync_inode_data, flags, -1);
> + xfs_log_force(mp, 0,
> + (flags & SYNC_WAIT) ?
> + XFS_LOG_FORCE | XFS_LOG_SYNC :
> + XFS_LOG_FORCE);
> + return 0;
> +}
>
> - if (flags & SYNC_ATTR)
> - error = xfs_inode_ag_iterator(mp, xfs_sync_inode_attr, flags, -1);
> +/*
> + * Write out inode metadata (attributes) for the whole filesystem.
> + */
> +int
> +xfs_sync_attr(
> + struct xfs_mount *mp,
> + int flags)
> +{
> + ASSERT((flags & ~SYNC_WAIT) == 0);
>
> - if (!error && (flags & SYNC_DELWRI))
> - xfs_log_force(mp, 0, lflags);
> - return XFS_ERROR(error);
> + return xfs_inode_ag_iterator(mp, xfs_sync_inode_attr, flags, -1);
> }
>
> STATIC int
> @@ -401,12 +412,12 @@ xfs_quiesce_data(
> int error;
>
> /* push non-blocking */
> - xfs_sync_inodes(mp, SYNC_DELWRI|SYNC_BDFLUSH);
> + xfs_sync_data(mp, 0);
> xfs_qm_sync(mp, SYNC_BDFLUSH);
> xfs_filestream_flush(mp);
>
> /* push and block */
> - xfs_sync_inodes(mp, SYNC_DELWRI|SYNC_WAIT|SYNC_IOWAIT);
> + xfs_sync_data(mp, SYNC_WAIT|SYNC_IOWAIT);
> xfs_qm_sync(mp, SYNC_WAIT);
>
> /* write superblock and hoover up shutdown errors */
> @@ -435,7 +446,7 @@ xfs_quiesce_fs(
> * logged before we can write the unmount record.
> */
> do {
> - xfs_sync_inodes(mp, SYNC_ATTR|SYNC_WAIT);
> + xfs_sync_attr(mp, SYNC_WAIT);
> pincount = xfs_flush_buftarg(mp->m_ddev_targp, 1);
> if (!pincount) {
> delay(50);
> @@ -518,8 +529,8 @@ xfs_flush_inodes_work(
> void *arg)
> {
> struct inode *inode = arg;
> - xfs_sync_inodes(mp, SYNC_DELWRI | SYNC_TRYLOCK);
> - xfs_sync_inodes(mp, SYNC_DELWRI | SYNC_TRYLOCK | SYNC_IOWAIT);
> + xfs_sync_data(mp, SYNC_TRYLOCK);
> + xfs_sync_data(mp, SYNC_TRYLOCK | SYNC_IOWAIT);
> iput(inode);
> }
>
> Index: xfs/fs/xfs/linux-2.6/xfs_quotaops.c
> ===================================================================
> --- xfs.orig/fs/xfs/linux-2.6/xfs_quotaops.c 2009-05-14 19:05:24.908659400 +0200
> +++ xfs/fs/xfs/linux-2.6/xfs_quotaops.c 2009-05-14 19:09:05.280834851 +0200
> @@ -50,9 +50,11 @@ xfs_fs_quota_sync(
> {
> struct xfs_mount *mp = XFS_M(sb);
>
> + if (sb->s_flags & MS_RDONLY)
> + return -EROFS;
> if (!XFS_IS_QUOTA_RUNNING(mp))
> return -ENOSYS;
> - return -xfs_sync_inodes(mp, SYNC_DELWRI);
> + return -xfs_sync_data(mp, 0);
> }
>
> STATIC int
> Index: xfs/fs/xfs/linux-2.6/xfs_sync.h
> ===================================================================
> --- xfs.orig/fs/xfs/linux-2.6/xfs_sync.h 2009-05-14 19:09:04.694659368 +0200
> +++ xfs/fs/xfs/linux-2.6/xfs_sync.h 2009-05-14 19:09:05.280834851 +0200
> @@ -29,8 +29,6 @@ typedef struct xfs_sync_work {
> struct completion *w_completion;
> } xfs_sync_work_t;
>
> -#define SYNC_ATTR 0x0001 /* sync attributes */
> -#define SYNC_DELWRI 0x0002 /* look at delayed writes */
> #define SYNC_WAIT 0x0004 /* wait for i/o to complete */
> #define SYNC_BDFLUSH 0x0008 /* BDFLUSH is calling -- don't block */
> #define SYNC_IOWAIT 0x0010 /* wait for all I/O to complete */
> @@ -41,7 +39,8 @@ void xfs_syncd_stop(struct xfs_mount *mp
>
> int xfs_inode_flush(struct xfs_inode *ip, int sync);
>
> -int xfs_sync_inodes(struct xfs_mount *mp, int flags);
> +int xfs_sync_attr(struct xfs_mount *mp, int flags);
> +int xfs_sync_data(struct xfs_mount *mp, int flags);
> int xfs_sync_fsdata(struct xfs_mount *mp, int flags);
>
> int xfs_quiesce_data(struct xfs_mount *mp);
> Index: xfs/fs/xfs/xfs_filestream.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_filestream.c 2009-05-14 19:05:24.929659282 +0200
> +++ xfs/fs/xfs/xfs_filestream.c 2009-05-14 19:09:05.283807995 +0200
> @@ -542,10 +542,8 @@ xfs_filestream_associate(
> * waiting for the lock because someone else is waiting on the lock we
> * hold and we cannot drop that as we are in a transaction here.
> *
> - * Lucky for us, this inversion is rarely a problem because it's a
> - * directory inode that we are trying to lock here and that means the
> - * only place that matters is xfs_sync_inodes() and SYNC_DELWRI is
> - * used. i.e. freeze, remount-ro, quotasync or unmount.
> + * Lucky for us, this inversion is not a problem because it's a
> + * directory inode that we are trying to lock here.
> *
> * So, if we can't get the iolock without sleeping then just give up
> */
>
> _______________________________________________
> xfs mailing list
> xfs at oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
>
More information about the xfs
mailing list