xfs
[Top] [All Lists]

[PATCH 9/9] xfs: inode unlinked list needs to recalculate the inode CRC

To: xfs@xxxxxxxxxxx
Subject: [PATCH 9/9] xfs: inode unlinked list needs to recalculate the inode CRC
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Mon, 27 May 2013 16:38:27 +1000
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1369636707-15150-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1369636707-15150-1-git-send-email-david@xxxxxxxxxxxxx>
From: Dave Chinner <dchinner@xxxxxxxxxx>

The inode unlinked list manipulations operate directly on the inode
buffer, and so bypass the inode CRC calculation mechanisms. Hence an
inode on the unlinked list has an invalid CRC. Fix this by
recalculating the CRC whenever we modify an unlinked list pointer in
an inode. This is trivial to do, and the new CRC gets logged with
the inode core so on replay of the unlinked list operations this
will be updated as expected and remain consistent.

Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
---
 fs/xfs/xfs_inode.c       |   49 ++++++++++++++++++++++++++++++++++++++--------
 fs/xfs/xfs_log_recover.c |   22 +++++++++++++++++++++
 2 files changed, 63 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index efbe1ac..f4daadf 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1579,6 +1579,23 @@ out_bmap_cancel:
 }
 
 /*
+ * Helper function to calculate range of the inode to log and recalculate the
+ * on-disk inode crc if necessary.
+ */
+static int
+xfs_iunlink_dinode_calc_crc(
+       struct xfs_mount        *mp,
+       struct xfs_dinode       *dip)
+{
+       if (dip->di_version < 3)
+               return sizeof(xfs_agino_t);
+
+       xfs_dinode_calc_crc(mp, dip);
+       return offsetof(struct xfs_dinode, di_changecount) -
+               offsetof(struct xfs_dinode, di_next_unlinked);
+}
+
+/*
  * This is called when the inode's link count goes to 0.
  * We place the on-disk inode on a list in the AGI.  It
  * will be pulled from this list when the inode is freed.
@@ -1623,6 +1640,8 @@ xfs_iunlink(
        ASSERT(be32_to_cpu(agi->agi_unlinked[bucket_index]) != agino);
 
        if (agi->agi_unlinked[bucket_index] != cpu_to_be32(NULLAGINO)) {
+               int     size;
+
                /*
                 * There is already another inode in the bucket we need
                 * to add ourselves to.  Add us at the front of the list.
@@ -1638,10 +1657,14 @@ xfs_iunlink(
                dip->di_next_unlinked = agi->agi_unlinked[bucket_index];
                offset = ip->i_imap.im_boffset +
                        offsetof(xfs_dinode_t, di_next_unlinked);
+
+               /* need to recalc and log the inode CRC if appropriate */
+               size = xfs_iunlink_dinode_calc_crc(mp, dip);
+
                xfs_trans_inode_buf(tp, ibp);
-               xfs_trans_log_buf(tp, ibp, offset,
-                                 (offset + sizeof(xfs_agino_t) - 1));
+               xfs_trans_log_buf(tp, ibp, offset, offset + size - 1);
                xfs_inobp_check(mp, ibp);
+
        }
 
        /*
@@ -1678,6 +1701,7 @@ xfs_iunlink_remove(
        short           bucket_index;
        int             offset, last_offset = 0;
        int             error;
+       int             size;
 
        mp = tp->t_mountp;
        agno = XFS_INO_TO_AGNO(mp, ip->i_ino);
@@ -1723,9 +1747,12 @@ xfs_iunlink_remove(
                        dip->di_next_unlinked = cpu_to_be32(NULLAGINO);
                        offset = ip->i_imap.im_boffset +
                                offsetof(xfs_dinode_t, di_next_unlinked);
+
+                       /* need to recalc and log the inode CRC if appropriate 
*/
+                       size = xfs_iunlink_dinode_calc_crc(mp, dip);
+
                        xfs_trans_inode_buf(tp, ibp);
-                       xfs_trans_log_buf(tp, ibp, offset,
-                                         (offset + sizeof(xfs_agino_t) - 1));
+                       xfs_trans_log_buf(tp, ibp, offset, offset + size - 1);
                        xfs_inobp_check(mp, ibp);
                } else {
                        xfs_trans_brelse(tp, ibp);
@@ -1796,9 +1823,12 @@ xfs_iunlink_remove(
                        dip->di_next_unlinked = cpu_to_be32(NULLAGINO);
                        offset = ip->i_imap.im_boffset +
                                offsetof(xfs_dinode_t, di_next_unlinked);
+
+                       /* need to recalc and log the inode CRC if appropriate 
*/
+                       size = xfs_iunlink_dinode_calc_crc(mp, dip);
+
                        xfs_trans_inode_buf(tp, ibp);
-                       xfs_trans_log_buf(tp, ibp, offset,
-                                         (offset + sizeof(xfs_agino_t) - 1));
+                       xfs_trans_log_buf(tp, ibp, offset, offset + size - 1);
                        xfs_inobp_check(mp, ibp);
                } else {
                        xfs_trans_brelse(tp, ibp);
@@ -1809,9 +1839,12 @@ xfs_iunlink_remove(
                last_dip->di_next_unlinked = cpu_to_be32(next_agino);
                ASSERT(next_agino != 0);
                offset = last_offset + offsetof(xfs_dinode_t, di_next_unlinked);
+
+               /* need to recalc and log the inode CRC if appropriate */
+               size = xfs_iunlink_dinode_calc_crc(mp, dip);
+
                xfs_trans_inode_buf(tp, last_ibp);
-               xfs_trans_log_buf(tp, last_ibp, offset,
-                                 (offset + sizeof(xfs_agino_t) - 1));
+               xfs_trans_log_buf(tp, last_ibp, offset, offset + size - 1);
                xfs_inobp_check(mp, last_ibp);
        }
        return 0;
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index d6204d1..285ad16 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -1861,6 +1861,28 @@ xlog_recover_do_inode_buffer(
                buffer_nextp = (xfs_agino_t *)xfs_buf_offset(bp,
                                              next_unlinked_offset);
                *buffer_nextp = *logged_nextp;
+
+               /*
+                * Check if we have to recover the CRC, too.i The CRC is
+                * adjacent to the unlinked field, so should always be in the
+                * same dirty region.
+                */
+               if (xfs_sb_version_hascrc(&mp->m_sb)) {
+                       int     crc_off;
+                       __le32  *buffer_crcp;
+                       __le32  *logged_crcp;
+
+                       crc_off = next_unlinked_offset +
+                               offsetof(struct xfs_dinode, di_crc) -
+                               offsetof(struct xfs_dinode, di_next_unlinked);
+
+                       buffer_crcp = (__le32 *)xfs_buf_offset(bp, crc_off);
+                       logged_crcp = item->ri_buf[item_index].i_addr +
+                                       crc_off - reg_buf_offset;
+
+                       *buffer_crcp = *logged_crcp;
+               }
+
        }
 
        return 0;
-- 
1.7.10.4

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