Hi,

In the past, pass5 has kept track of different counts for the different
metadata types, but the counts did not correspond to the type in the
bitmap. This patch changes pass5 so that it makes a little more sense.
There should be a straightforward blockmap-to-bitmap conversion, and
the counts should be based on the bitmap type. For example, before the
patch, the rgrp's inode count would be kept in count[1], but in the
bitmap, dinodes are of type 3. So this patch reworks that system so
that the count of dinodes is kept in count[3] and so forth, and it
uses a simple blockmap-to-bitmap to figure out the index to "count".
This breaks down for GFS1 which keeps a few extra numbers in the rgrp,
however this patch compensates for that. This patch is being done
as a precursor to another patch that reduces fsck's memory requirements
by using a blockmap that's 1:1 with the bitmaps rather than today's
blockmap that's 4:1.

Regards,

Bob Peterson
Red Hat File Systems

Signed-off-by: Bob Peterson <rpete...@redhat.com> 
---
diff --git a/gfs2/fsck/pass5.c b/gfs2/fsck/pass5.c
index b2e8adf..2b8536d 100644
--- a/gfs2/fsck/pass5.c
+++ b/gfs2/fsck/pass5.c
@@ -12,94 +12,6 @@
 #include "fsck.h"
 #include "util.h"
 
-static int gfs1_convert_mark(uint8_t q, uint32_t *count)
-{
-       switch(q) {
-
-       case gfs2_meta_inval:
-       case gfs2_inode_invalid:
-               /* Convert invalid metadata to free blocks */
-       case gfs2_block_free:
-               count[0]++;
-               return GFS2_BLKST_FREE;
-
-       case gfs2_block_used:
-               count[2]++;
-               return GFS2_BLKST_USED;
-
-       case gfs2_inode_dir:
-       case gfs2_inode_file:
-       case gfs2_inode_lnk:
-       case gfs2_inode_device:
-       case gfs2_inode_fifo:
-       case gfs2_inode_sock:
-               count[1]++;
-               return GFS2_BLKST_DINODE;
-
-       case gfs2_indir_blk:
-       case gfs2_leaf_blk:
-       /*case gfs2_meta_rgrp:*/
-       case gfs2_jdata: /* gfs1 jdata blocks count as "metadata" and gfs1
-                           metadata is marked the same as gfs2 inode in the
-                           bitmap. */
-       case gfs2_meta_eattr:
-               count[3]++;
-               return GFS2_BLKST_DINODE;
-
-       case gfs2_freemeta:
-               count[4]++;
-               return GFS2_BLKST_UNLINKED;
-
-       default:
-               log_err( _("Invalid block type %d found\n"), q);
-       }
-       return -1;
-}
-
-static int gfs2_convert_mark(uint8_t q, uint32_t *count)
-{
-       switch(q) {
-
-       case gfs2_meta_inval:
-       case gfs2_inode_invalid:
-               /* Convert invalid metadata to free blocks */
-       case gfs2_block_free:
-               count[0]++;
-               return GFS2_BLKST_FREE;
-
-       case gfs2_block_used:
-               count[2]++;
-               return GFS2_BLKST_USED;
-
-       case gfs2_inode_dir:
-       case gfs2_inode_file:
-       case gfs2_inode_lnk:
-       case gfs2_inode_device:
-       case gfs2_jdata: /* gfs1 jdata blocks count as "metadata" and gfs1
-                           metadata is marked the same as gfs2 inode in the
-                           bitmap. */
-       case gfs2_inode_fifo:
-       case gfs2_inode_sock:
-               count[1]++;
-               return GFS2_BLKST_DINODE;
-
-       case gfs2_indir_blk:
-       case gfs2_leaf_blk:
-       case gfs2_meta_eattr:
-               count[2]++;
-               return GFS2_BLKST_USED;
-
-       case gfs2_freemeta:
-               log_err( _("Invalid freemeta type %d found\n"), q);
-               count[4]++;
-               return -1;
-
-       default:
-               log_err( _("Invalid block type %d found\n"), q);
-       }
-       return -1;
-}
-
 static int check_block_status(struct gfs2_sbd *sdp, char *buffer,
                              unsigned int buflen, uint64_t *rg_block,
                              uint64_t rg_data, uint32_t *count)
