cluster-commits January 2010

cluster-commits@lists.stg.fedorahosted.org

7 participants
186 discussions

cluster: STABLE3 - fsck.gfs2: Don't add extended attrib blocks to list twice
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=9f… Commit: 9f288bf60cc6d6b5285ba207d277a382c53f8460 Parent: 684770304edb7a23cc1d49a27553ba7d0035ec5a Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Mon Jan 25 14:43:14 2010 -0600 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Jan 26 14:39:32 2010 -0600 fsck.gfs2: Don't add extended attrib blocks to list twice This patch adds a simple check to pass1 where it is adding a block to the list of blocks that contain an extended attribute. If the block is already on the list, no need to add it again. This can sometimes happen in duplicate reference situations and other obscure code paths. Since pass1 processes the file system sequentially by block number it's enough to check against the block most recently added to the list. This is a performance increase; early versions of this code ran the entire linked list of special blocks. If the file system was labelled for selinux, it could have millions of entries on the linked list, causing an enormous slowdown. rhbz#455300 --- gfs2/fsck/pass1.c | 26 ++++++++++++++++++++++---- 1 files changed, 22 insertions(+), 4 deletions(-) diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 0a57939..aab44a9 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -494,6 +494,8 @@ static int finish_eattr_indir(struct gfs2_inode *ip, int leaf_pointers, int leaf_pointer_errors, void *private) { struct block_count *bc = (struct block_count *) private; + osi_list_t *head; + struct special_blocks *b = NULL; if (leaf_pointer_errors == leaf_pointers) /* All eas were bad */ return ask_remove_inode_eattr(ip, bc); @@ -502,8 +504,17 @@ static int finish_eattr_indir(struct gfs2_inode *ip, int leaf_pointers, (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); /* Mark the inode as having an eattr in the block map - so pass1c can check it. */ - gfs2_special_add(&ip->i_sbd->eattr_blocks, ip->i_di.di_num.no_addr); + so pass1c can check it. We may have previously added this inode + to the eattr_blocks list and if we did, it would be the first + one on the list. So check that one only (to save time) and + if that one matches, no need to add it again. */ + if (!osi_list_empty(&ip->i_sbd->eattr_blocks.list)) { + head = &ip->i_sbd->eattr_blocks.list; + b = osi_list_entry(head->next, struct special_blocks, list); + } + if (!b || b->block != ip->i_di.di_num.no_addr) + gfs2_special_add(&ip->i_sbd->eattr_blocks, + ip->i_di.di_num.no_addr); if (!leaf_pointer_errors) return 0; log_err( _("Inode %lld (0x%llx) has recoverable indirect " @@ -624,6 +635,8 @@ static int check_eattr_leaf(struct gfs2_inode *ip, uint64_t block, void *private) { struct gfs2_sbd *sdp = ip->i_sbd; + osi_list_t *head; + struct special_blocks *b = NULL; /* This inode contains an eattr - it may be invalid, but the * eattr attributes points to a non-zero block. @@ -634,8 +647,13 @@ static int check_eattr_leaf(struct gfs2_inode *ip, uint64_t block, "block(s) attached.\n"), (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); - gfs2_special_add(&sdp->eattr_blocks, ip->i_di.di_num.no_addr); - if(gfs2_check_range(sdp, block)) { + if (!osi_list_empty(&ip->i_sbd->eattr_blocks.list)) { + head = &ip->i_sbd->eattr_blocks.list; + b = osi_list_entry(head->next, struct special_blocks, list); + } + if (!b || b->block != ip->i_di.di_num.no_addr) + gfs2_special_add(&sdp->eattr_blocks, ip->i_di.di_num.no_addr); + if (gfs2_check_range(sdp, block)) { log_warn( _("Inode #%llu (0x%llx): Extended Attribute leaf " "block #%llu (0x%llx) is out of range.\n"), (unsigned long long)ip->i_di.di_num.no_addr,

1 0

cluster: STABLE3 - fsck.gfs2: Free metadata list memory we don't need
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=68… Commit: 684770304edb7a23cc1d49a27553ba7d0035ec5a Parent: 43632210b4fc2121005cb8ca5bbe1d10cdc97da6 Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Mon Jan 25 14:34:16 2010 -0600 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Jan 26 14:39:32 2010 -0600 fsck.gfs2: Free metadata list memory we don't need This patch modifies function check_metatree so that it frees up metadata from memory for heights it's no longer interested in. This patches a memory leak. rhbz#455300 --- gfs2/fsck/metawalk.c | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-) diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 4a6651f..4ab2603 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -1298,6 +1298,21 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) return error; } + /* Free the metalist buffers from heights we don't need to check. + For the rest we'll free as we check them to save time. + metalist[0] will only have the dinode bh, so we can skip it. */ + for (i = 1; i < height - 1; i++) { + list = &metalist[i]; + while (!osi_list_empty(list)) { + bh = osi_list_entry(list->next, + struct gfs2_buffer_head, b_altlist); + if (bh == ip->i_bh) + osi_list_del(&bh->b_altlist); + else + brelse(bh); + } + } + /* check data blocks */ list = &metalist[height - 1]; if (ip->i_di.di_blocks > COMFORTABLE_BLKS)

1 0

cluster: STABLE3 - fsck.gfs2: Force intermediate lost+found inode updates
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=43… Commit: 43632210b4fc2121005cb8ca5bbe1d10cdc97da6 Parent: fbf9933c3626b3e670fec6f2d218eec3c36d81bc Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Mon Jan 25 14:27:31 2010 -0600 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Jan 26 14:39:32 2010 -0600 fsck.gfs2: Force intermediate lost+found inode updates This patch forces the lost+found dinode to be rewritten every time an orphan inode is moved into lost+found. That way, if the user interrupts fsck.gfs2 during pass4, they won't be left with a lost+found dinode with an invalid number of blocks, links and so forth. rhbz#455300 --- gfs2/fsck/lost_n_found.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/gfs2/fsck/lost_n_found.c b/gfs2/fsck/lost_n_found.c index c6a8a6e..c5051f9 100644 --- a/gfs2/fsck/lost_n_found.c +++ b/gfs2/fsck/lost_n_found.c @@ -183,5 +183,7 @@ int add_inode_to_lf(struct gfs2_inode *ip){ log_notice( _("Added inode #%llu (0x%llx) to lost+found dir\n"), (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); + gfs2_dinode_out(&lf_dip->i_di, lf_dip->i_bh); + bwrite(lf_dip->i_bh); return 0; }

1 0

