xfs: split xfs_sync_inodes

xfs_sync_inodes is used to write back either file data or inode metadata.
In general we always do these separately, except for one fishy case in
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 difference 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@lst.de>
Reviewed-by: Eric Sandeen <sandeen@sandeen.net>

diff --git a/fs/xfs/linux-2.6/xfs_quotaops.c b/fs/xfs/linux-2.6/xfs_quotaops.c
index 94d9a633d3d91bd6df491d67c425ebf8acf80f6a..cb6e2cca214f4b82528571c11e7dcd3138c7bf37 100644 (file)
--- a/fs/xfs/linux-2.6/xfs_quotaops.c
+++ b/fs/xfs/linux-2.6/xfs_quotaops.c
@@ -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

diff --git a/fs/xfs/linux-2.6/xfs_super.c b/fs/xfs/linux-2.6/xfs_super.c
index 0d9b64b219e0b7c4196b08845d7074771134407f..d4e7ef8f8df96b1ad4b082658228c55a89eb98a5 100644 (file)
--- a/fs/xfs/linux-2.6/xfs_super.c
+++ b/fs/xfs/linux-2.6/xfs_super.c
@@ -1071,7 +1071,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 not 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) {

diff --git a/fs/xfs/linux-2.6/xfs_sync.c b/fs/xfs/linux-2.6/xfs_sync.c
index a3d2e77130687baa6b7605b07652d5a9a7403ffe..c1a9a11350730cfd09f9406d2b246834923a9a36 100644 (file)
--- a/fs/xfs/linux-2.6/xfs_sync.c
+++ b/fs/xfs/linux-2.6/xfs_sync.c
@@ -268,29 +268,42 @@ 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,
+                                     XFS_ICI_NO_TAG);
+       if (error)
+               return XFS_ERROR(error);
 
-       if (flags & SYNC_DELWRI)
-               error = xfs_inode_ag_iterator(mp, xfs_sync_inode_data, flags, XFS_ICI_NO_TAG);
+       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, XFS_ICI_NO_TAG);
+/*
+ * 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,
+                                    XFS_ICI_NO_TAG);
 }
 
 STATIC int
@@ -404,12 +417,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 */
@@ -438,7 +451,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);
@@ -521,8 +534,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);
 }
 

diff --git a/fs/xfs/linux-2.6/xfs_sync.h b/fs/xfs/linux-2.6/xfs_sync.h
index 9bb1253a402304bcb11b26305d0b592158a775b1..26bfb5c42e761bbf64594d28d029249e1112c3d1 100644 (file)
--- a/fs/xfs/linux-2.6/xfs_sync.h
+++ b/fs/xfs/linux-2.6/xfs_sync.h
@@ -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 */
@@ -39,7 +37,8 @@ typedef struct xfs_sync_work {
 int xfs_syncd_init(struct xfs_mount *mp);
 void xfs_syncd_stop(struct xfs_mount *mp);
 
-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);

diff --git a/fs/xfs/xfs_filestream.c b/fs/xfs/xfs_filestream.c
index 6c87c8f304efb8f7d887c7c41a667e4c85105882..edf8bdf4141fdbd626acf69dc8db4b2d471c568d 100644 (file)
--- a/fs/xfs/xfs_filestream.c
+++ b/fs/xfs/xfs_filestream.c
@@ -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
         */