This is an automated email from the git hooks/post-receive script. andyp pushed a commit to branch RHEL7 in repository gfs2-utils. commit 5cf083a5c231bfe0950a276aff9249dd63f708d3 Author: Andrew Price <anprice(a)redhat.com&gt; AuthorDate: Fri Oct 18 19:16:08 2019 +0100 fsck.gfs2: Retain context for indirect pointers in ->check_metalist When the pass->check_metalist() functions are called, the pointer is thrown away and the function is just called with a block address, meaning that ->check_metalist() is unable to fix the pointer itself if it deems the block pointed to is garbage. Add an iptr structure which collects together the inode, the metadata block buffer and the offset of the pointer in that buffer. The iptr is now passed to ->check_metadata instead of the separate inode and block address arguments. Resolves: rhbz#1487726 Signed-off-by: Andrew Price <anprice(a)redhat.com&gt; --- gfs2/fsck/afterpass1_common.c | 6 +++-- gfs2/fsck/afterpass1_common.h | 4 +-- gfs2/fsck/fs_recovery.c | 8 +++--- gfs2/fsck/metawalk.c | 55 ++++++++++++++++++++-------------------- gfs2/fsck/metawalk.h | 15 +++++++++-- gfs2/fsck/pass1.c | 59 ++++++++++++++++++++++--------------------- gfs2/fsck/pass1b.c | 22 ++++++++-------- gfs2/fsck/pass2.c | 8 +++--- 8 files changed, 98 insertions(+), 79 deletions(-) diff --git a/gfs2/fsck/afterpass1_common.c b/gfs2/fsck/afterpass1_common.c index b747640..55a6c76 100644 --- a/gfs2/fsck/afterpass1_common.c +++ b/gfs2/fsck/afterpass1_common.c @@ -175,10 +175,12 @@ int remove_dentry_from_dir(struct gfs2_sbd *sdp, uint64_t dir, return error; } -int delete_metadata(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, int *is_valid, +int delete_metadata(struct iptr iptr, struct gfs2_buffer_head **bh, int h, int *is_valid, int *was_duplicate, void *private) { + struct gfs2_inode *ip = iptr.ipt_ip; + uint64_t block = iptr_block(iptr); + *is_valid = 1; *was_duplicate = 0; return delete_block_if_notdup(ip, block, bh, _("metadata"), diff --git a/gfs2/fsck/afterpass1_common.h b/gfs2/fsck/afterpass1_common.h index 829828f..74b913f 100644 --- a/gfs2/fsck/afterpass1_common.h +++ b/gfs2/fsck/afterpass1_common.h @@ -2,9 +2,9 @@ #define _AFTERPASS1_H #include "util.h" +#include "metawalk.h" -extern int delete_metadata(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, int *is_valid, +extern int delete_metadata(struct iptr iptr, struct gfs2_buffer_head **bh, int h, int *is_valid, int *was_duplicate, void *private); extern int delete_leaf(struct gfs2_inode *ip, uint64_t block, void *private); extern int delete_data(struct gfs2_inode *ip, uint64_t metablock, diff --git a/gfs2/fsck/fs_recovery.c b/gfs2/fsck/fs_recovery.c index 3acbb5d..614c01a 100644 --- a/gfs2/fsck/fs_recovery.c +++ b/gfs2/fsck/fs_recovery.c @@ -636,11 +636,11 @@ static int rangecheck_jblock(struct gfs2_inode *ip, uint64_t block) return meta_is_good; } -static int rangecheck_jmeta(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, - int *is_valid, int *was_duplicate, - void *private) +static int rangecheck_jmeta(struct iptr iptr, struct gfs2_buffer_head **bh, int h, + int *is_valid, int *was_duplicate, void *private) { + struct gfs2_inode *ip = iptr.ipt_ip; + uint64_t block = iptr_block(iptr); int rc; *bh = NULL; diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index dbe9f1f..d91ed63 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -1207,9 +1207,11 @@ static void file_ra(struct gfs2_inode *ip, struct gfs2_buffer_head *bh, extlen * sdp->bsize, POSIX_FADV_WILLNEED); } -static int do_check_metalist(struct gfs2_inode *ip, uint64_t block, int height, - struct gfs2_buffer_head **bhp, struct metawalk_fxns *pass) +static int do_check_metalist(struct iptr iptr, int height, struct gfs2_buffer_head **bhp, + struct metawalk_fxns *pass) { + struct gfs2_inode *ip = iptr.ipt_ip; + uint64_t block = iptr_block(iptr); int was_duplicate = 0; int is_valid = 1; int error; @@ -1217,7 +1219,7 @@ static int do_check_metalist(struct gfs2_inode *ip, uint64_t block, int height, if (pass->check_metalist == NULL) return 0; - error = pass->check_metalist(ip, block, bhp, height, &is_valid, + error = pass->check_metalist(iptr, bhp, height, &is_valid, &was_duplicate, pass->private); if (error == meta_error) { stack; @@ -1267,10 +1269,11 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, struct metawalk_fxns *pass) { uint32_t height = ip->i_di.di_height; - struct gfs2_buffer_head *bh, *nbh, *metabh = ip->i_bh; + struct gfs2_buffer_head *metabh = ip->i_bh; osi_list_t *prev_list, *cur_list, *tmp; + struct iptr iptr = { .ipt_ip = ip, 0}; int h, head_size, iblk_type; - uint64_t *ptr, block, *undoptr; + uint64_t *undoptr; int maxptrs; int error; @@ -1310,39 +1313,37 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, prev_list = &mlp[h - 1]; cur_list = &mlp[h]; - for (tmp = prev_list->next; tmp != prev_list; tmp = tmp->next){ - bh = osi_list_entry(tmp, struct gfs2_buffer_head, - b_altlist); - if (gfs2_check_meta(bh->b_data, iblk_type)) { + for (tmp = prev_list->next; tmp != prev_list; tmp = tmp->next) { + iptr.ipt_off = head_size; + iptr.ipt_bh = osi_list_entry(tmp, struct gfs2_buffer_head, b_altlist); + + if (gfs2_check_meta(iptr_buf(iptr), iblk_type)) { if (pass->invalid_meta_is_fatal) return meta_error; continue; } - if (pass->readahead) - file_ra(ip, bh, head_size, maxptrs, h); + file_ra(ip, iptr.ipt_bh, head_size, maxptrs, h); + /* Now check the metadata itself */ - for (ptr = (uint64_t *)(bh->b_data + head_size); - (char *)ptr < (bh->b_data + ip->i_sbd->bsize); - ptr++) { + for (; iptr.ipt_off < ip->i_sbd->bsize; iptr.ipt_off += sizeof(uint64_t)) { + struct gfs2_buffer_head *nbh = NULL; + if (skip_this_pass || fsck_abort) { free_metalist(ip, mlp); return meta_is_good; } - nbh = NULL; - - if (!*ptr) + if (!iptr_block(iptr)) continue; - block = be64_to_cpu(*ptr); - error = do_check_metalist(ip, block, h, &nbh, pass); + error = do_check_metalist(iptr, h, &nbh, pass); if (error == meta_error || error == meta_skip_further) goto error_undo; if (error == meta_skip_one) continue; if (!nbh) - nbh = bread(ip->i_sbd, block); + nbh = bread(ip->i_sbd, iptr_block(iptr)); osi_list_add_prev(&nbh->b_altlist, cur_list); } /* for all data on the indirect block */ } /* for blocks at that height */ @@ -1353,16 +1354,16 @@ error_undo: /* undo what we've done so far for this block */ if (pass->undo_check_meta == NULL) return error; - log_info(_("Undoing the work we did before the error on block %llu " - "(0x%llx).\n"), (unsigned long long)bh->b_blocknr, - (unsigned long long)bh->b_blocknr); - for (undoptr = (uint64_t *)(bh->b_data + head_size); undoptr < ptr && - (char *)undoptr < (bh->b_data + ip->i_sbd->bsize); + log_info(_("Undoing the work we did before the error on block %"PRIu64" (0x%"PRIx64").\n"), + iptr.ipt_bh->b_blocknr, iptr.ipt_bh->b_blocknr); + for (undoptr = (uint64_t *)(iptr_buf(iptr) + head_size); + undoptr < iptr_ptr(iptr) && undoptr < iptr_endptr(iptr); undoptr++) { - if (!*undoptr) + uint64_t block = be64_to_cpu(*undoptr); + + if (block == 0) continue; - block = be64_to_cpu(*undoptr); pass->undo_check_meta(ip, block, h, pass->private); } return error; diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h index b5a037a..592479d 100644 --- a/gfs2/fsck/metawalk.h +++ b/gfs2/fsck/metawalk.h @@ -42,6 +42,17 @@ enum meta_check_rc { meta_skip_one = 2, }; +struct iptr { + struct gfs2_inode *ipt_ip; + struct gfs2_buffer_head *ipt_bh; + unsigned ipt_off; +}; + +#define iptr_ptr(i) ((uint64_t *)(i.ipt_bh->b_data + i.ipt_off)) +#define iptr_block(i) be64_to_cpu(*iptr_ptr(i)) +#define iptr_endptr(i) ((uint64_t *)(iptr.ipt_bh->b_data + i.ipt_ip->i_sbd->bsize)) +#define iptr_buf(i) (i.ipt_bh->b_data) + /* metawalk_fxns: function pointers to check various parts of the fs * * The functions should return -1 on fatal errors, 1 if the block @@ -66,7 +77,7 @@ struct metawalk_fxns { int (*check_leaf) (struct gfs2_inode *ip, uint64_t block, void *private); /* parameters to the check_metalist sub-functions: - ip: incore inode pointer + iptr: reference to the inode and its indirect pointer that we're analyzing block: block number of the metadata block to be checked bh: buffer_head to be returned h: height @@ -79,7 +90,7 @@ struct metawalk_fxns { returns: 0 - everything is good, but there may be duplicates 1 - skip further processing */ - int (*check_metalist) (struct gfs2_inode *ip, uint64_t block, + int (*check_metalist) (struct iptr iptr, struct gfs2_buffer_head **bh, int h, int *is_valid, int *was_duplicate, void *private); diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index f03078e..66cf6dc 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -39,9 +39,8 @@ struct block_count { }; static int p1check_leaf(struct gfs2_inode *ip, uint64_t block, void *private); -static int pass1_check_metalist(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, int *is_valid, - int *was_duplicate, void *private); +static int pass1_check_metalist(struct iptr iptr, struct gfs2_buffer_head **bh, int h, + int *is_valid, int *was_duplicate, void *private); static int undo_check_metalist(struct gfs2_inode *ip, uint64_t block, int h, void *private); static int pass1_check_data(struct gfs2_inode *ip, uint64_t metablock, @@ -69,10 +68,8 @@ static int check_extended_leaf_eattr(struct gfs2_inode *ip, int i, 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, int h, - int *is_valid, int *was_duplicate, - void *private); +static int invalidate_metadata(struct iptr iptr, struct gfs2_buffer_head **bh, int h, + int *is_valid, int *was_duplicate, void *private); static int invalidate_leaf(struct gfs2_inode *ip, uint64_t block, void *private); static int invalidate_data(struct gfs2_inode *ip, uint64_t metablock, @@ -223,12 +220,12 @@ struct metawalk_fxns invalidate_fxns = { * marked "in use" by the bitmap. You don't want root's indirect blocks * deleted, do you? Or worse, reused for lost+found. */ -static int resuscitate_metalist(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, - int *is_valid, int *was_duplicate, - void *private) +static int resuscitate_metalist(struct iptr iptr, struct gfs2_buffer_head **bh, int h, + int *is_valid, int *was_duplicate, void *private) { struct block_count *bc = (struct block_count *)private; + struct gfs2_inode *ip = iptr.ipt_ip; + uint64_t block = iptr_block(iptr); *is_valid = 1; *was_duplicate = 0; @@ -344,15 +341,16 @@ static int p1check_leaf(struct gfs2_inode *ip, uint64_t block, void *private) return 0; } -static int pass1_check_metalist(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, int *is_valid, - int *was_duplicate, void *private) +static int pass1_check_metalist(struct iptr iptr, struct gfs2_buffer_head **bh, int h, + int *is_valid, int *was_duplicate, void *private) { - int q; - int iblk_type; - struct gfs2_buffer_head *nbh; struct block_count *bc = (struct block_count *)private; + struct gfs2_inode *ip = iptr.ipt_ip; + uint64_t block = iptr_block(iptr); + struct gfs2_buffer_head *nbh; const char *blktypedesc; + int iblk_type; + int q; *bh = NULL; @@ -1111,11 +1109,12 @@ static int mark_block_invalid(struct gfs2_inode *ip, uint64_t block, return meta_is_good; } -static int invalidate_metadata(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, - int *is_valid, int *was_duplicate, - void *private) +static int invalidate_metadata(struct iptr iptr, struct gfs2_buffer_head **bh, int h, + int *is_valid, int *was_duplicate, void *private) { + struct gfs2_inode *ip = iptr.ipt_ip; + uint64_t block = iptr_block(iptr); + *is_valid = 1; *was_duplicate = 0; return mark_block_invalid(ip, block, ref_as_meta, _("metadata"), @@ -1214,11 +1213,12 @@ static int rangecheck_block(struct gfs2_inode *ip, uint64_t block, return meta_is_good; } -static int rangecheck_metadata(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, - int *is_valid, int *was_duplicate, - void *private) +static int rangecheck_metadata(struct iptr iptr, struct gfs2_buffer_head **bh, int h, + int *is_valid, int *was_duplicate, void *private) { + struct gfs2_inode *ip = iptr.ipt_ip; + uint64_t block = iptr_block(iptr); + *is_valid = 1; *was_duplicate = 0; return rangecheck_block(ip, block, bh, btype_meta, private); @@ -1317,12 +1317,13 @@ static int set_ip_blockmap(struct gfs2_inode *ip) return 0; } -static int alloc_metalist(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, int *is_valid, - int *was_duplicate, void *private) +static int alloc_metalist(struct iptr iptr, struct gfs2_buffer_head **bh, int h, + int *is_valid, int *was_duplicate, void *private) { - int q; const char *desc = (const char *)private; + struct gfs2_inode *ip = iptr.ipt_ip; + uint64_t block = iptr_block(iptr); + int q; /* No need to range_check here--if it was added, it's in range. */ /* We can't check the bitmap here because this function is called diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c index 62686fe..350902b 100644 --- a/gfs2/fsck/pass1b.c +++ b/gfs2/fsck/pass1b.c @@ -69,9 +69,8 @@ static void log_inode_reference(struct duptree *dt, osi_list_t *tmp, int inval) (unsigned long long)dt->block, reftypestring); } -static int findref_meta(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, - int *is_valid, int *was_duplicate, void *private) +static int findref_meta(struct iptr iptr, struct gfs2_buffer_head **bh, int h, + int *is_valid, int *was_duplicate, void *private) { *is_valid = 1; *was_duplicate = 0; @@ -393,10 +392,12 @@ static void resolve_dup_references(struct gfs2_sbd *sdp, struct duptree *dt, return; } -static int clone_check_meta(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, - int *is_valid, int *was_duplicate, void *private) +static int clone_check_meta(struct iptr iptr, struct gfs2_buffer_head **bh, int h, + int *is_valid, int *was_duplicate, void *private) { + struct gfs2_inode *ip = iptr.ipt_ip; + uint64_t block = iptr_block(iptr); + *was_duplicate = 0; *is_valid = 1; *bh = bread(ip->i_sbd, block); @@ -788,11 +789,12 @@ static int check_leaf_refs(struct gfs2_inode *ip, uint64_t block, return add_duplicate_ref(ip, block, ref_as_meta, 1, INODE_VALID); } -static int check_metalist_refs(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, - int *is_valid, int *was_duplicate, - void *private) +static int check_metalist_refs(struct iptr iptr, struct gfs2_buffer_head **bh, int h, + int *is_valid, int *was_duplicate, void *private) { + struct gfs2_inode *ip = iptr.ipt_ip; + uint64_t block = iptr_block(iptr); + *was_duplicate = 0; *is_valid = 1; return add_duplicate_ref(ip, block, ref_as_meta, 1, INODE_VALID); diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index d10b908..d80f306 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -1873,10 +1873,12 @@ struct metawalk_fxns pass2_fxns = { .repair_leaf = pass2_repair_leaf, }; -static int check_metalist_qc(struct gfs2_inode *ip, uint64_t block, - struct gfs2_buffer_head **bh, int h, - int *is_valid, int *was_duplicate, void *private) +static int check_metalist_qc(struct iptr iptr, struct gfs2_buffer_head **bh, int h, + int *is_valid, int *was_duplicate, void *private) { + struct gfs2_inode *ip = iptr.ipt_ip; + uint64_t block = iptr_block(iptr); + *was_duplicate = 0; *is_valid = 1; *bh = bread(ip->i_sbd, block); -- To stop receiving notification emails like this one, please contact the administrator of this repository.