Hi,

Before this patch, fsck.gfs2 called "reprocess_inode" in several
places, especially when the number of blocks had changed from the
original. That works in almost all cases. However, there's a corner
case where a block may be deleted, due to errors (for example, a
bad directory leaf), and another block takes its place. In that
case the count of the number of blocks is still the same, but the
inode should be reprocessed anyway, because the deleted blocks
and replacement blocks may be at different locations or maybe of
different types. A hash table block may be "data" when it's freed,
but if the block is reused, it may be repurposed as a "leaf" block.
That may confuse subsequent processing. Another reason to call
reprocess_inode is when the goal block changes to a value outside
the resource group, due to block allocations for repairs.

Regards,

Bob Peterson
Red Hat File Systems

Signed-off-by: Bob Peterson <rpete...@redhat.com> 
---
diff --git a/gfs2/fsck/lost_n_found.c b/gfs2/fsck/lost_n_found.c
index 9672c7a..b2e35e2 100644
--- a/gfs2/fsck/lost_n_found.c
+++ b/gfs2/fsck/lost_n_found.c
@@ -174,7 +174,6 @@ void make_sure_lf_exists(struct gfs2_inode *ip)
 int add_inode_to_lf(struct gfs2_inode *ip){
        char tmp_name[256];
        __be32 inode_type;
-       uint64_t lf_blocks;
        struct gfs2_sbd *sdp = ip->i_sbd;
        int err = 0;
        uint32_t mode;
@@ -184,7 +183,6 @@ int add_inode_to_lf(struct gfs2_inode *ip){
                log_err( _("Trying to add lost+found to itself...skipping"));
                return 0;
        }
-       lf_blocks = lf_dip->i_di.di_blocks;
 
        if (sdp->gfs1)
                mode = gfs_to_gfs2_mode(ip);
@@ -242,10 +240,6 @@ int add_inode_to_lf(struct gfs2_inode *ip){
                         tmp_name, strerror(errno));
                exit(FSCK_ERROR);
        }
-       /* If the lf directory had new blocks added we have to mark them
-          properly in the bitmap so they're not freed. */
-       if (lf_dip->i_di.di_blocks != lf_blocks)
-               reprocess_inode(lf_dip, "lost+found");
 
        /* This inode is linked from lost+found */
        incr_link_count(ip->i_di.di_num, lf_dip, _("from lost+found"));
diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
index 16faafb..217bb07 100644
--- a/gfs2/fsck/metawalk.c
+++ b/gfs2/fsck/metawalk.c
@@ -1693,11 +1693,11 @@ int check_dir(struct gfs2_sbd *sdp, uint64_t block, 
struct metawalk_fxns *pass)
 {
        struct gfs2_inode *ip;
        int error = 0;
-       uint64_t cur_blks;
+       struct alloc_state as;
 
        ip = fsck_load_inode(sdp, block);
 
-       cur_blks = ip->i_di.di_blocks;
+       astate_save(ip, &as);
 
        if (ip->i_di.di_flags & GFS2_DIF_EXHASH)
                error = check_leaf_blks(ip, pass);
@@ -1707,7 +1707,7 @@ int check_dir(struct gfs2_sbd *sdp, uint64_t block, 
struct metawalk_fxns *pass)
        if (error < 0)
                stack;
 
-       if (ip->i_di.di_blocks != cur_blks)
+       if (astate_changed(ip, &as))
                reprocess_inode(ip, _("Current"));
 
        fsck_inode_put(&ip); /* does a brelse */
diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c
index 4ea322a..74bcc26 100644
--- a/gfs2/fsck/pass2.c
+++ b/gfs2/fsck/pass2.c
@@ -1723,7 +1723,8 @@ build_it:
 static int check_system_dir(struct gfs2_inode *sysinode, const char *dirname,
                     int builder(struct gfs2_sbd *sdp))
 {
-       uint64_t iblock = 0, cur_blks;
+       uint64_t iblock = 0;
+       struct alloc_state as;
        struct dir_status ds = {0};
        int error = 0;
 
@@ -1740,14 +1741,14 @@ static int check_system_dir(struct gfs2_inode 
*sysinode, const char *dirname,
 
        pass2_fxns.private = (void *) &ds;
        if (ds.q == gfs2_bad_block) {
-               cur_blks = sysinode->i_di.di_blocks;
+               astate_save(sysinode, &as);
                /* First check that the directory's metatree is valid */
                error = check_metatree(sysinode, &pass2_fxns);
                if (error < 0) {
                        stack;
                        return error;
                }
-               if (sysinode->i_di.di_blocks != cur_blks)
+               if (astate_changed(sysinode, &as))
                        reprocess_inode(sysinode, _("System inode"));
        }
        error = check_dir(sysinode->i_sbd, iblock, &pass2_fxns);
@@ -1768,7 +1769,7 @@ static int check_system_dir(struct gfs2_inode *sysinode, 
const char *dirname,
        if (!ds.dotdir) {
                log_err( _("No '.' entry found for %s directory.\n"), dirname);
                if (query( _("Is it okay to add '.' entry? (y/n) "))) {
-                       cur_blks = sysinode->i_di.di_blocks;
+                       astate_save(sysinode, &as);
                        log_warn( _("Adding '.' entry\n"));
                        error = dir_add(sysinode, ".", 1, 
&(sysinode->i_di.di_num),
                                        (sysinode->i_sbd->gfs1 ? GFS_FILE_DIR : 
DT_DIR));
@@ -1777,7 +1778,7 @@ static int check_system_dir(struct gfs2_inode *sysinode, 
const char *dirname,
                                         strerror(errno));
                                return -errno;
                        }
-                       if (cur_blks != sysinode->i_di.di_blocks)
+                       if (astate_changed(sysinode, &as))
                                reprocess_inode(sysinode, dirname);
                        /* This system inode is linked to itself via '.' */
                        incr_link_count(sysinode->i_di.di_num, sysinode,
@@ -1859,7 +1860,8 @@ static inline int is_system_dir(struct gfs2_sbd *sdp, 
uint64_t block)
  */
 int pass2(struct gfs2_sbd *sdp)
 {
-       uint64_t dirblk, cur_blks;
+       uint64_t dirblk;
+       struct alloc_state as;
        uint8_t q;
        struct dir_status ds = {0};
        struct gfs2_inode *ip;
@@ -1926,14 +1928,14 @@ int pass2(struct gfs2_sbd *sdp)
                        /* First check that the directory's metatree
                         * is valid */
                        ip = fsck_load_inode(sdp, dirblk);
-                       cur_blks = ip->i_di.di_blocks;
+                       astate_save(ip, &as);
                        error = check_metatree(ip, &pass2_fxns);
                        if (error < 0) {
                                stack;
                                fsck_inode_put(&ip);
                                return error;
                        }
-                       if (ip->i_di.di_blocks != cur_blks)
+                       if (astate_changed(ip, &as))
                                reprocess_inode(ip, "current");
                        fsck_inode_put(&ip);
                }
@@ -1994,7 +1996,7 @@ int pass2(struct gfs2_sbd *sdp)
                                (unsigned long long)dirblk);
 
                        if (query( _("Is it okay to add '.' entry? (y/n) "))) {
-                               cur_blks = ip->i_di.di_blocks;
+                               astate_save(ip, &as);
                                error = dir_add(ip, ".", 1, &(ip->i_di.di_num),
                                                (sdp->gfs1 ? GFS_FILE_DIR : 
DT_DIR));
                                if (error) {
@@ -2003,7 +2005,7 @@ int pass2(struct gfs2_sbd *sdp)
                                        fsck_inode_put(&ip);
                                        return -errno;
                                }
-                               if (cur_blks != ip->i_di.di_blocks) {
+                               if (astate_changed(ip, &as)) {
                                        char dirname[80];
 
                                        sprintf(dirname, _("Directory at %lld "
diff --git a/gfs2/fsck/pass3.c b/gfs2/fsck/pass3.c
index 33865df..3e183e5 100644
--- a/gfs2/fsck/pass3.c
+++ b/gfs2/fsck/pass3.c
@@ -24,7 +24,7 @@ static int attach_dotdot_to(struct gfs2_sbd *sdp, uint64_t 
newdotdot,
        int filename_len = 2;
        int err;
        struct gfs2_inode *ip, *pip;
-       uint64_t cur_blks;
+       struct alloc_state as;
 
        ip = fsck_load_inode(sdp, block);
        pip = fsck_load_inode(sdp, newdotdot);
@@ -39,7 +39,7 @@ static int attach_dotdot_to(struct gfs2_sbd *sdp, uint64_t 
newdotdot,
                log_warn( _("Unable to remove \"..\" directory entry.\n"));
        else
                decr_link_count(olddotdot, block, _("old \"..\""));
-       cur_blks = ip->i_di.di_blocks;
+       astate_save(ip, &as);
        err = dir_add(ip, filename, filename_len, &pip->i_di.di_num,
                      (sdp->gfs1 ? GFS_FILE_DIR : DT_DIR));
        if (err) {
@@ -47,7 +47,7 @@ static int attach_dotdot_to(struct gfs2_sbd *sdp, uint64_t 
newdotdot,
                        filename, strerror(errno));
                exit(FSCK_ERROR);
        }
-       if (cur_blks != ip->i_di.di_blocks) {
+       if (astate_changed(ip, &as)) {
                char dirname[80];
 
                sprintf(dirname, _("Directory at %lld (0x%llx)"),
@@ -179,6 +179,7 @@ int pass3(struct gfs2_sbd *sdp)
        struct dir_info *di, *tdi;
        struct gfs2_inode *ip;
        uint8_t q;
+       struct alloc_state lf_as = {.as_blocks = 0, .as_meta_goal = 0};
 
        di = dirtree_find(sdp->md.rooti->i_di.di_num.no_addr);
        if (di) {
@@ -225,6 +226,8 @@ int pass3(struct gfs2_sbd *sdp)
         */
        log_info( _("Checking directory linkage.\n"));
        for (tmp = osi_first(&dirtree); tmp; tmp = next) {
+               if (lf_dip && lf_as.as_blocks == 0)
+                       astate_save(lf_dip, &lf_as);
                next = osi_next(tmp);
                di = (struct dir_info *)tmp;
                while (!di->checked) {
@@ -330,8 +333,14 @@ int pass3(struct gfs2_sbd *sdp)
                        break;
                }
        }
-       if (lf_dip)
+       if (lf_dip) {
+               /* If the lf directory had new blocks added we have to mark
+                  them properly in the blockmap so they're not freed. */
+               if (astate_changed(lf_dip, &lf_as))
+                       reprocess_inode(lf_dip, "lost+found");
+               
                log_debug( _("At end of pass3, lost+found entries is %u\n"),
                                  lf_dip->i_di.di_entries);
+       }
        return FSCK_OK;
 }
diff --git a/gfs2/fsck/pass4.c b/gfs2/fsck/pass4.c
index f9d08dc..324ea9f 100644
--- a/gfs2/fsck/pass4.c
+++ b/gfs2/fsck/pass4.c
@@ -48,12 +48,15 @@ static int scan_inode_list(struct gfs2_sbd *sdp) {
        struct gfs2_inode *ip;
        int lf_addition = 0;
        uint8_t q;
+       struct alloc_state lf_as = {.as_blocks = 0, .as_meta_goal = 0};
 
        /* FIXME: should probably factor this out into a generic
         * scanning fxn */
        for (tmp = osi_first(&inodetree); tmp; tmp = next) {
                if (skip_this_pass || fsck_abort) /* if asked to skip the rest 
*/
                        return 0;
+               if (lf_dip && lf_as.as_blocks == 0)
+                       astate_save(lf_dip, &lf_as);
                next = osi_next(tmp);
                ii = (struct inode_info *)tmp;
                /* Don't check reference counts on the special gfs files */
@@ -179,6 +182,9 @@ static int scan_inode_list(struct gfs2_sbd *sdp) {
                         (unsigned long long)ii->di_num.no_addr, ii->di_nlink);
        } /* osi_list_foreach(tmp, list) */
 
+       if (lf_dip && astate_changed(lf_dip, &lf_as))
+               reprocess_inode(lf_dip, "lost+found");
+
        if (lf_addition) {
                if (!(ii = inodetree_find(lf_dip->i_di.di_num.no_addr))) {
                        log_crit( _("Unable to find lost+found inode in 
inode_hash!!\n"));
diff --git a/gfs2/fsck/util.h b/gfs2/fsck/util.h
index 276c726..66b9c7a 100644
--- a/gfs2/fsck/util.h
+++ b/gfs2/fsck/util.h
@@ -12,6 +12,11 @@
 #define INODE_VALID 1
 #define INODE_INVALID 0
 
+struct alloc_state {
+       uint64_t as_blocks;
+       uint64_t as_meta_goal;
+};
+
 struct di_info *search_list(osi_list_t *list, uint64_t addr);
 void big_file_comfort(struct gfs2_inode *ip, uint64_t blks_checked);
 void warm_fuzzy_stuff(uint64_t block);
@@ -27,6 +32,21 @@ extern const char *reftypes[ref_types + 1];
 #define BLOCKMAP_BYTE_OFFSET4(x) (((x) & 0x0000000000000001) << 2)
 #define BLOCKMAP_MASK4 (0xf)
 
+static inline void astate_save(struct gfs2_inode *ip, struct alloc_state *as)
+{
+       as->as_blocks = ip->i_di.di_blocks;
+       as->as_meta_goal = ip->i_di.di_goal_meta;
+}
+
+static inline int astate_changed(struct gfs2_inode *ip, struct alloc_state *as)
+{
+       if (as->as_blocks != ip->i_di.di_blocks)
+               return 1;
+       if (as->as_meta_goal != ip->i_di.di_goal_meta)
+               return 1;
+       return 0;
+}
+
 static inline uint8_t block_type(uint64_t bblock)
 {
        static unsigned char *byte;

Reply via email to