xfs
[Top] [All Lists]

Review: Make xfs_bulkstat() to report unlinked but referenced inodes

To: xfs-dev <xfs-dev@xxxxxxx>
Subject: Review: Make xfs_bulkstat() to report unlinked but referenced inodes
From: Vlad Apostolov <vapo@xxxxxxx>
Date: Wed, 17 Oct 2007 12:16:19 +1000
Cc: linux-xfs@xxxxxxxxxxx
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Thunderbird 2.0.0.6 (X11/20070728)
We need xfs_bulkstat() to report inode stat for inodes with
link count zero but reference count non zero.

The fix here:

http://oss.sgi.com/archives/xfs/2007-09/msg00266.html

changed this behavior and made xfs_bulkstat() to filter all
unlinked inodes including those that are not destroyed yet but
held by reference.

The attached patch returns back to the original behavior by
marking the on-disk inode buffer dirty when di_mode is cleared
(at that time both inode link and reference counter are zero).

Regards,
Vlad
Index: linux-xfs1/fs/xfs/xfs_inode.c
===================================================================
--- linux-xfs1.orig/fs/xfs/xfs_inode.c
+++ linux-xfs1/fs/xfs/xfs_inode.c
@@ -1951,24 +1951,6 @@ xfs_iunlink(
        ASSERT(agi->agi_unlinked[bucket_index]);
        ASSERT(be32_to_cpu(agi->agi_unlinked[bucket_index]) != agino);
 
-       error = xfs_itobp(mp, tp, ip, &dip, &ibp, 0, 0);
-       if (error)
-               return error;
-
-       /*
-        * Clear the on-disk di_nlink. This is to prevent xfs_bulkstat
-        * from picking up this inode when it is reclaimed (its incore state
-        * initialzed but not flushed to disk yet). The in-core di_nlink is
-        * already cleared in xfs_droplink() and a corresponding transaction
-        * logged. The hack here just synchronizes the in-core to on-disk
-        * di_nlink value in advance before the actual inode sync to disk.
-        * This is OK because the inode is already unlinked and would never
-        * change its di_nlink again for this inode generation.
-        * This is a temporary hack that would require a proper fix
-        * in the future.
-        */
-       dip->di_core.di_nlink = 0;
-
        if (be32_to_cpu(agi->agi_unlinked[bucket_index]) != NULLAGINO) {
                /*
                 * There is already another inode in the bucket we need
@@ -1976,6 +1958,10 @@ xfs_iunlink(
                 * Here we put the head pointer into our next pointer,
                 * and then we fall through to point the head at us.
                 */
+               error = xfs_itobp(mp, tp, ip, &dip, &ibp, 0, 0);
+               if (error)
+                       return error;
+
                ASSERT(be32_to_cpu(dip->di_next_unlinked) == NULLAGINO);
                /* both on-disk, don't endian flip twice */
                dip->di_next_unlinked = agi->agi_unlinked[bucket_index];
@@ -2365,6 +2351,8 @@ xfs_ifree(
        int                     error;
        int                     delete;
        xfs_ino_t               first_ino;
+       xfs_dinode_t            *dip;
+       xfs_buf_t               *ibp;
 
        ASSERT(ismrlocked(&ip->i_lock, MR_UPDATE));
        ASSERT(ip->i_transp == tp);
@@ -2400,8 +2388,27 @@ xfs_ifree(
         * by reincarnations of this inode.
         */
        ip->i_d.di_gen++;
+
        xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
 
+       error = xfs_itobp(ip->i_mount, tp, ip, &dip, &ibp, 0, 0);
+       if (error)
+               return error;
+
+        /*
+       * Clear the on-disk di_mode. This is to prevent xfs_bulkstat
+       * from picking up this inode when it is reclaimed (its incore state
+       * initialzed but not flushed to disk yet). The in-core di_mode is
+       * already cleared  and a corresponding transaction logged.
+       * The hack here just synchronizes the in-core to on-disk
+       * di_mode value in advance before the actual inode sync to disk.
+       * This is OK because the inode is already unlinked and would never
+       * change its di_mode again for this inode generation.
+       * This is a temporary hack that would require a proper fix
+       * in the future.
+       */
+       dip->di_core.di_mode = 0;
+
        if (delete) {
                xfs_ifree_cluster(ip, tp, first_ino);
        }
Index: linux-xfs1/fs/xfs/xfs_itable.c
===================================================================
--- linux-xfs1.orig/fs/xfs/xfs_itable.c
+++ linux-xfs1/fs/xfs/xfs_itable.c
@@ -291,7 +291,7 @@ xfs_bulkstat_use_dinode(
        dip = (xfs_dinode_t *)
                        xfs_buf_offset(bp, clustidx << mp->m_sb.sb_inodelog);
        /*
-        * Check the buffer containing the on-disk inode for di_nlink == 0.
+        * Check the buffer containing the on-disk inode for di_mode == 0.
         * This is to prevent xfs_bulkstat from picking up just reclaimed
         * inodes that have their in-core state initialized but not flushed
         * to disk yet. This is a temporary hack that would require a proper
@@ -299,7 +299,7 @@ xfs_bulkstat_use_dinode(
         */
        if (be16_to_cpu(dip->di_core.di_magic) != XFS_DINODE_MAGIC ||
            !XFS_DINODE_GOOD_VERSION(dip->di_core.di_version) ||
-           !dip->di_core.di_nlink)
+           !dip->di_core.di_mode)
                return 0;
        if (flags & BULKSTAT_FG_QUICK) {
                *dipp = dip;
<Prev in Thread] Current Thread [Next in Thread>