[Top] [All Lists]

Re: [patch v4 06/13] [PATCH 06/13] xfs: xfs_sync_data is redundant.

To: Ben Myers <bpm@xxxxxxx>
Subject: Re: [patch v4 06/13] [PATCH 06/13] xfs: xfs_sync_data is redundant.
From: Mark Tinguely <tinguely@xxxxxxx>
Date: Fri, 05 Oct 2012 13:15:53 -0500
Cc: Dave Chinner <david@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx, Dave Chinner <dchinner@xxxxxxxxxx>
In-reply-to: <20121005180446.GN25175@xxxxxxx>
References: <20121005171853.985930109@xxxxxxx> <20121005171946.330155632@xxxxxxx> <506F1F21.6000104@xxxxxxx> <20121005180446.GN25175@xxxxxxx>
User-agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:9.0) Gecko/20120122 Thunderbird/9.0
On 10/05/12 13:04, Ben Myers wrote:
Hey Mark,

On Fri, Oct 05, 2012 at 12:55:45PM -0500, Mark Tinguely wrote:
On 10/05/12 12:18, Ben Myers wrote:
Index: xfs/fs/xfs/xfs_mount.h
--- xfs.orig/fs/xfs/xfs_mount.h
+++ xfs/fs/xfs/xfs_mount.h
@@ -198,7 +198,6 @@ typedef struct xfs_mount {
        struct xfs_mru_cache    *m_filestream;  /* per-mount filestream data */
        struct delayed_work     m_reclaim_work; /* background inode reclaim */
-       struct work_struct      m_flush_work;   /* background inode flush */
        __int64_t               m_update_flags; /* sb flags we need to update
                                                   on the next remount,rw */
        struct shrinker         m_inode_shrink; /* inode reclaim shrinker */
@@ -381,6 +380,27 @@ extern int xfs_dev_is_read_only(struct x

  extern void   xfs_set_low_space_thresholds(struct xfs_mount *);

+ * Flush all dirty data to disk. Must not be called while holding an XFS_ILOCK
+ * or a page lock.
+ *
+ * We have to hold the s_umount lock here, but because this call can nest
+ * inside i_mutex (the parent directory in the create case, held by the VFS),
+ * we have to use down_read_trylock() to avoid potential deadlocks. In
+ * practice, this trylock will succeed on almost every attempt as
+ * unmount/remount type operations are exceedingly rare.
+ */
+static inline void
+xfs_flush_inodes(struct xfs_mount *mp)
+       struct super_block *sb = mp->m_super;
+       if (down_read_trylock(&sb->s_umount)) {
+               sync_inodes_sb(sb);
+               up_read(&sb->s_umount);
+       }

Was this suppose to be in xfs_inode.h? Otherwise....

Christoph suggested that xfs_flush_inodes should take an xfs_mount instead of
an inode.  It is operating on the super_block level to flush multiple inodes
rather than the single inode whose pointer was passed in, so I moved it from
xfs_inode.h to xfs_mount.h.


You are right - that is where it belongs with the inode->mount parameter change. Sorry for the noise.


<Prev in Thread] Current Thread [Next in Thread>