cluster: STABLE3 - fsck.gfs2: invalidate invalid mode inodes
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=fb… Commit: fbf9933c3626b3e670fec6f2d218eec3c36d81bc Parent: 1ab1e80fa38db9de3d9146ee1a5845c91c007970 Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Mon Jan 25 13:52:20 2010 -0600 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Jan 26 14:39:32 2010 -0600 fsck.gfs2: invalidate invalid mode inodes When fsck.gfs2 encountered a dinode that had an invalid type (file, directory, socket, etc), it marked that block in the blockmap as an invalid block. That caused pass2 to delete it AND its metadata. Since the metadata had not been previously processed, it would be deleted wholesale. That's okay for most cases, but if there was a duplicate reference to any of those metadata blocks, pass2 would not realize it and free the block anyway, out from under the valid reference. In other words, the valid reference to the block would remain intact but the block was freed in the bitmap because it was associated with the dinode that had an unknown type. That means big time corruption because future block allocations could then reuse the freed metadata, once again causing another duplicate block reference. This patch invalidates the metadata associated with dinodes that do not have a valid inode type. That causes all of its metadata to be processed and duplicate references are put on the new invalid references list. Later, in pass1b, the duplicate block references are resolved and the last remaining reference is left in a valid state. And pass2 had no more cause to free the block. rhbz#455300 --- gfs2/fsck/pass1.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 100 insertions(+), 0 deletions(-) diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 8ce1206..0a57939 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -59,6 +59,19 @@ static int check_extended_leaf_eattr(struct gfs2_inode *ip, uint64_t *data_ptr, void *private); static int finish_eattr_indir(struct gfs2_inode *ip, int leaf_pointers, int leaf_pointer_errors, void *private); +static int invalidate_metadata(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head **bh, void *private); +static int invalidate_leaf(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head *bh, void *private); +static int invalidate_data(struct gfs2_inode *ip, uint64_t block, + void *private); +static int invalidate_eattr_indir(struct gfs2_inode *ip, uint64_t block, + uint64_t parent, + struct gfs2_buffer_head **bh, + void *private); +static int invalidate_eattr_leaf(struct gfs2_inode *ip, uint64_t block, + uint64_t parent, struct gfs2_buffer_head **bh, + void *private); struct metawalk_fxns pass1_fxns = { .private = NULL, @@ -79,6 +92,15 @@ struct metawalk_fxns undo_fxns = { .check_data = undo_check_data, }; +struct metawalk_fxns invalidate_fxns = { + .private = NULL, + .check_metalist = invalidate_metadata, + .check_data = invalidate_data, + .check_leaf = invalidate_leaf, + .check_eattr_indir = invalidate_eattr_indir, + .check_eattr_leaf = invalidate_eattr_leaf, +}; + static int leaf(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head *bh, void *private) { @@ -669,6 +691,72 @@ static int check_eattr_entries(struct gfs2_inode *ip, } /** + * mark_block_invalid - mark blocks associated with an inode as invalid + * unless the block is a duplicate. + * + * An "invalid" block is now considered free in the bitmap, and pass2 will + * delete any invalid blocks. This is nearly identical to function + * delete_block_if_notdup. + */ +static int mark_block_invalid(struct gfs2_inode *ip, uint64_t block, + enum dup_ref_type reftype, const char *btype) +{ + uint8_t q; + + if (gfs2_check_range(ip->i_sbd, block) != 0) + return -EFAULT; + + q = block_type(block); + if (q != gfs2_block_free) { + add_duplicate_ref(ip, block, reftype, 0, INODE_INVALID); + log_info( _("%s block %lld (0x%llx), part of inode " + "%lld (0x%llx), was free so the invalid " + "reference is ignored.\n"), + btype, (unsigned long long)block, + (unsigned long long)block, + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr); + return 0; + } + fsck_blockmap_set(ip, block, btype, gfs2_meta_inval); + return 0; +} + +static int invalidate_metadata(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head **bh, void *private) +{ + return mark_block_invalid(ip, block, ref_as_meta, _("metadata")); +} + +static int invalidate_leaf(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head *bh, void *private) +{ + return mark_block_invalid(ip, block, ref_as_meta, _("leaf")); +} + +static int invalidate_data(struct gfs2_inode *ip, uint64_t block, + void *private) +{ + return mark_block_invalid(ip, block, ref_as_data, _("data")); +} + +static int invalidate_eattr_indir(struct gfs2_inode *ip, uint64_t block, + uint64_t parent, + struct gfs2_buffer_head **bh, void *private) +{ + return mark_block_invalid(ip, block, ref_as_ea, + _("indirect extended attribute")); +} + +static int invalidate_eattr_leaf(struct gfs2_inode *ip, uint64_t block, + uint64_t parent, struct gfs2_buffer_head **bh, + void *private) +{ + return mark_block_invalid(ip, block, ref_as_ea, + _("extended attribute")); +} + +/** * Check for massive amounts of pointer corruption. If the block has * lots of out-of-range pointers, we can't trust any of the pointers. * For example, a stray pointer with a value of 0x1d might be @@ -878,6 +966,18 @@ static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh, } break; default: + /* We found a dinode that has an invalid mode, so we can't + tell if it's a data file, directory or a socket. + Regardless, we have to invalidate its metadata in case there + are duplicate blocks referenced. If we don't call + check_metatree, the blocks it references will be deleted + wholesale by pass2, and if any of those blocks are + duplicates--referenced by another dinode for some reason-- + we will mark it free, even though it's in use. In other + words, we would introduce file system corruption. So we + need to keep track of the fact that it's invalid and + skip parts that we can't be sure of based on dinode type. */ + check_metatree(ip, &invalidate_fxns); if (fsck_blockmap_set(ip, block, _("invalid mode"), gfs2_inode_invalid)) { stack;

1 0

cluster: STABLE3 - fsck.gfs2: Overhaul duplicate reference processing
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=1a… Commit: 1ab1e80fa38db9de3d9146ee1a5845c91c007970 Parent: 3f3635066f6cc191bf77576ee6c6622fee5b446c Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Mon Jan 25 13:39:24 2010 -0600 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Jan 26 14:39:32 2010 -0600 fsck.gfs2: Overhaul duplicate reference processing This patch is a major overhaul of the duplicate reference processing that had so many problems. Before this patch, pass1 would flag a block as a duplicate reference, then pass1b would go to great lengths to find all dinodes that reference the block in question. Then it tried to figure out which reference to keep and which to discard. This was slow and prone to errors. With this patch, when pass1 discovers a block that's already got a blocktype, it keeps tabs on that block and what dinodes reference it from then on. That means pass1b only needs to find one reference: The original reference that caused it to become not free. Once that is found, it can stop looking. Furthermore, this patch introduces two linked lists of inode references: valid references and invalid references. When an inode is determined to be bad or invalid, its duplicate block references are moved from the valid to the invalid reference list. That way, pass1b can favor the most valid references over the invalid references, and it's more likely to keep good data and discard trash. rhbz#455300 --- gfs2/fsck/fsck.h | 6 +- gfs2/fsck/main.c | 1 + gfs2/fsck/pass1.c | 62 +++--- gfs2/fsck/pass1b.c | 569 +++++++++++++++++++++++++++++++--------------------- gfs2/fsck/util.c | 134 ++++++++++++- gfs2/fsck/util.h | 8 +- 6 files changed, 517 insertions(+), 263 deletions(-) diff --git a/gfs2/fsck/fsck.h b/gfs2/fsck/fsck.h index 30eb223..022b61e 100644 --- a/gfs2/fsck/fsck.h +++ b/gfs2/fsck/fsck.h @@ -71,7 +71,7 @@ struct inode_with_dups { osi_list_t list; uint64_t block_no; int dup_count; - int ea_only; + int reftypecount[ref_types]; uint64_t parent; char *name; }; @@ -126,5 +126,7 @@ extern uint64_t first_data_block; extern struct osi_root dup_blocks; extern struct osi_root dirtree; extern struct osi_root inodetree; - +extern int dups_found; /* How many duplicate references have we found? */ +extern int dups_found_first; /* How many duplicates have we found the original + reference for? */ #endif /* _FSCK_H */ diff --git a/gfs2/fsck/main.c b/gfs2/fsck/main.c index 43aa77a..6bddeea 100644 --- a/gfs2/fsck/main.c +++ b/gfs2/fsck/main.c @@ -32,6 +32,7 @@ int preen = 0, force_check = 0; struct osi_root dup_blocks = (struct osi_root) { NULL, }; struct osi_root dirtree = (struct osi_root) { NULL, }; struct osi_root inodetree = (struct osi_root) { NULL, }; +int dups_found = 0, dups_found_first = 0; /* This function is for libgfs2's sake. */ void print_it(const char *label, const char *fmt, const char *fmt2, ...) diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index a993466..8ce1206 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -127,7 +127,7 @@ static int check_metalist(struct gfs2_inode *ip, uint64_t block, (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr, q, block_type_string(q)); - gfs2_dup_set(block); + add_duplicate_ref(ip, block, ref_as_meta, 0, INODE_VALID); found_dup = 1; } nbh = bread(ip->i_sbd, block); @@ -150,10 +150,12 @@ static int check_metalist(struct gfs2_inode *ip, uint64_t block, } else /* blk check ok */ *bh = nbh; - if (!found_dup) + bc->indir_count++; + if (found_dup) + return 1; /* don't process the metadata again */ + else fsck_blockmap_set(ip, block, _("indirect"), gfs2_indir_blk); - bc->indir_count++; return 0; } @@ -218,7 +220,6 @@ static int check_data(struct gfs2_inode *ip, uint64_t block, void *private) { uint8_t q; struct block_count *bc = (struct block_count *) private; - int error = 0; if (gfs2_check_range(ip->i_sbd, block)) { log_err( _("inode %lld (0x%llx) has a bad data block pointer " @@ -236,32 +237,33 @@ static int check_data(struct gfs2_inode *ip, uint64_t block, void *private) } q = block_type(block); if (q != gfs2_block_free) { - log_err( _("Found duplicate block referenced as data at %" - PRIu64 " (0x%"PRIx64 ")\n"), block, block); + log_err( _("Found duplicate %s block %llu (0x%llx) " + "referenced as data by dinode %llu (0x%llx)\n"), + block_type_string(q), + (unsigned long long)block, + (unsigned long long)block, + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr); if (q != gfs2_meta_inval) { - gfs2_dup_set(block); + log_info( _("Seems to be a normal duplicate; I'll " + "sort it out in pass1b.\n")); + add_duplicate_ref(ip, block, ref_as_data, 0, + INODE_VALID); /* If the prev ref was as data, this is likely a data block, so keep the block count for both refs. */ if (q == gfs2_block_used) bc->data_count++; return 1; } - /* An explanation is in order here. At this point we found - a duplicate block, a block that was already referenced - somewhere else. We'll resolve those duplicates in pass1b. - However, if the block is marked "invalid" that's a special - case. It's likely that the block was discovered to be - invalid metadata--i.e. doesn't have a metadata header. - However, it still may be a valid data block, since they - won't have metadata headers. In that case, the block is - marked as duplicate, but also as a data block. */ - error = 1; - gfs2_block_unmark(ip->i_sbd, bl, block, gfs2_meta_inval); - gfs2_dup_set(block); + log_info( _("The block was invalid as metadata but might be " + "okay as data. I'll sort it out in pass1b.\n")); + add_duplicate_ref(ip, block, ref_as_data, 0, INODE_VALID); + bc->data_count++; + return 1; } fsck_blockmap_set(ip, block, _("data"), gfs2_block_used); bc->data_count++; - return error; + return 0; } static int undo_check_data(struct gfs2_inode *ip, uint64_t block, @@ -433,12 +435,12 @@ static int check_eattr_indir(struct gfs2_inode *ip, uint64_t indirect, *bh = bread(sdp, indirect); if(gfs2_check_meta(*bh, GFS2_METATYPE_IN)) { if(q != gfs2_block_free) { /* Duplicate? */ + add_duplicate_ref(ip, indirect, ref_as_ea, 0, + INODE_VALID); if (!clear_eas(ip, bc, indirect, 1, _("Bad indirect Extended Attribute " - "duplicate found"))) { - gfs2_dup_set(indirect); + "duplicate found"))) bc->ea_count++; - } return 1; } clear_eas(ip, bc, indirect, 0, @@ -454,7 +456,7 @@ static int check_eattr_indir(struct gfs2_inode *ip, uint64_t indirect, (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)indirect, (unsigned long long)indirect); - gfs2_dup_set(indirect); + add_duplicate_ref(ip, indirect, ref_as_ea, 0, INODE_VALID); bc->ea_count++; ret = 1; } else { @@ -512,6 +514,8 @@ static int check_leaf_block(struct gfs2_inode *ip, uint64_t block, int btype, leaf_bh = bread(sdp, block); if(gfs2_check_meta(leaf_bh, btype)) { if(q != gfs2_block_free) { /* Duplicate? */ + add_duplicate_ref(ip, block, ref_as_ea, 0, + INODE_VALID); clear_eas(ip, bc, block, 1, _("Bad Extended Attribute duplicate found")); } else { @@ -526,7 +530,7 @@ static int check_leaf_block(struct gfs2_inode *ip, uint64_t block, int btype, log_debug( _("Duplicate block found at #%lld (0x%llx).\n"), (unsigned long long)block, (unsigned long long)block); - gfs2_dup_set(block); + add_duplicate_ref(ip, block, ref_as_data, 0, INODE_VALID); bc->ea_count++; brelse(leaf_bh); return 1; @@ -766,10 +770,10 @@ static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh, q = block_type(block); if(q != gfs2_block_free) { - log_err( _("Found duplicate block referenced as an inode at " - "#%" PRIu64 " (0x%" PRIx64 ")\n"), block, block); - gfs2_dup_set(block); - fsck_inode_put(&ip); + log_err( _("Found a duplicate inode block at #%" PRIu64 + " (0x%" PRIx64 ") previously marked as a %s\n"), + block, block, block_type_string(q)); + add_duplicate_ref(ip, block, ref_as_meta, 0, INODE_VALID); return 0; } diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c index 1180d98..e8e7428 100644 --- a/gfs2/fsck/pass1b.c +++ b/gfs2/fsck/pass1b.c @@ -3,6 +3,7 @@ #include <string.h> #include <unistd.h> #include <libintl.h> +#include <sys/stat.h> #define _(String) gettext(String) #include "libgfs2.h" @@ -25,28 +26,62 @@ struct dup_handler { int ref_count; }; -static inline void inc_if_found(uint64_t block, int not_ea, void *private) { - struct fxn_info *fi = (struct fxn_info *) private; - if(block == fi->block) { - (fi->found)++; - if(not_ea) - fi->ea_only = 0; - } -} +static int check_metalist(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head **bh, void *private); +static int check_data(struct gfs2_inode *ip, uint64_t block, void *private); +static int check_eattr_indir(struct gfs2_inode *ip, uint64_t block, + uint64_t parent, struct gfs2_buffer_head **bh, + void *private); +static int check_eattr_leaf(struct gfs2_inode *ip, uint64_t block, + uint64_t parent, struct gfs2_buffer_head **bh, + void *private); +static int check_eattr_entry(struct gfs2_inode *ip, + struct gfs2_buffer_head *leaf_bh, + struct gfs2_ea_header *ea_hdr, + struct gfs2_ea_header *ea_hdr_prev, + void *private); +static int check_eattr_extentry(struct gfs2_inode *ip, uint64_t *ea_data_ptr, + struct gfs2_buffer_head *leaf_bh, + struct gfs2_ea_header *ea_hdr, + struct gfs2_ea_header *ea_hdr_prev, + void *private); +static int find_dentry(struct gfs2_inode *ip, struct gfs2_dirent *de, + struct gfs2_dirent *prev, struct gfs2_buffer_head *bh, + char *filename, uint16_t *count, void *priv); + +struct metawalk_fxns find_refs = { + .private = NULL, + .check_leaf = NULL, + .check_metalist = check_metalist, + .check_data = check_data, + .check_eattr_indir = check_eattr_indir, + .check_eattr_leaf = check_eattr_leaf, + .check_dentry = NULL, + .check_eattr_entry = check_eattr_entry, + .check_eattr_extentry = check_eattr_extentry, +}; + +struct metawalk_fxns find_dirents = { + .private = NULL, + .check_leaf = NULL, + .check_metalist = NULL, + .check_data = NULL, + .check_eattr_indir = NULL, + .check_eattr_leaf = NULL, + .check_dentry = find_dentry, + .check_eattr_entry = NULL, + .check_eattr_extentry = NULL, +}; static int check_metalist(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head **bh, void *private) { - inc_if_found(block, 1, private); - - return 0; + return add_duplicate_ref(ip, block, ref_as_meta, 1, INODE_VALID); } static int check_data(struct gfs2_inode *ip, uint64_t block, void *private) { - inc_if_found(block, 1, private); - - return 0; + return add_duplicate_ref(ip, block, ref_as_data, 1, INODE_VALID); } static int check_eattr_indir(struct gfs2_inode *ip, uint64_t block, @@ -54,13 +89,13 @@ static int check_eattr_indir(struct gfs2_inode *ip, uint64_t block, void *private) { struct gfs2_sbd *sbp = ip->i_sbd; - struct gfs2_buffer_head *indir_bh = NULL; + int error; - inc_if_found(block, 0, private); - indir_bh = bread(sbp, block); - *bh = indir_bh; + error = add_duplicate_ref(ip, block, ref_as_ea, 1, INODE_VALID); + if (!error) + *bh = bread(sbp, block); - return 0; + return error; } static int check_eattr_leaf(struct gfs2_inode *ip, uint64_t block, @@ -68,13 +103,12 @@ static int check_eattr_leaf(struct gfs2_inode *ip, uint64_t block, void *private) { struct gfs2_sbd *sbp = ip->i_sbd; - struct gfs2_buffer_head *leaf_bh = NULL; + int error; - inc_if_found(block, 0, private); - leaf_bh = bread(sbp, block); - - *bh = leaf_bh; - return 0; + error = add_duplicate_ref(ip, block, ref_as_ea, 1, INODE_VALID); + if (!error) + *bh = bread(sbp, block); + return error; } static int check_eattr_entry(struct gfs2_inode *ip, @@ -93,8 +127,39 @@ static int check_eattr_extentry(struct gfs2_inode *ip, uint64_t *ea_data_ptr, { uint64_t block = be64_to_cpu(*ea_data_ptr); - inc_if_found(block, 0, private); + return add_duplicate_ref(ip, block, ref_as_ea, 1, INODE_VALID); +} +/* + * check_dir_dup_ref - check for a directory entry duplicate reference + * and if found, set the name into the id. + * Returns: 1 if filename was found, otherwise 0 + */ +static int check_dir_dup_ref(struct gfs2_inode *ip, struct gfs2_dirent *de, + osi_list_t *tmp2, char *filename) +{ + struct inode_with_dups *id; + + id = osi_list_entry(tmp2, struct inode_with_dups, list); + if(id->name) + /* We can only have one parent of inodes that contain duplicate + * blocks...no need to keep looking for this one. */ + return 1; + if(id->block_no == de->de_inum.no_addr) { + id->name = strdup(filename); + id->parent = ip->i_di.di_num.no_addr; + log_debug( _("Duplicate block %llu (0x%llx" + ") is in file or directory %llu" + " (0x%llx) named %s\n"), + (unsigned long long)id->block_no, + (unsigned long long)id->block_no, + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr, + filename); + /* If there are duplicates of duplicates, I guess we'll miss + them here. */ + return 1; + } return 0; } @@ -106,43 +171,27 @@ static int find_dentry(struct gfs2_inode *ip, struct gfs2_dirent *de, struct osi_node *n; osi_list_t *tmp2; struct duptree *b; - struct inode_with_dups *id; - struct gfs2_leaf leaf; + int found; for (n = osi_first(&dup_blocks); n; n = osi_next(n)) { b = (struct duptree *)n; - osi_list_foreach(tmp2, &b->ref_inode_list) { - id = osi_list_entry(tmp2, struct inode_with_dups, - list); - if(id->name) - /* We can only have one parent of - * inodes that contain duplicate - * blocks... */ - continue; - if(id->block_no == de->de_inum.no_addr) { - id->name = strdup(filename); - id->parent = ip->i_di.di_num.no_addr; - log_debug( _("Duplicate block %llu (0x%llx" - ") is in file or directory %llu" - " (0x%llx) named %s\n"), - (unsigned long long)id->block_no, - (unsigned long long)id->block_no, - (unsigned long long) - ip->i_di.di_num.no_addr, - (unsigned long long) - ip->i_di.di_num.no_addr, - filename); - /* If there are duplicates of - * duplicates, I guess we'll miss them - * here */ + found = 0; + osi_list_foreach(tmp2, &b->ref_invinode_list) { + if (check_dir_dup_ref(ip, de, tmp2, filename)) { + found = 1; break; } } + if (!found) { + osi_list_foreach(tmp2, &b->ref_inode_list) { + if (check_dir_dup_ref(ip, de, tmp2, filename)) + break; + } + } } /* Return the number of leaf entries so metawalk doesn't flag this leaf as having none. */ - gfs2_leaf_in(&leaf, bh); - *count = leaf.lf_entries; + *count = be16_to_cpu(((struct gfs2_leaf *)bh->b_data)->lf_entries); return 0; } @@ -150,12 +199,32 @@ static int clear_dup_metalist(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head **bh, void *private) { struct dup_handler *dh = (struct dup_handler *) private; - - if(dh->ref_count == 1) - return 1; + struct duptree *d; + + if (gfs2_check_range(ip->i_sbd, block) != 0) + return 0; + + /* This gets tricky. We're traversing a metadata tree trying to + delete an inode based on it having a duplicate block reference + somewhere in its metadata. We know this block is listed as data + or metadata for this inode, but it may or may not be one of the + actual duplicate references that caused the problem. If it's not + a duplicate, it's normal metadata that isn't referenced anywhere + else, but we're deleting the inode out from under it, so we need + to delete it altogether. If the block is a duplicate referenced + block, we need to keep its type intact and let the caller sort + it out once we're down to a single reference. */ + d = dupfind(block); + if (!d) { + fsck_blockmap_set(ip, block, _("no longer valid"), + gfs2_block_free); + return 0; + } + /* This block, having failed the above test, is duplicated somewhere */ if(block == dh->b->block) { - log_err( _("Found duplicate reference in inode \"%s\" at " - "block #%llu (0x%llx) to block #%llu (0x%llx)\n"), + log_err( _("Not clearing duplicate reference in inode \"%s\" " + "at block #%llu (0x%llx) to block #%llu (0x%llx) " + "because it's valid for another inode.\n"), dh->id->name ? dh->id->name : _("unknown name"), (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr, @@ -163,13 +232,15 @@ static int clear_dup_metalist(struct gfs2_inode *ip, uint64_t block, log_err( _("Inode %s is in directory %"PRIu64" (0x%" PRIx64 ")\n"), dh->id->name ? dh->id->name : "", dh->id->parent, dh->id->parent); - /* Setting the block to invalid means the inode is - * cleared in pass2 */ - fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, - _("inode with duplicate"), - gfs2_inode_invalid); } - return 0; + /* We return 1 not 0 because we need build_and_check_metalist to + bypass adding the metadata below it to the metalist. If that + were to happen, all the indirect blocks pointed to by the + duplicate block would be processed twice, which means it might + be mistakenly freed as "no longer valid" (in this function above) + even though it's valid metadata for a different inode. Returning + 1 ensures that the metadata isn't processed again. */ + return 1; } static int clear_dup_data(struct gfs2_inode *ip, uint64_t block, void *private) @@ -181,55 +252,14 @@ static int clear_dup_eattr_indir(struct gfs2_inode *ip, uint64_t block, uint64_t parent, struct gfs2_buffer_head **bh, void *private) { - struct dup_handler *dh = (struct dup_handler *) private; - /* Can't use fxns from eattr.c since we need to check the ref - * count */ - *bh = NULL; - if(dh->ref_count == 1) - return 1; - if(block == dh->b->block) { - log_err( _("Found dup in inode \"%s\" with address #%llu" - " (0x%llx) with block #%llu (0x%llx)\n"), - dh->id->name ? dh->id->name : _("unknown name"), - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)block, - (unsigned long long)block); - log_err( _("Inode %s is in directory %" PRIu64 " (0x%" PRIx64 ")\n"), - dh->id->name ? dh->id->name : "", - dh->id->parent, dh->id->parent); - fsck_blockmap_set(ip, ip->i_di.di_eattr, - _("indirect eattr"), gfs2_meta_inval); - } - - return 0; + return clear_dup_metalist(ip, block, NULL, private); } static int clear_dup_eattr_leaf(struct gfs2_inode *ip, uint64_t block, uint64_t parent, struct gfs2_buffer_head **bh, void *private) { - struct dup_handler *dh = (struct dup_handler *) private; - - if(dh->ref_count == 1) - return 1; - if(block == dh->b->block) { - log_err( _("Found dup in inode \"%s\" with address #%llu" - " (0x%llx) with block #%llu (0x%llx)\n"), - dh->id->name ? dh->id->name : _("unknown name"), - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)block, - (unsigned long long)block); - log_err( _("Inode %s is in directory %" PRIu64 " (0x%" PRIx64 ")\n"), - dh->id->name ? dh->id->name : "", - dh->id->parent, dh->id->parent); - /* mark the main eattr block invalid */ - fsck_blockmap_set(ip, ip->i_di.di_eattr, - _("indirect eattr leaf"), gfs2_meta_inval); - } - - return 0; + return clear_dup_metalist(ip, block, NULL, private); } static int clear_eattr_entry (struct gfs2_inode *ip, @@ -281,88 +311,86 @@ static int clear_eattr_extentry(struct gfs2_inode *ip, uint64_t *ea_data_ptr, void *private) { uint64_t block = be64_to_cpu(*ea_data_ptr); - struct dup_handler *dh = (struct dup_handler *) private; - - if(dh->ref_count == 1) - return 1; - if(block == dh->b->block) { - log_err( _("Found dup in inode \"%s\" with address #%llu" - " (0x%llx) with block #%llu (0x%llx)\n"), - dh->id->name ? dh->id->name : _("unknown name"), - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)block, (unsigned long long)block); - log_err( _("Inode %s is in directory %" PRIu64 " (0x%" PRIx64 ")\n"), - dh->id->name ? dh->id->name : "", - dh->id->parent, dh->id->parent); - /* mark the main eattr block invalid */ - fsck_blockmap_set(ip, ip->i_di.di_eattr, - _("extended eattr leaf"), gfs2_meta_inval); - } - - return 0; + return clear_dup_metalist(ip, block, NULL, private); } /* Finds all references to duplicate blocks in the metadata */ -static int find_block_ref(struct gfs2_sbd *sbp, uint64_t inode, struct duptree *b) +static int find_block_ref(struct gfs2_sbd *sbp, uint64_t inode) { struct gfs2_inode *ip; - struct fxn_info myfi = {b->block, 0, 1}; - struct inode_with_dups *id = NULL; - struct metawalk_fxns find_refs = { - .private = (void*) &myfi, - .check_leaf = NULL, - .check_metalist = check_metalist, - .check_data = check_data, - .check_eattr_indir = check_eattr_indir, - .check_eattr_leaf = check_eattr_leaf, - .check_dentry = NULL, - .check_eattr_entry = check_eattr_entry, - .check_eattr_extentry = check_eattr_extentry, - }; + int error = 0; ip = fsck_load_inode(sbp, inode); /* bread, inode_get */ - log_debug( _("Checking inode %" PRIu64 " (0x%" PRIx64 ")'s " - "metatree for references to block %" PRIu64 " (0x%" PRIx64 - ")\n"), inode, inode, b->block, b->block); - if(check_metatree(ip, &find_refs)) { - stack; - fsck_inode_put(&ip); /* out, brelse, free */ - return -1; + /*log_debug( _("Checking inode %" PRIu64 " (0x%" PRIx64 ")'s " + "metatree for references to duplicate blocks)\n"), + inode, inode);*/ + /* double-check the meta header just to be sure it's metadata */ + if (ip->i_di.di_header.mh_magic != GFS2_MAGIC || + ip->i_di.di_header.mh_type != GFS2_METATYPE_DI) { + log_debug( _("Block %lld (0x%llx) is not gfs2 metadata.\n"), + (unsigned long long)inode, + (unsigned long long)inode); + return 1; } - log_debug( _("Done checking metatree\n")); - /* Check for ea references in the inode */ - if(check_inode_eattr(ip, &find_refs) < 0){ + error = check_metatree(ip, &find_refs); + if (error < 0) { stack; fsck_inode_put(&ip); /* out, brelse, free */ - return -1; - } - if (myfi.found) { - if(!(id = malloc(sizeof(*id)))) { - log_crit( _("Unable to allocate inode_with_dups structure\n")); - return -1; - } - if(!(memset(id, 0, sizeof(*id)))) { - log_crit( _("Unable to zero inode_with_dups structure\n")); - return -1; - } - log_debug( _("Found %d entries with block %" PRIu64 - " (0x%" PRIx64 ") in inode #%" PRIu64 " (0x%" PRIx64 ")\n"), - myfi.found, b->block, b->block, inode, inode); - id->dup_count = myfi.found; - id->block_no = inode; - id->ea_only = myfi.ea_only; - osi_list_add_prev(&id->list, &b->ref_inode_list); + return error; } + + /* Exhash dir leafs will be checked by check_metatree (right after + the "end:" label.) But if this is a linear directory we need to + check the dir with check_linear_dir. */ + if(S_ISDIR(ip->i_di.di_mode) && !(ip->i_di.di_flags & GFS2_DIF_EXHASH)) + error = check_linear_dir(ip, ip->i_bh, &find_dirents); + + /* Check for ea references in the inode */ + if(!error) + error = check_inode_eattr(ip, &find_refs); + fsck_inode_put(&ip); /* out, brelse, free */ - return 0; + + return error; } -static int handle_dup_blk(struct gfs2_sbd *sbp, struct duptree *b) +static void log_inode_reference(struct duptree *b, osi_list_t *tmp, int inval) { - osi_list_t *tmp; + char reftypestring[32]; + struct inode_with_dups *id; + + id = osi_list_entry(tmp, struct inode_with_dups, list); + if (id->dup_count == 1) { + if (id->reftypecount[ref_as_data]) + strcpy(reftypestring, "as data"); + else if (id->reftypecount[ref_as_meta]) + strcpy(reftypestring, "as metadata"); + else + strcpy(reftypestring, "as extended attribute"); + } else { + sprintf(reftypestring, "%d/%d/%d", + id->reftypecount[ref_as_data], + id->reftypecount[ref_as_meta], + id->reftypecount[ref_as_ea]); + } + if (inval) + log_warn( _("Invalid ")); + log_warn( _("Inode %s (%lld/0x%llx) has %d reference(s) to " + "block %llu (0x%llx) (%s)\n"), id->name, + (unsigned long long)id->block_no, + (unsigned long long)id->block_no, id->dup_count, + (unsigned long long)b->block, + (unsigned long long)b->block, reftypestring); +} + +static int clear_a_reference(struct gfs2_sbd *sbp, struct duptree *b, + osi_list_t *ref_list, struct dup_handler *dh, + int inval) +{ + struct gfs2_inode *ip; struct inode_with_dups *id; + osi_list_t *tmp, *x; struct metawalk_fxns clear_dup_fxns = { .private = NULL, .check_leaf = NULL, @@ -374,9 +402,60 @@ static int handle_dup_blk(struct gfs2_sbd *sbp, struct duptree *b) .check_eattr_entry = clear_eattr_entry, .check_eattr_extentry = clear_eattr_extentry, }; + + osi_list_foreach_safe(tmp, ref_list, x) { + id = osi_list_entry(tmp, struct inode_with_dups, list); + dh->b = b; + dh->id = id; + if(dh->ref_inode_count == 1) /* down to the last reference */ + return 1; + if (!(query( _("Okay to clear %s inode %lld (0x%llx)? (y/n) "), + (inval ? _("invalidated") : ""), + (unsigned long long)id->block_no, + (unsigned long long)id->block_no))) { + log_warn( _("The bad inode was not cleared...\n")); + continue; + } + log_warn( _("Clearing inode %lld (0x%llx)....\n"), + (unsigned long long)id->block_no, + (unsigned long long)id->block_no); + clear_dup_fxns.private = (void *) dh; + /* Clear the EAs for the inode first */ + ip = fsck_load_inode(sbp, id->block_no); + check_inode_eattr(ip, &clear_dup_fxns); + /* If the dup wasn't only in the EA, clear the inode */ + if (id->reftypecount[ref_as_data] || + id->reftypecount[ref_as_meta]) + check_metatree(ip, &clear_dup_fxns); + + fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, + _("bad"), gfs2_inode_invalid); + fsck_inode_put(&ip); /* out, brelse, free */ + (dh->ref_inode_count)--; + /* Inode is marked invalid and is removed in pass2 */ + /* FIXME: other option should be to duplicate the + * block for each duplicate and point the metadata at + * the cloned blocks */ + } + if(dh->ref_inode_count == 1) /* down to the last reference */ + return 1; + return 0; +} + +static int handle_dup_blk(struct gfs2_sbd *sbp, struct duptree *b) +{ struct gfs2_inode *ip; + osi_list_t *tmp; + struct inode_with_dups *id; struct dup_handler dh = {0}; + int last_reference, ref_in_invalid_inode = 0; + osi_list_foreach(tmp, &b->ref_invinode_list) { + id = osi_list_entry(tmp, struct inode_with_dups, list); + dh.ref_inode_count++; + dh.ref_count += id->dup_count; + ref_in_invalid_inode = 1; + } osi_list_foreach(tmp, &b->ref_inode_list) { id = osi_list_entry(tmp, struct inode_with_dups, list); dh.ref_inode_count++; @@ -390,6 +469,11 @@ static int handle_dup_blk(struct gfs2_sbd *sbp, struct duptree *b) just _look_ like metadata by coincidence, and at the time we're checking, we might not have processed the referenced block. Here in pass1b we're sure. */ + /* Another possibility here is that there is a single reference + because all the other metadata references were in inodes that got + invalidated for other reasons, such as bad pointers. So we need to + make sure at this point that any inode deletes reverse out any + duplicate reference before we get to this point. */ if (dh.ref_count == 1) { struct gfs2_buffer_head *bh; uint32_t cmagic; @@ -398,7 +482,10 @@ static int handle_dup_blk(struct gfs2_sbd *sbp, struct duptree *b) cmagic = ((struct gfs2_meta_header *)(bh->b_data))->mh_magic; brelse(bh); if (be32_to_cpu(cmagic) == GFS2_MAGIC) { - tmp = b->ref_inode_list.next; + if (ref_in_invalid_inode) + tmp = b->ref_invinode_list.next; + else + tmp = b->ref_inode_list.next; id = osi_list_entry(tmp, struct inode_with_dups, list); log_warn( _("Inode %s (%lld/0x%llx) has a reference to" " data block %llu (0x%llx), " @@ -428,6 +515,13 @@ static int handle_dup_blk(struct gfs2_sbd *sbp, struct duptree *b) } return 0; } + /* The other references may have been discredited due to + invalid metadata or something. Use the last remaining. */ + log_notice( _("Block %llu (0x%llx) has only one remaining " + "reference.\n"), + (unsigned long long)b->block, + (unsigned long long)b->block); + return 0; } log_notice( _("Block %llu (0x%llx) has %d inodes referencing it" @@ -435,46 +529,65 @@ static int handle_dup_blk(struct gfs2_sbd *sbp, struct duptree *b) (unsigned long long)b->block, (unsigned long long)b->block, dh.ref_inode_count, dh.ref_count); - osi_list_foreach(tmp, &b->ref_inode_list) { - id = osi_list_entry(tmp, struct inode_with_dups, list); - log_warn( _("Inode %s (%lld/0x%llx) has %d reference(s) to " - "block %llu (0x%llx)\n"), id->name, - (unsigned long long)id->block_no, - (unsigned long long)id->block_no, - id->dup_count, (unsigned long long)b->block, - (unsigned long long)b->block); - } - osi_list_foreach(tmp, &b->ref_inode_list) { + osi_list_foreach(tmp, &b->ref_invinode_list) + log_inode_reference(b, tmp, 1); + osi_list_foreach(tmp, &b->ref_inode_list) + log_inode_reference(b, tmp, 0); + + last_reference = clear_a_reference(sbp, b, &b->ref_invinode_list, + &dh, 1); + if (!last_reference) + last_reference = clear_a_reference(sbp, b, &b->ref_inode_list, + &dh, 0); + + if (last_reference) { + uint8_t q; + + /* If we're down to a single reference (and not all references + deleted, which may be the case of an inode that has only + itself and a reference), we need to reset the block type + from invalid to data or metadata. Start at the first one + in the list, not the structure's place holder. */ + tmp = (&b->ref_inode_list)->next; id = osi_list_entry(tmp, struct inode_with_dups, list); - if (!(query( _("Okay to clear inode %lld (0x%llx)? (y/n) "), - (unsigned long long)id->block_no, - (unsigned long long)id->block_no))) { - log_warn( _("The bad inode was not cleared...\n")); - continue; - } - log_warn( _("Clearing inode %lld (0x%llx)...\n"), - (unsigned long long)id->block_no, - (unsigned long long)id->block_no); + log_debug( _("Resetting the type based on the remaining " + "reference in inode %lld (0x%llx).\n"), + (unsigned long long)id->block_no, + (unsigned long long)id->block_no); ip = fsck_load_inode(sbp, id->block_no); - dh.b = b; - dh.id = id; - clear_dup_fxns.private = (void *) &dh; - /* Clear the EAs for the inode first */ - check_inode_eattr(ip, &clear_dup_fxns); - /* If the dup wasn't only in the EA, clear the inode */ - if(!id->ea_only) - check_metatree(ip, &clear_dup_fxns); - fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, - _("bad"), gfs2_inode_invalid); + q = block_type(id->block_no); + if (q == gfs2_inode_invalid) { + log_debug( _("The remaining reference inode %lld " + "(0x%llx) is marked invalid: Marking " + "the block as free.\n"), + (unsigned long long)id->block_no, + (unsigned long long)id->block_no); + fsck_blockmap_set(ip, b->block, + _("reference-repaired leaf"), + gfs2_block_free); + } else if (id->reftypecount[ref_as_data]) { + fsck_blockmap_set(ip, b->block, + _("reference-repaired data"), + gfs2_block_used); + } else if (id->reftypecount[ref_as_meta]) { + if (S_ISDIR(ip->i_di.di_mode)) + fsck_blockmap_set(ip, b->block, + _("reference-repaired leaf"), + gfs2_leaf_blk); + else + fsck_blockmap_set(ip, b->block, + _("reference-repaired " + "indirect"), + gfs2_indir_blk); + } else + fsck_blockmap_set(ip, b->block, + _("reference-repaired extended " + "attribute"), + gfs2_meta_eattr); fsck_inode_put(&ip); /* out, brelse, free */ - dh.ref_inode_count--; - if(dh.ref_inode_count == 1) - break; - /* Inode is marked invalid and is removed in pass2 */ - /* FIXME: other option should be to duplicate the - * block for each duplicate and point the metadata at - * the cloned blocks */ + } else { + log_debug( _("All duplicate references were resolved.\n")); } return 0; @@ -489,9 +602,7 @@ int pass1b(struct gfs2_sbd *sbp) uint64_t i; uint8_t q; struct osi_node *n; - struct metawalk_fxns find_dirents = {0}; int rc = FSCK_OK; - find_dirents.check_dentry = &find_dentry; log_info( _("Looking for duplicate blocks...\n")); @@ -506,29 +617,33 @@ int pass1b(struct gfs2_sbd *sbp) log_info( _("Scanning filesystem for inodes containing duplicate blocks...\n")); log_debug( _("Filesystem has %"PRIu64" (0x%" PRIx64 ") blocks total\n"), last_fs_block, last_fs_block); - for(i = 0; i < last_fs_block; i += 1) { - warm_fuzzy_stuff(i); + for(i = 0; i < last_fs_block; i++) { if (skip_this_pass || fsck_abort) /* if asked to skip the rest */ goto out; - log_debug( _("Scanning block %" PRIu64 " (0x%" PRIx64 ") for inodes\n"), - i, i); + + if (dups_found_first == dups_found) { + log_debug(_("Found all %d original references to " + "duplicates.\n"), dups_found); + break; + } q = block_type(i); if (q < gfs2_inode_dir) continue; - if (q > gfs2_inode_sock) + if (q > gfs2_inode_invalid) continue; - for (n = osi_first(&dup_blocks); n; n = osi_next(n)) { - b = (struct duptree *)n; - if(find_block_ref(sbp, i, b)) { - stack; - rc = FSCK_ERROR; - goto out; - } - } - if(q == gfs2_inode_dir) { - check_dir(sbp, i, &find_dirents); + if (q == gfs2_inode_invalid) + log_debug( _("Checking invalidated duplicate dinode " + "%lld (0x%llx)\n"), + (unsigned long long)i, + (unsigned long long)i); + + warm_fuzzy_stuff(i); + if (find_block_ref(sbp, i) < 0) { + stack; + rc = FSCK_ERROR; + goto out; } } diff --git a/gfs2/fsck/util.c b/gfs2/fsck/util.c index 4622eea..9827300 100644 --- a/gfs2/fsck/util.c +++ b/gfs2/fsck/util.c @@ -10,9 +10,10 @@ #include "libgfs2.h" #include "fs_bits.h" -#include "metawalk.h" #include "util.h" +const char *reftypes[3] = {"data", "metadata", "extended attribute"}; + void big_file_comfort(struct gfs2_inode *ip, uint64_t blks_checked) { static struct timeval tv; @@ -139,7 +140,15 @@ int fsck_query(const char *format, ...) return ret; } -struct duptree *gfs2_dup_set(uint64_t dblock) +/* + * gfs2_dup_set - Flag a block as a duplicate + * We keep the references in a red/black tree. We can't keep track of every + * single inode in the file system, so the first time this function is called + * will actually be for the second reference to the duplicated block. + * This will return the number of references to the block. + * + * create - will be set if the call is supposed to create the reference. */ +static struct duptree *gfs2_dup_set(uint64_t dblock, int create) { struct osi_node **newn = &dup_blocks.osi_node, *parent = NULL; struct duptree *data; @@ -157,18 +166,129 @@ struct duptree *gfs2_dup_set(uint64_t dblock) return cur; } + if (!create) + return NULL; data = malloc(sizeof(struct duptree)); + dups_found++; memset(data, 0, sizeof(struct duptree)); /* Add new node and rebalance tree. */ data->block = dblock; - data->refs = 2; /* first call is the second reference to the block */ + data->refs = 1; /* reference 1 is actually the reference we need to + discover in pass1b. */ + data->first_ref_found = 0; osi_list_init(&data->ref_inode_list); + osi_list_init(&data->ref_invinode_list); osi_link_node(&data->node, parent, newn); osi_insert_color(&data->node, &dup_blocks); return data; } +/* + * add_duplicate_ref - Add a duplicate reference to the duplicates tree list + * A new element of the tree will be created as needed + * When the first reference is discovered in pass1, it realizes it's a + * duplicate but it has already forgotten where the first reference was. + * So we need to recreate the duplicate reference structure if it's not there. + * Later, in pass1b, it has to go back through the file system + * and figure out those original references in order to resolve them. + */ +int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block, + enum dup_ref_type reftype, int first, int inode_valid) +{ + osi_list_t *ref; + struct inode_with_dups *id, *found_id; + struct duptree *dt; + + if (gfs2_check_range(ip->i_sbd, block) != 0) + return 0; + /* If this is not the first reference (i.e. all calls from pass1) we + need to create the duplicate reference. If this is pass1b, we want + to ignore references that aren't found. */ + dt = gfs2_dup_set(block, !first); + if (!dt) /* If this isn't a duplicate */ + return 0; + + /* If we found the duplicate reference but we've already discovered + the first reference (in pass1b) and the other references in pass1, + we don't need to count it, so just return. */ + if (dt->first_ref_found) + return 0; + + /* The first time this is called from pass1 is actually the second + reference. When we go back in pass1b looking for the original + reference, we don't want to increment the reference count because + it's already accounted for. */ + if (first) { + if (!dt->first_ref_found) { + dt->first_ref_found = 1; + dups_found_first++; /* We found another first ref. */ + } + } else { + dt->refs++; + } + + /* Check for a previous reference to this duplicate on the "invalid + inode" reference list. */ + found_id = NULL; + osi_list_foreach(ref, &dt->ref_invinode_list) { + id = osi_list_entry(ref, struct inode_with_dups, list); + + if (id->block_no == ip->i_di.di_num.no_addr) { + found_id = id; + break; + } + } + if (found_id == NULL) { + osi_list_foreach(ref, &dt->ref_inode_list) { + id = osi_list_entry(ref, struct inode_with_dups, list); + + if (id->block_no == ip->i_di.di_num.no_addr) { + found_id = id; + break; + } + } + } + if (found_id == NULL) { + /* Check for the inode on the invalid inode reference list. */ + uint8_t q; + + if(!(found_id = malloc(sizeof(*found_id)))) { + log_crit( _("Unable to allocate " + "inode_with_dups structure\n")); + return -1; + } + if(!(memset(found_id, 0, sizeof(*found_id)))) { + log_crit( _("Unable to zero inode_with_dups " + "structure\n")); + return -1; + } + found_id->block_no = ip->i_di.di_num.no_addr; + q = block_type(ip->i_di.di_num.no_addr); + /* If it's an invalid dinode, put it first on the invalid + inode reference list otherwise put it on the normal list. */ + if (!inode_valid || q == gfs2_inode_invalid) + osi_list_add_prev(&found_id->list, + &dt->ref_invinode_list); + else + osi_list_add_prev(&found_id->list, + &dt->ref_inode_list); + } + found_id->reftypecount[reftype]++; + found_id->dup_count++; + log_info( _("Found %d reference(s) to block %llu" + " (0x%llx) as %s in inode #%llu (0x%llx)\n"), + found_id->dup_count, (unsigned long long)block, + (unsigned long long)block, reftypes[reftype], + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr); + if (first) + log_info( _("This is the original reference.\n")); + else + log_info( _("This brings the total to: %d\n"), dt->refs); + return 0; +} + struct dir_info *dirtree_insert(uint64_t dblock) { struct osi_node **newn = &dirtree.osi_node, *parent = NULL; @@ -226,6 +346,14 @@ void dup_delete(struct duptree *b) struct inode_with_dups *id; osi_list_t *tmp; + while (!osi_list_empty(&b->ref_invinode_list)) { + tmp = (&b->ref_invinode_list)->next; + id = osi_list_entry(tmp, struct inode_with_dups, list); + if (id->name) + free(id->name); + osi_list_del(&id->list); + free(id); + } while (!osi_list_empty(&b->ref_inode_list)) { tmp = (&b->ref_inode_list)->next; id = osi_list_entry(tmp, struct inode_with_dups, list); diff --git a/gfs2/fsck/util.h b/gfs2/fsck/util.h index 3599b6b..d2c81db 100644 --- a/gfs2/fsck/util.h +++ b/gfs2/fsck/util.h @@ -7,11 +7,15 @@ #define fsck_lseek(fd, off) \ ((lseek((fd), (off), SEEK_SET) == (off)) ? 0 : -1) +#define INODE_VALID 1 +#define INODE_INVALID 0 + 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); -const char *block_type_string(uint8_t q); -struct duptree *gfs2_dup_set(uint64_t dblock); +int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block, + enum dup_ref_type reftype, int first, int inode_valid); +extern const char *reftypes[3]; static inline uint8_t block_type(uint64_t bblock) {

1 0

cluster: STABLE3 - fsck.gfs2: Make pass1 undo its work for unrecoverable inodes
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=3f… Commit: 3f3635066f6cc191bf77576ee6c6622fee5b446c Parent: cf82aa427614d6bae9216d78efe1782bfee336ec Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Mon Jan 25 12:47:27 2010 -0600 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Jan 26 14:39:32 2010 -0600 fsck.gfs2: Make pass1 undo its work for unrecoverable inodes When pass1 discovers a dinode that has so much damage it finally gives up on it, it was invalidating all the inode's metadata blocks. The problem with with doing that is that it often gave up on the dinode half-way through processing its metadata, so some of the metadata blocks were marked as some block type n the blockmap (such as indirect blocks or duplicate reference) while the unprocessed blocks were not marked at all, leaving them unaccounted for. Later, marking them as invalid caused subsequent processing of other dinodes that referenced those blocks to think they were duplicate references, often with disasterous consequences. Often the duplicate processing code in pass1b would favor the first ("invalidated") reference and delete the second ("good") reference, which means it introduced corruption. The proper thing to do is to undo its blockmap designations, and stop at the same place it did when it gave up on the dinode. This patch introduces a new set of "undo" metadata processing functions to accomplish this task. These undo_check_metalist and related functions MUST be kept in sync with pass1's normal pass1_fxns functions. In other words, they must stop at the same block of metadata and return the same error to ensure anything that pass1_fxns does is properly undone by the corresponding undo functions. rhbz#455300 --- gfs2/fsck/pass1.c | 197 +++++++++++++++++++++++++++++++++++++++++------------ 1 files changed, 154 insertions(+), 43 deletions(-) diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 2f6782a..a993466 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -36,7 +36,11 @@ static int leaf(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head *bh, void *private); static int check_metalist(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head **bh, void *private); +static int undo_check_metalist(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head **bh, void *private); static int check_data(struct gfs2_inode *ip, uint64_t block, void *private); +static int undo_check_data(struct gfs2_inode *ip, uint64_t block, + void *private); static int check_eattr_indir(struct gfs2_inode *ip, uint64_t indirect, uint64_t parent, struct gfs2_buffer_head **bh, void *private); @@ -69,6 +73,12 @@ struct metawalk_fxns pass1_fxns = { .finish_eattr_indir = finish_eattr_indir, }; +struct metawalk_fxns undo_fxns = { + .private = NULL, + .check_metalist = undo_check_metalist, + .check_data = undo_check_data, +}; + static int leaf(struct gfs2_inode *ip, uint64_t block, struct gfs2_buffer_head *bh, void *private) { @@ -148,6 +158,62 @@ static int check_metalist(struct gfs2_inode *ip, uint64_t block, return 0; } +static int undo_check_metalist(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head **bh, void *private) +{ + struct duptree *d; + int found_dup = 0, iblk_type; + struct gfs2_buffer_head *nbh; + struct block_count *bc = (struct block_count *)private; + + *bh = NULL; + + if (gfs2_check_range(ip->i_sbd, block)){ /* blk outside of FS */ + fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, + _("itself"), gfs2_block_free); + return 1; + } + if (S_ISDIR(ip->i_di.di_mode)) + iblk_type = GFS2_METATYPE_JD; + else + iblk_type = GFS2_METATYPE_IN; + + d = dupfind(block); + if (d) { + log_err( _("Reversing duplicate status of block %llu (0x%llx) " + "referenced as metadata in indirect block for " + "dinode %llu (0x%llx)\n"), + (unsigned long long)block, + (unsigned long long)block, + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr); + d->refs--; /* one less reference */ + if (d->refs == 1) + dup_delete(d); + found_dup = 1; + } + nbh = bread(ip->i_sbd, block); + + if (gfs2_check_meta(nbh, iblk_type)) { + if(!found_dup) { + fsck_blockmap_set(ip, block, _("bad indirect"), + gfs2_block_free); + brelse(nbh); + return 1; + } + brelse(nbh); + } else /* blk check ok */ + *bh = nbh; + + bc->indir_count--; + if (found_dup) + return 1; /* don't process the metadata again */ + else + fsck_blockmap_set(ip, block, _("bad indirect"), + gfs2_block_free); + return 0; +} + static int check_data(struct gfs2_inode *ip, uint64_t block, void *private) { uint8_t q; @@ -198,12 +264,87 @@ static int check_data(struct gfs2_inode *ip, uint64_t block, void *private) return error; } -static int remove_inode_eattr(struct gfs2_inode *ip, struct block_count *bc, - int duplicate) +static int undo_check_data(struct gfs2_inode *ip, uint64_t block, + void *private) { - if (!duplicate) - fsck_blockmap_set(ip, ip->i_di.di_eattr, _("deleted eattr"), + struct duptree *d; + struct block_count *bc = (struct block_count *) private; + + if (gfs2_check_range(ip->i_sbd, block)) { + /* Mark the owner of this block with the bad_block + * designator so we know to check it for out of range + * blocks later */ + fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, + _("bad (out of range) data"), gfs2_block_free); + return 1; + } + d = dupfind(block); + if (d) { + log_err( _("Reversing duplicate status of block %llu (0x%llx) " + "referenced as data by dinode %llu (0x%llx)\n"), + (unsigned long long)block, + (unsigned long long)block, + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr); + d->refs--; /* one less reference */ + if (d->refs == 1) + dup_delete(d); + bc->data_count--; + return 1; + } + fsck_blockmap_set(ip, block, _("data"), gfs2_block_free); + bc->data_count--; + return 0; +} + +static int remove_inode_eattr(struct gfs2_inode *ip, struct block_count *bc) +{ + struct duptree *dt; + struct inode_with_dups *id; + osi_list_t *ref; + int moved = 0; + + /* If it's a duplicate reference to the block, we need to check + if the reference is on the valid or invalid inodes list. + If it's on the valid inode's list, move it to the invalid + inodes list. The reason is simple: This inode, although + valid, has an now-invalid reference, so we should not give + this reference preferential treatment over others. */ + dt = dupfind(ip->i_di.di_eattr); + if (dt) { + osi_list_foreach(ref, &dt->ref_inode_list) { + id = osi_list_entry(ref, struct inode_with_dups, list); + if (id->block_no == ip->i_di.di_num.no_addr) { + log_debug( _("Moving inode %lld (0x%llx)'s " + "duplicate reference to %lld " + "(0x%llx) from the valid to the " + "invalid reference list.\n"), + (unsigned long long) + ip->i_di.di_num.no_addr, + (unsigned long long) + ip->i_di.di_num.no_addr, + (unsigned long long) + ip->i_di.di_eattr, + (unsigned long long) + ip->i_di.di_eattr); + /* Move from the normal to the invalid list */ + osi_list_del(&id->list); + osi_list_add_prev(&id->list, + &dt->ref_invinode_list); + moved = 1; + break; + } + } + if (!moved) + log_debug( _("Duplicate reference to %lld " + "(0x%llx) not moved.\n"), + (unsigned long long)ip->i_di.di_eattr, + (unsigned long long)ip->i_di.di_eattr); + } else { + delete_block(ip, ip->i_di.di_eattr, NULL, + "extended attribute", NULL); + } ip->i_di.di_eattr = 0; bc->ea_count = 0; ip->i_di.di_blocks = 1 + bc->indir_count + bc->data_count; @@ -219,8 +360,7 @@ static int ask_remove_inode_eattr(struct gfs2_inode *ip, "errors.\n"), (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); if (query( _("Clear all Extended Attributes from the inode? (y/n) "))){ - if (!remove_inode_eattr(ip, bc, - is_duplicate(ip->i_di.di_eattr))) + if (!remove_inode_eattr(ip, bc)) log_err( _("Extended attributes were removed.\n")); else log_err( _("Unable to remove inode eattr pointer; " @@ -250,7 +390,7 @@ static int clear_eas(struct gfs2_inode *ip, struct block_count *bc, (unsigned long long)block, (unsigned long long)block); if (query( _("Clear the bad Extended Attribute? (y/n) "))) { if (block == ip->i_di.di_eattr) { - remove_inode_eattr(ip, bc, duplicate); + remove_inode_eattr(ip, bc); log_err( _("The bad extended attribute was " "removed.\n")); } else if (!duplicate) { @@ -524,34 +664,6 @@ static int check_eattr_entries(struct gfs2_inode *ip, return 0; } -static int clear_metalist(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, void *private) -{ - *bh = NULL; - - if(!is_duplicate(block)) - fsck_blockmap_set(ip, block, _("cleared metadata"), - gfs2_block_free); - return 0; -} - -static int clear_data(struct gfs2_inode *ip, uint64_t block, void *private) -{ - if(!is_duplicate(block)) - fsck_blockmap_set(ip, block, _("cleared data"), - gfs2_block_free); - return 0; -} - -static int clear_leaf(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head *bh, void *private) -{ - if(!is_duplicate(block)) - fsck_blockmap_set(ip, block, _("cleared directory leaf"), - gfs2_block_free); - return 0; -} - /** * Check for massive amounts of pointer corruption. If the block has * lots of out-of-range pointers, we can't trust any of the pointers. @@ -650,7 +762,6 @@ static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh, struct gfs2_inode *ip; int error; struct block_count bc = {0}; - struct metawalk_fxns invalidate_metatree = {0}; long bad_pointers; q = block_type(block); @@ -809,14 +920,14 @@ static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh, "errors; invalidating.\n"), (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); - invalidate_metatree.check_metalist = clear_metalist; - invalidate_metatree.check_data = clear_data; - invalidate_metatree.check_leaf = clear_leaf; - - /* FIXME: Must set all leaves invalid as well */ - check_metatree(ip, &invalidate_metatree); + undo_fxns.private = &bc; + check_metatree(ip, &undo_fxns); + /* If we undo the metadata accounting, including metadatas + duplicate block status, we need to make sure later passes + don't try to free up the metadata referenced by this inode. + Therefore we mark the inode as free space. */ fsck_blockmap_set(ip, ip->i_di.di_num.no_addr, - _("corrupt"), gfs2_meta_inval); + _("corrupt"), gfs2_block_free); fsck_inode_put(&ip); return 0; }

1 0

cluster: STABLE3 - fsck.gfs2: cleanup: refactor pass3
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=cf… Commit: cf82aa427614d6bae9216d78efe1782bfee336ec Parent: f3bfe0f7ee0b6fc59ddcd399a4858eb05ebdeaec Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Mon Jan 25 11:44:45 2010 -0600 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Jan 26 14:39:32 2010 -0600 fsck.gfs2: cleanup: refactor pass3 This patch just cleans up pass3 a bit. It is mostly just rearranging the code to make it smaller and more readable. A check that was at the bottom of the pass3 function was moved to the top of the function and a continue inserted, which made it possible to change the indentation to be more readable. I also removed a place where the function was marking the dinode as modified. The code should only mark it modified in places where it actually IS modified, so this was removed. rhbz#455300 --- gfs2/fsck/pass3.c | 142 +++++++++++++++++++++++++--------------------------- 1 files changed, 68 insertions(+), 74 deletions(-) diff --git a/gfs2/fsck/pass3.c b/gfs2/fsck/pass3.c index c10e0bb..a15c63f 100644 --- a/gfs2/fsck/pass3.c +++ b/gfs2/fsck/pass3.c @@ -203,47 +203,21 @@ int pass3(struct gfs2_sbd *sbp) return FSCK_OK; tdi = mark_and_return_parent(sbp, di); - /* FIXME: Factor this ? */ - if(!tdi) { - q = block_type(di->dinode); - if(q == gfs2_bad_block) { - log_err( _("Found unlinked directory containing bad block\n")); - if(query( - _("Clear unlinked directory with bad blocks? (y/n) "))) { - log_warn( _("inode %lld (0x%llx) is " - "now marked as free\n"), - (unsigned long long) - di->dinode, - (unsigned long long) - di->dinode); - /* Can't use fsck_blockmap_set - because we don't have ip */ - gfs2_blockmap_set(bl, - di->dinode, - gfs2_block_free); - check_n_fix_bitmap(sbp, - di->dinode, - gfs2_block_free); - break; - } else - log_err( _("Unlinked directory with bad block remains\n")); - } - if(q != gfs2_inode_dir && - q != gfs2_inode_file && - q != gfs2_inode_lnk && - q != gfs2_inode_blk && - q != gfs2_inode_chr && - q != gfs2_inode_fifo && - q != gfs2_inode_sock) { - log_err( _("Unlinked block marked as inode not an inode\n")); - if(!query(_("Clear the unlinked block?" - " (y/n) "))) { - log_err( _("The block was not " - "cleared\n")); - break; - } - log_warn( _("inode %lld (0x%llx) is now " - "marked as free\n"), + if (tdi) { + log_debug( _("Directory at block %" PRIu64 + " (0x%" PRIx64 ") connected\n"), + di->dinode, di->dinode); + di = tdi; + continue; + } + q = block_type(di->dinode); + if(q == gfs2_bad_block) { + log_err( _("Found unlinked directory " + "containing bad block\n")); + if(query(_("Clear unlinked directory " + "with bad blocks? (y/n) "))) { + log_warn( _("inode %lld (0x%llx) is " + "now marked as free\n"), (unsigned long long) di->dinode, (unsigned long long) @@ -254,49 +228,69 @@ int pass3(struct gfs2_sbd *sbp) gfs2_block_free); check_n_fix_bitmap(sbp, di->dinode, gfs2_block_free); - log_err( _("The block was cleared\n")); + break; + } else + log_err( _("Unlinked directory with bad block remains\n")); + } + if(q != gfs2_inode_dir && q != gfs2_inode_file && + q != gfs2_inode_lnk && q != gfs2_inode_blk && + q != gfs2_inode_chr && q != gfs2_inode_fifo && + q != gfs2_inode_sock) { + log_err( _("Unlinked block marked as an inode " + "is not an inode\n")); + if(!query(_("Clear the unlinked block?" + " (y/n) "))) { + log_err( _("The block was not " + "cleared\n")); break; } + log_warn( _("inode %lld (0x%llx) is now " + "marked as free\n"), + (unsigned long long)di->dinode, + (unsigned long long)di->dinode); + /* Can't use fsck_blockmap_set + because we don't have ip */ + gfs2_blockmap_set(bl, di->dinode, + gfs2_block_free); + check_n_fix_bitmap(sbp, di->dinode, + gfs2_block_free); + log_err( _("The block was cleared\n")); + break; + } - log_err( _("Found unlinked directory at block %" PRIu64 - " (0x%" PRIx64 ")\n"), di->dinode, di->dinode); - ip = fsck_load_inode(sbp, di->dinode); - /* Don't skip zero size directories - * with eattrs */ - if(!ip->i_di.di_size && !ip->i_di.di_eattr){ - log_err( _("Unlinked directory has zero size.\n")); - if(query( _("Remove zero-size unlinked directory? (y/n) "))) { - fsck_blockmap_set(ip, - di->dinode, + log_err( _("Found unlinked directory at block %" PRIu64 + " (0x%" PRIx64 ")\n"), di->dinode, + di->dinode); + ip = fsck_load_inode(sbp, di->dinode); + /* Don't skip zero size directories with eattrs */ + if(!ip->i_di.di_size && !ip->i_di.di_eattr){ + log_err( _("Unlinked directory has zero " + "size.\n")); + if(query( _("Remove zero-size unlinked " + "directory? (y/n) "))) { + fsck_blockmap_set(ip, di->dinode, _("zero-sized unlinked inode"), gfs2_block_free); - fsck_inode_put(&ip); - break; - } else { - log_err( _("Zero-size unlinked directory remains\n")); - } - } - if(query( _("Add unlinked directory to " - "lost+found? (y/n) "))) { - if(add_inode_to_lf(ip)) { - fsck_inode_put(&ip); - stack; - return FSCK_ERROR; - } - log_warn( _("Directory relinked to lost+found\n")); - bmodified(ip->i_bh); fsck_inode_put(&ip); + break; } else { - log_err( _("Unlinked directory remains unlinked\n")); - fsck_inode_put(&ip); + log_err( _("Zero-size unlinked " + "directory remains\n")); } - break; } - else { - log_debug( _("Directory at block %" PRIu64 " (0x%" - PRIx64 ") connected\n"), di->dinode, di->dinode); + if(query( _("Add unlinked directory to " + "lost+found? (y/n) "))) { + if(add_inode_to_lf(ip)) { + fsck_inode_put(&ip); + stack; + return FSCK_ERROR; + } + log_warn( _("Directory relinked to lost+found\n")); + } else { + log_err( _("Unlinked directory remains unlinked\n")); } - di = tdi; + fsck_inode_put(&ip); + break; } } if(lf_dip)

1 0

cluster: STABLE3 - Create a standard metadata delete interface
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=f3… Commit: f3bfe0f7ee0b6fc59ddcd399a4858eb05ebdeaec Parent: b6cd280f35778a1fa69426df5d5bdbb87e6f0425 Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Mon Jan 25 11:13:31 2010 -0600 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Jan 26 14:39:32 2010 -0600 Create a standard metadata delete interface This patch creates a standard set of functions to delete metadata that has been deemed invalid. The fsck.gfs2 program had several places where it would delete or invalidate blocks it deemed bad without regard to previous designation as a duplicate block reference. A block would be marked data, then duplicate, then deleted, then the duplicate block was resolved, leaving it in a questionable state. This lays some groundwork for deleteing blocks that already have duplicate references. rhbz#455300 --- gfs2/fsck/eattr.c | 33 +--------------- gfs2/fsck/metawalk.c | 104 ++++++++++++++++++++++++++++++++++--------------- gfs2/fsck/metawalk.h | 22 +++++----- gfs2/fsck/pass1.c | 6 +-- gfs2/fsck/pass2.c | 5 +- 5 files changed, 90 insertions(+), 80 deletions(-) diff --git a/gfs2/fsck/eattr.c b/gfs2/fsck/eattr.c index 2f31c38..5d3e7cd 100644 --- a/gfs2/fsck/eattr.c +++ b/gfs2/fsck/eattr.c @@ -5,36 +5,8 @@ #include "libgfs2.h" #include "fsck.h" -#include "eattr.h" #include "metawalk.h" - -static int clear_blk_nodup(struct gfs2_inode *ip, uint64_t block) -{ - if(is_duplicate(block)) { - log_debug( _("Not clearing block with marked as a duplicate\n")); - return 1; - } - - fsck_blockmap_set(ip, block, _("cleared eattr block"), - gfs2_block_free); - - return 0; - -} - -int clear_eattr_indir(struct gfs2_inode *ip, uint64_t block, - uint64_t parent, struct gfs2_buffer_head **bh, - void *private) -{ - return clear_blk_nodup(ip, block); -} - -int clear_eattr_leaf(struct gfs2_inode *ip, uint64_t block, - uint64_t parent, struct gfs2_buffer_head **bh, - void *private) -{ - return clear_blk_nodup(ip, block); -} +#include "eattr.h" int clear_eattr_entry (struct gfs2_inode *ip, struct gfs2_buffer_head *leaf_bh, @@ -82,8 +54,7 @@ int clear_eattr_extentry(struct gfs2_inode *ip, uint64_t *ea_data_ptr, { uint64_t block = be64_to_cpu(*ea_data_ptr); - return clear_blk_nodup(ip, block); - + return delete_eattr_leaf(ip, block, 0, &leaf_bh, private); } diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 609aa4b..4a6651f 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -908,6 +908,63 @@ static int check_leaf_eattr(struct gfs2_inode *ip, uint64_t block, } /** + * delete_block - delete a block associated with an inode + */ +int delete_block(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head **bh, const char *btype, + void *private) +{ + if (gfs2_check_range(ip->i_sbd, block) == 0) { + fsck_blockmap_set(ip, block, btype, gfs2_block_free); + return 0; + } + return -1; +} + +/** + * delete_block_if_notdup - delete blocks associated with an inode + * + * Ignore blocks that are already marked free. + * If it has been identified as duplicate, remove the duplicate reference. + * If all duplicate references have been removed, delete the block. + */ +static int delete_block_if_notdup(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head **bh, + const char *btype, void *private) +{ + uint8_t q; + struct duptree *d; + + if (gfs2_check_range(ip->i_sbd, block) != 0) + return -EFAULT; + + q = block_type(block); + if (q == gfs2_block_free) { + log_info( _("%s block %lld (0x%llx), part of inode " + "%lld (0x%llx), was already free.\n"), + btype, (unsigned long long)block, + (unsigned long long)block, + (unsigned long long)ip->i_di.di_num.no_addr, + (unsigned long long)ip->i_di.di_num.no_addr); + return 0; + } + d = dupfind(block); + if (d) { + log_info( _("Removing duplicate reference %d " + "to block %lld (0x%llx).\n"), d->refs, + (unsigned long long)block, + (unsigned long long)block); + d->refs--; /* one less reference */ + if (d->refs == 1) /* If down to the last reference */ + dup_delete(d); /* not duplicate now */ + return 1; /* but the original ref still exists + so return (do not free it). */ + } + fsck_blockmap_set(ip, block, btype, gfs2_block_free); + return 0; +} + +/** * check_indirect_eattr * @ip: the inode the eattr comes from * @indirect_block @@ -989,11 +1046,9 @@ static int check_indirect_eattr(struct gfs2_inode *ip, uint64_t indirect, leaf_pointer_errors, pass->private); } - if (leaf_pointer_errors == leaf_pointers) { - fsck_blockmap_set(ip, indirect, - _("indirect extended " - "attribute"), - gfs2_block_free); + if (leaf_pointer_errors && + leaf_pointer_errors == leaf_pointers) { + delete_block(ip, indirect, NULL, "leaf", NULL); error = 1; } } @@ -1389,51 +1444,36 @@ int remove_dentry_from_dir(struct gfs2_sbd *sbp, uint64_t dir, return error; } -/** - * delete_blocks - delete blocks associated with an inode - */ -int delete_blocks(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, const char *btype, - void *private) +int delete_metadata(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head **bh, void *private) { - if (gfs2_check_range(ip->i_sbd, block) == 0) { - if(!is_duplicate(block)) { - log_info( _("Deleting %s block %lld (0x%llx) as part " - "of inode %lld (0x%llx)\n"), btype, - (unsigned long long)block, - (unsigned long long)block, - (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)ip->i_di.di_num.no_addr); - fsck_blockmap_set(ip, block, btype, gfs2_block_free); - gfs2_free_block(ip->i_sbd, block); - } - } - return 0; + return delete_block_if_notdup(ip, block, bh, _("metadata"), private); } -int delete_metadata(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, void *private) +int delete_leaf(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head *bh, void *private) { - return delete_blocks(ip, block, bh, _("metadata"), private); + return delete_block_if_notdup(ip, block, &bh, _("leaf"), private); } int delete_data(struct gfs2_inode *ip, uint64_t block, void *private) { - return delete_blocks(ip, block, NULL, _("data"), private); + return delete_block_if_notdup(ip, block, NULL, _("data"), private); } int delete_eattr_indir(struct gfs2_inode *ip, uint64_t block, uint64_t parent, struct gfs2_buffer_head **bh, void *private) { - return delete_blocks(ip, block, NULL, _("indirect extended attribute"), - private); + return delete_block_if_notdup(ip, block, NULL, + _("indirect extended attribute"), + private); } int delete_eattr_leaf(struct gfs2_inode *ip, uint64_t block, uint64_t parent, struct gfs2_buffer_head **bh, void *private) { - return delete_blocks(ip, block, NULL, _("extended attribute"), - private); + return delete_block_if_notdup(ip, block, NULL, _("extended attribute"), + private); } static int alloc_metalist(struct gfs2_inode *ip, uint64_t block, diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h index 653b512..9eb2372 100644 --- a/gfs2/fsck/metawalk.h +++ b/gfs2/fsck/metawalk.h @@ -14,19 +14,19 @@ extern int check_dir(struct gfs2_sbd *sbp, uint64_t block, extern int check_linear_dir(struct gfs2_inode *ip, struct gfs2_buffer_head *bh, struct metawalk_fxns *pass); extern int remove_dentry_from_dir(struct gfs2_sbd *sbp, uint64_t dir, - uint64_t dentryblock); -extern int delete_blocks(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, const char *btype, - void *private); + uint64_t dentryblock); +extern int delete_block(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head **bh, const char *btype, + void *private); extern int delete_metadata(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, void *private); + struct gfs2_buffer_head **bh, void *private); +extern int delete_leaf(struct gfs2_inode *ip, uint64_t block, + struct gfs2_buffer_head *bh, void *private); extern int delete_data(struct gfs2_inode *ip, uint64_t block, void *private); -extern int delete_eattr_indir(struct gfs2_inode *ip, uint64_t block, - uint64_t parent, struct gfs2_buffer_head **bh, - void *private); -extern int delete_eattr_leaf(struct gfs2_inode *ip, uint64_t block, - uint64_t parent, struct gfs2_buffer_head **bh, - void *private); +extern int delete_eattr_indir(struct gfs2_inode *ip, uint64_t block, uint64_t parent, + struct gfs2_buffer_head **bh, void *private); +extern int delete_eattr_leaf(struct gfs2_inode *ip, uint64_t block, uint64_t parent, + struct gfs2_buffer_head **bh, void *private); extern int _fsck_blockmap_set(struct gfs2_inode *ip, uint64_t bblock, const char *btype, enum gfs2_mark_block mark, const char *caller, int line); diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 46aaefa..2f6782a 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -254,10 +254,8 @@ static int clear_eas(struct gfs2_inode *ip, struct block_count *bc, log_err( _("The bad extended attribute was " "removed.\n")); } else if (!duplicate) { - fsck_blockmap_set(ip, block, _("removed eattr"), - gfs2_block_free); - log_err( _("The bad Extended Attribute was " - "removed.\n")); + delete_block(ip, block, NULL, + _("bad extended attribute"), NULL); } return 1; } else { diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index ee29893..08dd5a4 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -154,6 +154,7 @@ struct metawalk_fxns pass2_fxns_delete = { .private = NULL, .check_metalist = delete_metadata, .check_data = delete_data, + .check_leaf = delete_leaf, .check_eattr_indir = delete_eattr_indir, .check_eattr_leaf = delete_eattr_leaf, }; @@ -180,8 +181,8 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, gfs2_dirent_in(&dentry, (char *)dent); de = &dentry; - clear_eattrs.check_eattr_indir = clear_eattr_indir; - clear_eattrs.check_eattr_leaf = clear_eattr_leaf; + clear_eattrs.check_eattr_indir = delete_eattr_indir; + clear_eattrs.check_eattr_leaf = delete_eattr_leaf; clear_eattrs.check_eattr_entry = clear_eattr_entry; clear_eattrs.check_eattr_extentry = clear_eattr_extentry;

1 0

cluster: STABLE3 - fsck.gfs2: rename gfs2_meta_other to gfs2_meta_rgrp.
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=b6… Commit: b6cd280f35778a1fa69426df5d5bdbb87e6f0425 Parent: 8e4f539322c240851e5bcf906e54905be9e0a924 Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Mon Jan 25 10:36:51 2010 -0600 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Jan 26 14:39:31 2010 -0600 fsck.gfs2: rename gfs2_meta_other to gfs2_meta_rgrp. This patch renames a blockmap block designation from gfs2_meta_other to gfs2_meta_rgrp. This is just a cleanup but makes the code easier to read, since the designation was only used for rgrps. rhbz#455300 --- gfs2/fsck/pass1.c | 9 ++++++++- gfs2/fsck/pass5.c | 2 +- gfs2/libgfs2/libgfs2.h | 6 +++--- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index b7ee6d5..46aaefa 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -908,11 +908,18 @@ int pass1(struct gfs2_sbd *sbp) rg_count); rgd = osi_list_entry(tmp, struct rgrp_list, list); for (i = 0; i < rgd->ri.ri_length; i++) { + log_debug( _("rgrp block %lld (0x%llx) " + "is now marked as 'rgrp data'\n"), + rgd->ri.ri_addr + i, rgd->ri.ri_addr + i); if(gfs2_blockmap_set(bl, rgd->ri.ri_addr + i, - gfs2_meta_other)){ + gfs2_meta_rgrp)){ stack; return FSCK_ERROR; } + /* rgrps and bitmaps don't have bits to represent + their blocks, so don't do this: + check_n_fix_bitmap(sbp, rgd->ri.ri_addr + i, + gfs2_meta_rgrp);*/ } offset = sizeof(struct gfs2_rgrp); diff --git a/gfs2/fsck/pass5.c b/gfs2/fsck/pass5.c index 07b0559..645d81c 100644 --- a/gfs2/fsck/pass5.c +++ b/gfs2/fsck/pass5.c @@ -37,7 +37,7 @@ static int convert_mark(uint8_t q, uint32_t *count) case gfs2_indir_blk: case gfs2_leaf_blk: - case gfs2_meta_other: + case gfs2_meta_rgrp: case gfs2_meta_eattr: count[2]++; return GFS2_BLKST_USED; diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h index 6d1a89f..0e7e1b4 100644 --- a/gfs2/libgfs2/libgfs2.h +++ b/gfs2/libgfs2/libgfs2.h @@ -280,7 +280,7 @@ enum gfs2_mark_block { gfs2_inode_invalid = (0xa), gfs2_meta_inval = (0xb), gfs2_leaf_blk = (0xc), - gfs2_meta_other = (0xd), + gfs2_meta_rgrp = (0xd), gfs2_meta_eattr = (0xe), gfs2_bad_block = (0xf), /* Contains at least one bad block */ @@ -301,10 +301,10 @@ static const inline char *block_type_string(uint8_t q) "fifo", "socket", - "journaled data", + "invalid inode", "invalid meta", "dir leaf", - "other metadata", + "rgrp meta", "eattribute", "bad"};

1 0

cluster: STABLE3 - Eliminate unnecessary block_list from gfs2_edit
by Bob Peterson 26 Jan '10

26 Jan '10

Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=8e… Commit: 8e4f539322c240851e5bcf906e54905be9e0a924 Parent: 7bd37e30f2dd4a54283ac57f0df003840fe010d7 Author: Bob Peterson <bob(a)ganesha.peterson> AuthorDate: Mon Jan 25 16:23:19 2010 -0600 Committer: Bob Peterson <rpeterso(a)redhat.com> CommitterDate: Tue Jan 26 14:39:31 2010 -0600 Eliminate unnecessary block_list from gfs2_edit There is no need for a blockmap in gfs2_edit's savemeta function so this patch eliminates it. rhbz#455300 --- gfs2/edit/savemeta.c | 18 +----------------- 1 files changed, 1 insertions(+), 17 deletions(-) diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c index 3e7ec7e..6df3a4b 100644 --- a/gfs2/edit/savemeta.c +++ b/gfs2/edit/savemeta.c @@ -35,7 +35,6 @@ struct saved_metablock { struct saved_metablock *savedata; struct gfs2_buffer_head *savebh; uint64_t last_fs_block, last_reported_block, blks_saved, total_out, pct; -struct gfs2_bmap *blocklist = NULL; uint64_t journal_blocks[MAX_JOURNALS_SAVED]; uint64_t gfs1_journal_size = 0; /* in blocks */ int journals_found = 0; @@ -499,7 +498,6 @@ void savemeta(char *out_fn, int saveoption) int out_fd; int slow; osi_list_t *tmp; - uint64_t memreq; int rgcount; uint64_t jindex_block; struct gfs2_buffer_head *lbh; @@ -599,12 +597,6 @@ void savemeta(char *out_fn, int saveoption) printf("Done.\n\n"); fflush(stdout); } - if (!slow) { - blocklist = gfs2_bmap_create(&sbd, last_fs_block + 1, - &memreq); - if (!blocklist) - slow = TRUE; - } get_journal_inode_blocks(); if (!slow) { /* Save off the superblock */ @@ -631,7 +623,7 @@ void savemeta(char *out_fn, int saveoption) for (tmp = sbd.rglist.next; tmp != &sbd.rglist; tmp = tmp->next){ struct rgrp_list *rgd; - int i, first; + int first; rgd = osi_list_entry(tmp, struct rgrp_list, list); slow = gfs2_rgrp_read(&sbd, rgd); @@ -641,12 +633,6 @@ void savemeta(char *out_fn, int saveoption) (unsigned long long)rgd->ri.ri_addr, (unsigned long long)rgd->ri.ri_addr, rgd->ri.ri_length); - for (i = 0; i < rgd->ri.ri_length; i++) { - if(gfs2_blockmap_set(blocklist, - rgd->ri.ri_addr + i, - gfs2_meta_other)) - break; - } first = 1; /* Save off the rg and bitmaps */ for (block = rgd->ri.ri_addr; @@ -685,8 +671,6 @@ void savemeta(char *out_fn, int saveoption) } } /* Clean up */ - if (blocklist) - gfs2_bmap_destroy(&sbd, blocklist); /* There may be a gap between end of file system and end of device */ /* so we tell the user that we've processed everything. */ block = last_fs_block;

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

cluster-commits January 2010