@@ -107,7 +19,6 @@ static int check_block_status(struct gfs2_sbd *sdp, char 
*buffer,
        unsigned char *byte, *end;
        unsigned int bit;
        unsigned char rg_status;
-       int block_status;
        uint8_t q;
        uint64_t block;
 
@@ -123,26 +34,33 @@ static int check_block_status(struct gfs2_sbd *sdp, char 
*buffer,
                if (skip_this_pass || fsck_abort) /* if asked to skip the rest 
*/
                        return 0;
 
-               q = block_type(block);
-
-               if (sdp->gfs1)
-                       block_status = gfs1_convert_mark(q, count);
-               else
-                       block_status = gfs2_convert_mark(q, count);
-
-               if (block_status < 0) {
-                       log_err( _("Invalid status for block %llu (0x%llx).\n"),
-                                (unsigned long long)block,
-                                (unsigned long long)block);
-                       return block_status;
+               q = blockmap_to_bitmap(block_type(block), sdp->gfs1);
+               /* GFS1 file systems will have to suffer from slower fsck run
+                * times because in GFS, there's no 1:1 relationship between
+                * bits and counts. If a bit is marked "dinode" in GFS1, it
+                * may be dinode -OR- any kind of metadata. I consider GFS1 to
+                * be a rare exception, so acceptable loss at this point. So
+                * we must determine whether it's really a dinode or other
+                * metadata by reading it in. */
+               if (sdp->gfs1 && q == GFS2_BLKST_DINODE) {
+                       struct gfs2_buffer_head *bh;
+
+                       bh = bread(sdp, block);
+                       if (gfs2_check_meta(bh, GFS2_METATYPE_DI) == 0)
+                               count[GFS2_BLKST_DINODE]++;
+                       else
+                               count[GFS2_BLKST_USED]++;
+                       brelse(bh);
+               } else {
+                       count[q]++;
                }
+
                /* If one node opens a file and another node deletes it, we
                   may be left with a block that appears to be "unlinked" in
                   the bitmap, but nothing links to it. This is a valid case
                   and should be cleaned up by the file system eventually.
                   So we ignore it. */
-               if (rg_status == GFS2_BLKST_UNLINKED &&
-                   block_status == GFS2_BLKST_FREE) {
+               if (q == GFS2_BLKST_UNLINKED) {
                        log_err( _("Unlinked inode found at block %llu "
                                   "(0x%llx).\n"),
                                 (unsigned long long)block,
@@ -150,33 +68,33 @@ static int check_block_status(struct gfs2_sbd *sdp, char 
*buffer,
                        if (query(_("Do you want to reclaim the block? "
                                   "(y/n) "))) {
                                lgfs2_rgrp_t rg = gfs2_blk2rgrpd(sdp, block);
-                               if (gfs2_set_bitmap(rg, block, block_status))
+                               if (gfs2_set_bitmap(rg, block, GFS2_BLKST_FREE))
                                        log_err(_("Unlinked block %llu "
                                                  "(0x%llx) bitmap not fixed."
                                                  "\n"),
                                                (unsigned long long)block,
                                                (unsigned long long)block);
-                               else
+                               else {
                                        log_err(_("Unlinked block %llu "
                                                  "(0x%llx) bitmap fixed.\n"),
                                                (unsigned long long)block,
                                                (unsigned long long)block);
+                                       count[GFS2_BLKST_UNLINKED]--;
+                                       count[GFS2_BLKST_FREE]++;
+                               }
                        } else {
                                log_info( _("Unlinked block found at block %llu"
                                            " (0x%llx), left unchanged.\n"),
                                        (unsigned long long)block,
                                        (unsigned long long)block);
                        }
-               } else if (rg_status != block_status) {
-                       const char *blockstatus[] = {"Free", "Data",
-                                                    "Unlinked", "inode"};
-
+               } else if (rg_status != q) {
                        log_err( _("Block %llu (0x%llx) bitmap says %u (%s) "
                                   "but FSCK saw %u (%s)\n"),
                                 (unsigned long long)block,
                                 (unsigned long long)block, rg_status,
-                                blockstatus[rg_status], block_status,
-                                blockstatus[block_status]);
+                                block_type_string(rg_status), q,
+                                block_type_string(q));
                        if (q) /* Don't print redundant "free" */
                                log_err( _("Metadata type is %u (%s)\n"), q,
                                         block_type_string(q));
@@ -185,7 +103,7 @@ static int check_block_status(struct gfs2_sbd *sdp, char 
*buffer,
                                 (unsigned long long)block,
                                 (unsigned long long)block)) {
                                lgfs2_rgrp_t rg = gfs2_blk2rgrpd(sdp, block);
-                               if (gfs2_set_bitmap(rg, block, block_status))
+                               if (gfs2_set_bitmap(rg, block, q))
                                        log_err( _("Repair failed.\n"));
                                else
                                        log_err( _("Fixed.\n"));
@@ -235,38 +153,41 @@ static void update_rgrp(struct gfs2_sbd *sdp, struct 
rgrp_tree *rgp,
                rgp->rg.rg_free = count[0];
                update = 1;
        }
-       if (rgp->rg.rg_dinodes != count[1]) {
+       if (rgp->rg.rg_dinodes != count[3]) {
                log_err( _("RG #%llu (0x%llx) Inode count inconsistent: is "
                           "%u should be %u\n"),
                         (unsigned long long)rgp->ri.ri_addr,
                         (unsigned long long)rgp->ri.ri_addr,
-                        rgp->rg.rg_dinodes, count[1]);
-               rgp->rg.rg_dinodes = count[1];
+                        rgp->rg.rg_dinodes, count[3]);
+               rgp->rg.rg_dinodes = count[3];
                update = 1;
        }
-       if (sdp->gfs1 && gfs1rg->rg_usedmeta != count[3]) {
+       if (sdp->gfs1 && gfs1rg->rg_usedmeta != count[1]) {
                log_err( _("RG #%llu (0x%llx) Used metadata count "
                           "inconsistent: is %u should be %u\n"),
                         (unsigned long long)rgp->ri.ri_addr,
                         (unsigned long long)rgp->ri.ri_addr,
-                        gfs1rg->rg_usedmeta, count[3]);
-               gfs1rg->rg_usedmeta = count[3];
+                        gfs1rg->rg_usedmeta, count[1]);
+               gfs1rg->rg_usedmeta = count[1];
                update = 1;
        }
-       if (sdp->gfs1 && gfs1rg->rg_freemeta != count[4]) {
+       if (sdp->gfs1 && gfs1rg->rg_freemeta != count[2]) {
                log_err( _("RG #%llu (0x%llx) Free metadata count "
                           "inconsistent: is %u should be %u\n"),
                         (unsigned long long)rgp->ri.ri_addr,
                         (unsigned long long)rgp->ri.ri_addr,
-                        gfs1rg->rg_freemeta, count[4]);
-               gfs1rg->rg_freemeta = count[4];
+                        gfs1rg->rg_freemeta, count[2]);
+               gfs1rg->rg_freemeta = count[2];
                update = 1;
        }
-       if (!sdp->gfs1 && (rgp->ri.ri_data - count[0] - count[1]) != count[2]) {
+       if (!sdp->gfs1 && (rgp->ri.ri_data != count[0] + count[1] +
+                          count[2] + count[3])) {
                /* FIXME not sure how to handle this case ATM - it
                 * means that the total number of blocks we've counted
                 * exceeds the blocks in the rg */
-               log_err( _("Internal fsck error - AAHHH!\n"));
+               log_err( _("Internal fsck error: %u != %u + %u + %u + %u\n"),
+                        rgp->ri.ri_data, count[0], count[1], count[2],
+                        count[3]);
                exit(FSCK_ERROR);
        }
        if (update) {

Reply via email to