xfs
[Top] [All Lists]

[PATCH 7/7] XFS: don't use vnodes where unnecessary

To: xfs@xxxxxxxxxxx
Subject: [PATCH 7/7] XFS: don't use vnodes where unnecessary
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 14 Aug 2008 17:14:43 +1000
Cc: Dave Chinner <david@xxxxxxxxxxxxx>
In-reply-to: <1218698083-11226-1-git-send-email-david@fromorbit.com>
References: <1218698083-11226-1-git-send-email-david@fromorbit.com>
Sender: xfs-bounce@xxxxxxxxxxx
Not that we have a combined xfs and linux inode, we can clean
up the use of linux indoes in several places. We can now check
for the xfs inode being in the reclaim state rather than whether
the linux inode exists, and we can use the linux inode directly
rather than via vnode helpers. Also clean up remaining checks
for the vnode being null and replace them with appropriate
flag checks.

Signed-off-by: Dave Chinner <david@xxxxxxxxxxxxx>
---
 fs/xfs/linux-2.6/xfs_sync.c    |   50 ++++++++++++++++++++--------------------
 fs/xfs/linux-2.6/xfs_vnode.c   |    4 +-
 fs/xfs/quota/xfs_qm_syscalls.c |   20 ++++++++++-----
 fs/xfs/xfs_vnodeops.c          |   11 ++------
 4 files changed, 43 insertions(+), 42 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_sync.c b/fs/xfs/linux-2.6/xfs_sync.c
