xfs
[Top] [All Lists]

[PATCH 1/7] factor out xfs_read_agi helper

To: xfs@xxxxxxxxxxx
Subject: [PATCH 1/7] factor out xfs_read_agi helper
From: Christoph Hellwig <hch@xxxxxx>
Date: Sun, 26 Oct 2008 16:35:15 -0400
Sender: Christoph Hellwig <hch@xxxxxxxxxxxxx>
User-agent: quilt/0.46-1
Add a helper to read the AGI header and perform basic verification.
Based on hunks from a larger patch from Dave Chinner.

(First sent on Juli 23rd)


Signed-off-by: Christoph Hellwig <hch@xxxxxx>

Index: linux-2.6-xfs/fs/xfs/xfs_ag.h
===================================================================
--- linux-2.6-xfs.orig/fs/xfs/xfs_ag.h  2008-07-23 18:06:20.000000000 +0200
+++ linux-2.6-xfs/fs/xfs/xfs_ag.h       2008-07-23 19:47:29.000000000 +0200
@@ -142,6 +142,9 @@ typedef struct xfs_agi {
 #define        XFS_AGI_BLOCK(mp)       XFS_HDR_BLOCK(mp, XFS_AGI_DADDR(mp))
 #define        XFS_BUF_TO_AGI(bp)      ((xfs_agi_t *)XFS_BUF_PTR(bp))
 
+extern int xfs_read_agi(struct xfs_mount *mp, struct xfs_trans *tp,
+                               xfs_agnumber_t agno, struct xfs_buf **bpp);
+
 /*
  * The third a.g. block contains the a.g. freelist, an array
  * of block pointers to blocks owned by the allocation btree code.
Index: linux-2.6-xfs/fs/xfs/xfs_ialloc.c
===================================================================
--- linux-2.6-xfs.orig/fs/xfs/xfs_ialloc.c      2008-07-23 18:06:20.000000000 
+0200
+++ linux-2.6-xfs/fs/xfs/xfs_ialloc.c   2008-07-23 19:47:29.000000000 +0200
@@ -1370,70 +1370,95 @@ xfs_ialloc_log_agi(
        xfs_trans_log_buf(tp, bp, first, last);
 }
 
+#ifdef DEBUG
+STATIC void
+xfs_check_agi_unlinked(
+       struct xfs_agi          *agi)
+{
+       int                     i;
+
+       for (i = 0; i < XFS_AGI_UNLINKED_BUCKETS; i++)
+               ASSERT(agi->agi_unlinked[i]);
+}
+#else
+#define xfs_check_agi_unlinked(agi)
+#endif
+
 /*
  * Read in the allocation group header (inode allocation section)
  */
 int
-xfs_ialloc_read_agi(
-       xfs_mount_t     *mp,            /* file system mount structure */
-       xfs_trans_t     *tp,            /* transaction pointer */
-       xfs_agnumber_t  agno,           /* allocation group number */
-       xfs_buf_t       **bpp)          /* allocation group hdr buf */
+xfs_read_agi(
+       struct xfs_mount        *mp,    /* file system mount structure */
+       struct xfs_trans        *tp,    /* transaction pointer */
+       xfs_agnumber_t          agno,   /* allocation group number */
+       struct xfs_buf          **bpp)  /* allocation group hdr buf */
 {
-       xfs_agi_t       *agi;           /* allocation group header */
-       int             agi_ok;         /* agi is consistent */
-       xfs_buf_t       *bp;            /* allocation group hdr buf */
-       xfs_perag_t     *pag;           /* per allocation group data */
-       int             error;
+       struct xfs_agi          *agi;   /* allocation group header */
+       int                     agi_ok; /* agi is consistent */
+       int                     error;
 
        ASSERT(agno != NULLAGNUMBER);
-       error = xfs_trans_read_buf(
-                       mp, tp, mp->m_ddev_targp,
+
+       error = xfs_trans_read_buf(mp, tp, mp->m_ddev_targp,
                        XFS_AG_DADDR(mp, agno, XFS_AGI_DADDR(mp)),
-                       XFS_FSS_TO_BB(mp, 1), 0, &bp);
+                       XFS_FSS_TO_BB(mp, 1), 0, bpp);
        if (error)
                return error;
-       ASSERT(bp && !XFS_BUF_GETERROR(bp));
+
+       ASSERT(*bpp && !XFS_BUF_GETERROR(*bpp));
+       agi = XFS_BUF_TO_AGI(*bpp);
 
        /*
         * Validate the magic number of the agi block.
         */
-       agi = XFS_BUF_TO_AGI(bp);
-       agi_ok =
-               be32_to_cpu(agi->agi_magicnum) == XFS_AGI_MAGIC &&
-               XFS_AGI_GOOD_VERSION(be32_to_cpu(agi->agi_versionnum));
+       agi_ok = be32_to_cpu(agi->agi_magicnum) == XFS_AGI_MAGIC &&
+               XFS_AGI_GOOD_VERSION(be32_to_cpu(agi->agi_versionnum)) &&
+               be32_to_cpu(agi->agi_seqno) == agno;
        if (unlikely(XFS_TEST_ERROR(!agi_ok, mp, XFS_ERRTAG_IALLOC_READ_AGI,
                        XFS_RANDOM_IALLOC_READ_AGI))) {
-               XFS_CORRUPTION_ERROR("xfs_ialloc_read_agi", XFS_ERRLEVEL_LOW,
+               XFS_CORRUPTION_ERROR("xfs_read_agi", XFS_ERRLEVEL_LOW,
                                     mp, agi);
-               xfs_trans_brelse(tp, bp);
+               xfs_trans_brelse(tp, *bpp);
                return XFS_ERROR(EFSCORRUPTED);
        }
+
+       XFS_BUF_SET_VTYPE_REF(*bpp, B_FS_AGI, XFS_AGI_REF);
+
+       xfs_check_agi_unlinked(agi);
+       return 0;
+}
+
+int
+xfs_ialloc_read_agi(
+       struct xfs_mount        *mp,    /* file system mount structure */
+       struct xfs_trans        *tp,    /* transaction pointer */
+       xfs_agnumber_t          agno,   /* allocation group number */
+       struct xfs_buf          **bpp)  /* allocation group hdr buf */
+{
+       struct xfs_agi          *agi;   /* allocation group header */
+       struct xfs_perag        *pag;   /* per allocation group data */
+       int                     error;
+
+       error = xfs_read_agi(mp, tp, agno, bpp);
+       if (error)
+               return error;
+
+       agi = XFS_BUF_TO_AGI(*bpp);
        pag = &mp->m_perag[agno];
+
        if (!pag->pagi_init) {
                pag->pagi_freecount = be32_to_cpu(agi->agi_freecount);
                pag->pagi_count = be32_to_cpu(agi->agi_count);
                pag->pagi_init = 1;
-       } else {
-               /*
-                * It's possible for these to be out of sync if
-                * we are in the middle of a forced shutdown.
-                */
-               ASSERT(pag->pagi_freecount == be32_to_cpu(agi->agi_freecount) ||
-                       XFS_FORCED_SHUTDOWN(mp));
-       }
-
-#ifdef DEBUG
-       {
-               int     i;
-
-               for (i = 0; i < XFS_AGI_UNLINKED_BUCKETS; i++)
-                       ASSERT(agi->agi_unlinked[i]);
        }
-#endif
 
-       XFS_BUF_SET_VTYPE_REF(bp, B_FS_AGI, XFS_AGI_REF);
-       *bpp = bp;
+       /*
+        * It's possible for these to be out of sync if
+        * we are in the middle of a forced shutdown.
+        */
+       ASSERT(pag->pagi_freecount == be32_to_cpu(agi->agi_freecount) ||
+               XFS_FORCED_SHUTDOWN(mp));
        return 0;
 }
 
Index: linux-2.6-xfs/fs/xfs/xfs_inode.c
===================================================================
--- linux-2.6-xfs.orig/fs/xfs/xfs_inode.c       2008-07-23 19:45:01.000000000 
+0200
+++ linux-2.6-xfs/fs/xfs/xfs_inode.c    2008-07-23 19:47:29.000000000 +0200
@@ -1776,13 +1776,10 @@ xfs_iunlink(
        xfs_dinode_t    *dip;
        xfs_buf_t       *agibp;
        xfs_buf_t       *ibp;
-       xfs_agnumber_t  agno;
-       xfs_daddr_t     agdaddr;
        xfs_agino_t     agino;
        short           bucket_index;
        int             offset;
        int             error;
-       int             agi_ok;
 
        ASSERT(ip->i_d.di_nlink == 0);
        ASSERT(ip->i_d.di_mode != 0);
@@ -1790,31 +1787,15 @@ xfs_iunlink(
 
        mp = tp->t_mountp;
 
-       agno = XFS_INO_TO_AGNO(mp, ip->i_ino);
-       agdaddr = XFS_AG_DADDR(mp, agno, XFS_AGI_DADDR(mp));
-
        /*
         * Get the agi buffer first.  It ensures lock ordering
         * on the list.
         */
-       error = xfs_trans_read_buf(mp, tp, mp->m_ddev_targp, agdaddr,
-                                  XFS_FSS_TO_BB(mp, 1), 0, &agibp);
+       error = xfs_read_agi(mp, tp, XFS_INO_TO_AGNO(mp, ip->i_ino), &agibp);
        if (error)
                return error;
-
-       /*
-        * Validate the magic number of the agi block.
-        */
        agi = XFS_BUF_TO_AGI(agibp);
-       agi_ok =
-               be32_to_cpu(agi->agi_magicnum) == XFS_AGI_MAGIC &&
-               XFS_AGI_GOOD_VERSION(be32_to_cpu(agi->agi_versionnum));
-       if (unlikely(XFS_TEST_ERROR(!agi_ok, mp, XFS_ERRTAG_IUNLINK,
-                       XFS_RANDOM_IUNLINK))) {
-               XFS_CORRUPTION_ERROR("xfs_iunlink", XFS_ERRLEVEL_LOW, mp, agi);
-               xfs_trans_brelse(tp, agibp);
-               return XFS_ERROR(EFSCORRUPTED);
-       }
+
        /*
         * Get the index into the agi hash table for the
         * list this inode will go on.
@@ -1874,7 +1855,6 @@ xfs_iunlink_remove(
        xfs_buf_t       *agibp;
        xfs_buf_t       *ibp;
        xfs_agnumber_t  agno;
-       xfs_daddr_t     agdaddr;
        xfs_agino_t     agino;
        xfs_agino_t     next_agino;
        xfs_buf_t       *last_ibp;
@@ -1882,45 +1862,23 @@ xfs_iunlink_remove(
        short           bucket_index;
        int             offset, last_offset = 0;
        int             error;
-       int             agi_ok;
 
-       /*
-        * First pull the on-disk inode from the AGI unlinked list.
-        */
        mp = tp->t_mountp;
-
        agno = XFS_INO_TO_AGNO(mp, ip->i_ino);
-       agdaddr = XFS_AG_DADDR(mp, agno, XFS_AGI_DADDR(mp));
 
        /*
         * Get the agi buffer first.  It ensures lock ordering
         * on the list.
         */
-       error = xfs_trans_read_buf(mp, tp, mp->m_ddev_targp, agdaddr,
-                                  XFS_FSS_TO_BB(mp, 1), 0, &agibp);
+       error = xfs_read_agi(mp, tp, agno, &agibp);
        if (error) {
                cmn_err(CE_WARN,
-                       "xfs_iunlink_remove: xfs_trans_read_buf()  returned an 
error %d on %s.  Returning error.",
+                       "xfs_iunlink_remove: xfs_read_agi() returned an error 
%d on %s. Returning error.",
                        error, mp->m_fsname);
                return error;
        }
-       /*
-        * Validate the magic number of the agi block.
-        */
        agi = XFS_BUF_TO_AGI(agibp);
-       agi_ok =
-               be32_to_cpu(agi->agi_magicnum) == XFS_AGI_MAGIC &&
-               XFS_AGI_GOOD_VERSION(be32_to_cpu(agi->agi_versionnum));
-       if (unlikely(XFS_TEST_ERROR(!agi_ok, mp, XFS_ERRTAG_IUNLINK_REMOVE,
-                       XFS_RANDOM_IUNLINK_REMOVE))) {
-               XFS_CORRUPTION_ERROR("xfs_iunlink_remove", XFS_ERRLEVEL_LOW,
-                                    mp, agi);
-               xfs_trans_brelse(tp, agibp);
-               cmn_err(CE_WARN,
-                       "xfs_iunlink_remove: XFS_TEST_ERROR()  returned an 
error on %s.  Returning EFSCORRUPTED.",
-                        mp->m_fsname);
-               return XFS_ERROR(EFSCORRUPTED);
-       }
+
        /*
         * Get the index into the agi hash table for the
         * list this inode will go on.
Index: linux-2.6-xfs/fs/xfs/xfs_log_recover.c
===================================================================
--- linux-2.6-xfs.orig/fs/xfs/xfs_log_recover.c 2008-07-23 18:06:20.000000000 
+0200
+++ linux-2.6-xfs/fs/xfs/xfs_log_recover.c      2008-07-23 19:47:29.000000000 
+0200
@@ -3134,19 +3134,16 @@ xlog_recover_clear_agi_bucket(
        int             error;
 
        tp = xfs_trans_alloc(mp, XFS_TRANS_CLEAR_AGI_BUCKET);
-       error = xfs_trans_reserve(tp, 0, XFS_CLEAR_AGI_BUCKET_LOG_RES(mp), 0, 
0, 0);
-       if (!error)
-               error = xfs_trans_read_buf(mp, tp, mp->m_ddev_targp,
-                                  XFS_AG_DADDR(mp, agno, XFS_AGI_DADDR(mp)),
-                                  XFS_FSS_TO_BB(mp, 1), 0, &agibp);
+       error = xfs_trans_reserve(tp, 0, XFS_CLEAR_AGI_BUCKET_LOG_RES(mp),
+                                 0, 0, 0);
        if (error)
                goto out_abort;
 
-       error = EINVAL;
-       agi = XFS_BUF_TO_AGI(agibp);
-       if (be32_to_cpu(agi->agi_magicnum) != XFS_AGI_MAGIC)
+       error = xfs_read_agi(mp, tp, agno, &agibp);
+       if (error)
                goto out_abort;
 
+       agi = XFS_BUF_TO_AGI(agibp);
        agi->agi_unlinked[bucket] = cpu_to_be32(NULLAGINO);
        offset = offsetof(xfs_agi_t, agi_unlinked) +
                 (sizeof(xfs_agino_t) * bucket);
@@ -3207,16 +3204,21 @@ xlog_recover_process_iunlinks(
                /*
                 * Find the agi for this ag.
                 */
-               agibp = xfs_buf_read(mp->m_ddev_targp,
-                               XFS_AG_DADDR(mp, agno, XFS_AGI_DADDR(mp)),
-                               XFS_FSS_TO_BB(mp, 1), 0);
-               if (XFS_BUF_ISERROR(agibp)) {
-                       xfs_ioerror_alert("xlog_recover_process_iunlinks(#1)",
-                               log->l_mp, agibp,
-                               XFS_AG_DADDR(mp, agno, XFS_AGI_DADDR(mp)));
+               error = xfs_read_agi(mp, NULL, agno, &agibp);
+               if (error) {
+                       /*
+                        * AGI is b0rked. Don't process it.
+                        *
+                        * We should probably mark the filesystem as corrupt
+                        * after we've recovered all the ag's we can....
+                        */
+                       xfs_fs_cmn_err(CE_ALERT, mp,
+                                       "xlog_recover_process_iunlinks(#1)"
+                                       "agi read failed agno %d error %d",
+                                       agno, error);
+                       continue;
                }
                agi = XFS_BUF_TO_AGI(agibp);
-               ASSERT(XFS_AGI_MAGIC == be32_to_cpu(agi->agi_magicnum));
 
                for (bucket = 0; bucket < XFS_AGI_UNLINKED_BUCKETS; bucket++) {
 
@@ -3295,22 +3297,18 @@ xlog_recover_process_iunlinks(
 
                                /*
                                 * Reacquire the agibuffer and continue around
-                                * the loop.
+                                * the loop. This should never fail as we know
+                                * the buffer was good earlier on.
                                 */
-                               agibp = xfs_buf_read(mp->m_ddev_targp,
-                                               XFS_AG_DADDR(mp, agno,
-                                                       XFS_AGI_DADDR(mp)),
-                                               XFS_FSS_TO_BB(mp, 1), 0);
-                               if (XFS_BUF_ISERROR(agibp)) {
-                                       xfs_ioerror_alert(
-                               "xlog_recover_process_iunlinks(#2)",
-                                               log->l_mp, agibp,
-                                               XFS_AG_DADDR(mp, agno,
-                                                       XFS_AGI_DADDR(mp)));
+                               error = xfs_read_agi(mp, NULL, agno, &agibp);
+                               ASSERT(error == 0);
+                               if (error) {
+                                       xfs_fs_cmn_err(CE_ALERT, mp,
+                                       "xlog_recover_process_iunlinks(#2)"
+                                       "agi read failed agno %d error %d",
+                                                       agno, error);
                                }
                                agi = XFS_BUF_TO_AGI(agibp);
-                               ASSERT(XFS_AGI_MAGIC == be32_to_cpu(
-                                       agi->agi_magicnum));
                        }
                }
 
@@ -4004,7 +4002,6 @@ xlog_recover_check_summary(
        xfs_buf_t       *agfbp;
        xfs_buf_t       *agibp;
        xfs_daddr_t     agfdaddr;
-       xfs_daddr_t     agidaddr;
        xfs_buf_t       *sbbp;
 #ifdef XFS_LOUD_RECOVERY
        xfs_sb_t        *sbp;
@@ -4013,6 +4010,7 @@ xlog_recover_check_summary(
        __uint64_t      freeblks;
        __uint64_t      itotal;
        __uint64_t      ifree;
+       int             error;
 
        mp = log->l_mp;
 
@@ -4036,21 +4034,19 @@ xlog_recover_check_summary(
                            be32_to_cpu(agfp->agf_flcount);
                xfs_buf_relse(agfbp);
 
-               agidaddr = XFS_AG_DADDR(mp, agno, XFS_AGI_DADDR(mp));
-               agibp = xfs_buf_read(mp->m_ddev_targp, agidaddr,
-                               XFS_FSS_TO_BB(mp, 1), 0);
-               if (XFS_BUF_ISERROR(agibp)) {
-                       xfs_ioerror_alert("xlog_recover_check_summary(agi)",
-                                         mp, agibp, agidaddr);
-               }
-               agip = XFS_BUF_TO_AGI(agibp);
-               ASSERT(XFS_AGI_MAGIC == be32_to_cpu(agip->agi_magicnum));
-               ASSERT(XFS_AGI_GOOD_VERSION(be32_to_cpu(agip->agi_versionnum)));
-               ASSERT(be32_to_cpu(agip->agi_seqno) == agno);
+               error = xfs_read_agi(mp, NULL, agno, &agibp);
+               if (error) {
+                       xfs_fs_cmn_err(CE_ALERT, mp,
+                                       "xlog_recover_check_summary(agi)"
+                                       "agi read failed agno %d error %d",
+                                                       agno, error);
+               } else {
+                       agip = XFS_BUF_TO_AGI(agibp);
 
-               itotal += be32_to_cpu(agip->agi_count);
-               ifree += be32_to_cpu(agip->agi_freecount);
-               xfs_buf_relse(agibp);
+                       itotal += be32_to_cpu(agip->agi_count);
+                       ifree += be32_to_cpu(agip->agi_freecount);
+                       xfs_buf_relse(agibp);
+               }
        }
 
        sbbp = xfs_getsb(mp, 0);

-- 

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