index 53d85ec..1a04769 100644
--- a/fs/xfs/linux-2.6/xfs_sync.c
+++ b/fs/xfs/linux-2.6/xfs_sync.c
@@ -131,10 +131,7 @@ xfs_sync_inodes_ag(
        int             flags,
        int             *bypassed)
 {
-       xfs_inode_t     *ip = NULL;
-       struct inode    *vp = NULL;
        xfs_perag_t     *pag = &mp->m_perag[ag];
-       boolean_t       vnode_refed = B_FALSE;
        int             nr_found;
        int             first_index = 0;
        int             error = 0;
@@ -156,6 +153,10 @@ xfs_sync_inodes_ag(
        }
 
        do {
+               struct inode    *inode;
+               boolean_t       vnode_refed;
+               xfs_inode_t     *ip = NULL;
+
                /*
                 * use a gang lookup to find the next inode in the tree
                 * as the tree is sparse and a gang lookup walks to find
@@ -177,14 +178,14 @@ xfs_sync_inodes_ag(
                 * skip inodes in reclaim. Let xfs_syncsub do that for
                 * us so we don't need to worry.
                 */
-               vp = VFS_I(ip);
-               if (!vp) {
+               if (xfs_iflags_test(ip, (XFS_IRECLAIM|XFS_IRECLAIMABLE))) {
                        read_unlock(&pag->pag_ici_lock);
                        continue;
                }
 
                /* bad inodes are dealt with elsewhere */
-               if (VN_BAD(vp)) {
+               inode = VFS_I(ip);
+               if (VN_BAD(inode)) {
                        read_unlock(&pag->pag_ici_lock);
                        continue;
                }
@@ -196,30 +197,30 @@ xfs_sync_inodes_ag(
                }
 
                /*
-                * The inode lock here actually coordinates with the almost
-                * spurious inode lock in xfs_ireclaim() to prevent the vnode
-                * we handle here without a reference from being freed while we
-                * reference it.  If we lock the inode while it's on the mount
-                * list here, then the spurious inode lock in xfs_ireclaim()
-                * after the inode is pulled from the mount list will sleep
-                * until we release it here.  This keeps the vnode from being
-                * freed while we reference it.
+                * If we can't get a reference on the VFS_I, the inode must be
+                * in reclaim. If we can get the inode lock without blocking,
+                * it is safe to flush the inode because we hold the tree lock
+                * and xfs_iextract will block right now. Hence if we lock the
+                * inode while holding the tree lock, xfs_ireclaim() is
+                * guaranteed to block on the inode lock we now hold and hence
+                * it is safe to reference the inode until we drop the inode
+                * locks completely.
                 */
-               if (xfs_ilock_nowait(ip, lock_flags) == 0) {
-                       vp = vn_grab(vp);
+               vnode_refed = B_FALSE;
+               inode = igrab(inode);
+               if (inode) {
                        read_unlock(&pag->pag_ici_lock);
-                       if (!vp)
-                               continue;
                        xfs_ilock(ip, lock_flags);
-
-                       ASSERT(vp == VFS_I(ip));
-                       ASSERT(ip->i_mount == mp);
-
                        vnode_refed = B_TRUE;
                } else {
-                       /* safe to unlock here as we have a reference */
+                       if (!xfs_ilock_nowait(ip, lock_flags)) {
+                               /* leave it to reclaim */
+                               read_unlock(&pag->pag_ici_lock);
+                               continue;
+                       }
                        read_unlock(&pag->pag_ici_lock);
                }
+
                /*
                 * If we have to flush data or wait for I/O completion
                 * we need to drop the ilock that we currently hold.
@@ -240,7 +241,7 @@ xfs_sync_inodes_ag(
                        xfs_ilock(ip, XFS_ILOCK_SHARED);
                }
 
-               if ((flags & SYNC_DELWRI) && VN_DIRTY(vp)) {
+               if ((flags & SYNC_DELWRI) && VN_DIRTY(VFS_I(ip))) {
                        xfs_iunlock(ip, XFS_ILOCK_SHARED);
                        error = xfs_flush_pages(ip, 0, -1, fflag, FI_NONE);
                        if (flags & SYNC_IOWAIT)
@@ -270,7 +271,6 @@ xfs_sync_inodes_ag(
 
                if (vnode_refed) {
                        IRELE(ip);
-                       vnode_refed = B_FALSE;
                }
 
                if (error)
diff --git a/fs/xfs/linux-2.6/xfs_vnode.c b/fs/xfs/linux-2.6/xfs_vnode.c
index 5cad327..34030ec 100644
--- a/fs/xfs/linux-2.6/xfs_vnode.c
+++ b/fs/xfs/linux-2.6/xfs_vnode.c
@@ -92,8 +92,8 @@ static inline int xfs_icount(struct xfs_inode *ip)
 {
        struct inode *vp = VFS_I(ip);
 
-       if (vp)
-               return vn_count(vp);
+       if (!(inode->i_state & I_CLEAR))
+               return atomic_read(&inode->i_count);
        return -1;
 }
 
diff --git a/fs/xfs/quota/xfs_qm_syscalls.c b/fs/xfs/quota/xfs_qm_syscalls.c
index d292e55..bbd7419 100644
--- a/fs/xfs/quota/xfs_qm_syscalls.c
+++ b/fs/xfs/quota/xfs_qm_syscalls.c
@@ -1037,7 +1037,7 @@ xfs_qm_dqrele_inodes_ag(
        int             nr_found;
 
        do {
-               boolean_t       vnode_refd = B_FALSE;
+               boolean_t       vnode_refd;
 
                /*
                 * use a gang lookup to find the next inode in the tree
@@ -1057,23 +1057,29 @@ xfs_qm_dqrele_inodes_ag(
                first_index = XFS_INO_TO_AGINO(mp, ip->i_ino + 1);
 
                /* Root inode, rbmip and rsumip have associated blocks */
-               vp = VFS_I(ip);
                if (!vp || ip == XFS_QI_UQIP(mp) || ip == XFS_QI_GQIP(mp)) {
                        ASSERT(ip->i_udquot == NULL);
                        ASSERT(ip->i_gdquot == NULL);
                        read_unlock(&pag->pag_ici_lock);
                        continue;
                }
-               if (xfs_ilock_nowait(ip, XFS_ILOCK_EXCL) == 0) {
-                       vp = vn_grab(vp);
+
+               /* see xfs_sync_inodes_ag for description of locking */
+               vnode_refd = B_FALSE;
+               vp = vn_grab(VFS_I(ip));
+               if (vp) {
                        read_unlock(&pag->pag_ici_lock);
-                       if (!vp)
-                               continue;
-                       vnode_refd = B_TRUE;
                        xfs_ilock(ip, XFS_ILOCK_EXCL);
+                       vnode_refd = B_TRUE;
                } else {
+                       if (!xfs_ilock_nowait(ip, XFS_ILOCK_EXCL)) {
+                               /* leave it to reclaim */
+                               read_unlock(&pag->pag_ici_lock);
+                               continue;
+                       }
                        read_unlock(&pag->pag_ici_lock);
                }
+
                if ((flags & XFS_UQUOTA_ACCT) && ip->i_udquot) {
                        xfs_qm_dqrele(ip->i_udquot);
                        ip->i_udquot = NULL;
diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
index d36e5bc..ebb6eda 100644
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -2801,6 +2801,7 @@ xfs_reclaim(
        if (!ip->i_update_core && (ip->i_itemp == NULL)) {
                xfs_ilock(ip, XFS_ILOCK_EXCL);
                xfs_iflock(ip);
+               xfs_iflags_set(ip, XFS_IRECLAIMABLE);
                return xfs_finish_reclaim(ip, 1, XFS_IFLUSH_DELWRI_ELSE_SYNC);
        } else {
                xfs_mount_t     *mp = ip->i_mount;
@@ -2823,10 +2824,6 @@ xfs_finish_reclaim(
        int             sync_mode)
 {
        xfs_perag_t     *pag = xfs_get_perag(ip->i_mount, ip->i_ino);
-       struct inode    *vp = VFS_I(ip);
-
-       if (vp && VN_BAD(vp))
-               goto reclaim;
 
        /* The hash lock here protects a thread in xfs_iget_core from
         * racing with us on linking the inode back with a vnode.
@@ -2836,7 +2833,7 @@ xfs_finish_reclaim(
        write_lock(&pag->pag_ici_lock);
        spin_lock(&ip->i_flags_lock);
        if (__xfs_iflags_test(ip, XFS_IRECLAIM) ||
-           (!__xfs_iflags_test(ip, XFS_IRECLAIMABLE) && vp == NULL)) {
+           !__xfs_iflags_test(ip, XFS_IRECLAIMABLE)) {
                spin_unlock(&ip->i_flags_lock);
                write_unlock(&pag->pag_ici_lock);
                if (locked) {
@@ -2870,15 +2867,13 @@ xfs_finish_reclaim(
         * In the case of a forced shutdown we rely on xfs_iflush() to
         * wait for the inode to be unpinned before returning an error.
         */
-       if (xfs_iflush(ip, sync_mode) == 0) {
+       if (!VN_BAD(VFS_I(ip)) && xfs_iflush(ip, sync_mode) == 0) {
                /* synchronize with xfs_iflush_done */
                xfs_iflock(ip);
                xfs_ifunlock(ip);
        }
 
        xfs_iunlock(ip, XFS_ILOCK_EXCL);
-
- reclaim:
        xfs_ireclaim(ip);
        return 0;
 }
-- 
1.5.6


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