From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Whitehouse Date: Fri, 12 Aug 2011 10:41:13 +0100 Subject: [Cluster-devel] [Patch 19/44] fsck.gfs2: Move function check_num_ptrs from metawalk.c to pass1.c In-Reply-To: <1709886589.544738.1313096797201.JavaMail.root@zmail06.collab.prod.int.phx2.redhat.com> References: <1709886589.544738.1313096797201.JavaMail.root@zmail06.collab.prod.int.phx2.redhat.com> Message-ID: <1313142073.2704.37.camel@menhir> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Hi, This looks a bit confusing. The number of pointers in a leaf block is, in general, not a power of two. I assume that this refers to the dir hash table and maybe needs renaming accordingly? Steve. On Thu, 2011-08-11 at 17:06 -0400, Bob Peterson wrote: > >From 4768302284585037fada762a1bc5107f9212de71 Mon Sep 17 00:00:00 2001 > From: Bob Peterson > Date: Tue, 9 Aug 2011 09:23:05 -0500 > Subject: [PATCH 19/44] fsck.gfs2: Move function check_num_ptrs from > metawalk.c to pass1.c > > This patch moves function check_num_ptrs from metawalk.c to pass1.c > and also its helper function fix_leaf_pointers. This function checks > that the number of pointers in a leaf block is a factor of 2, and sane. > This should only happen once, when leaf blocks are initially checked. > Previously it was wasting a lot of time doing this check from every pass. > > rhbz#675723 > --- > gfs2/fsck/metawalk.c | 129 +++--------------------------------------------- > gfs2/fsck/metawalk.h | 3 + > gfs2/fsck/pass1.c | 134 ++++++++++++++++++++++++++++++++++++++++++++++++- > 3 files changed, 142 insertions(+), 124 deletions(-) > > diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c > index 9abec79..622f9d7 100644 > --- a/gfs2/fsck/metawalk.c > +++ b/gfs2/fsck/metawalk.c > @@ -463,123 +463,6 @@ static void warn_and_patch(struct gfs2_inode *ip, uint64_t *leaf_no, > *leaf_no = old_leaf; > } > > -/* > - * fix_leaf_pointers - fix a directory dinode that has a number of pointers > - * that is not a multiple of 2. > - * dip - the directory inode having the problem > - * lindex - the index of the leaf right after the problem (need to back up) > - * cur_numleafs - current (incorrect) number of instances of the leaf block > - * correct_numleafs - the correct number instances of the leaf block > - */ > -static int fix_leaf_pointers(struct gfs2_inode *dip, int *lindex, > - int cur_numleafs, int correct_numleafs) > -{ > - int count; > - char *ptrbuf; > - int start_lindex = *lindex - cur_numleafs; /* start of bad ptrs */ > - int tot_num_ptrs = (1 << dip->i_di.di_depth) - start_lindex; > - int bufsize = tot_num_ptrs * sizeof(uint64_t); > - int off_by = cur_numleafs - correct_numleafs; > - > - ptrbuf = malloc(bufsize); > - if (!ptrbuf) { > - log_err( _("Error: Cannot allocate memory to fix the leaf " > - "pointers.\n")); > - return -1; > - } > - /* Read all the pointers, starting with the first bad one */ > - count = gfs2_readi(dip, ptrbuf, start_lindex * sizeof(uint64_t), > - bufsize); > - if (count != bufsize) { > - log_err( _("Error: bad read while fixing leaf pointers.\n")); > - free(ptrbuf); > - return -1; > - } > - > - bufsize -= off_by * sizeof(uint64_t); /* We need to write fewer */ > - /* Write the same pointers, but offset them so they fit within the > - smaller factor of 2. So if we have 12 pointers, write out only > - the last 8 of them. If we have 7, write the last 4, etc. > - We need to write these starting at the current lindex and adjust > - lindex accordingly. */ > - count = gfs2_writei(dip, ptrbuf + (off_by * sizeof(uint64_t)), > - start_lindex * sizeof(uint64_t), bufsize); > - if (count != bufsize) { > - log_err( _("Error: bad read while fixing leaf pointers.\n")); > - free(ptrbuf); > - return -1; > - } > - /* Now zero out the hole left at the end */ > - memset(ptrbuf, 0, off_by * sizeof(uint64_t)); > - gfs2_writei(dip, ptrbuf, (start_lindex * sizeof(uint64_t)) + > - bufsize, off_by * sizeof(uint64_t)); > - free(ptrbuf); > - *lindex -= off_by; /* adjust leaf index to account for the change */ > - return 0; > -} > - > -/* check_num_ptrs - check a previously processed leaf's pointer count */ > -static int check_num_ptrs(struct gfs2_inode *ip, uint64_t old_leaf, > - int *ref_count, int *exp_count, int *lindex, > - struct gfs2_leaf *oldleaf) > -{ > - int factor = 0, divisor = *ref_count, multiple = 1, error = 0; > - struct gfs2_buffer_head *lbh; > - > - /* Check to see if the number of pointers we found is a power of 2. > - It needs to be and if it's not we need to fix it.*/ > - while (divisor > 1) { > - factor++; > - divisor /= 2; > - multiple = multiple << 1; > - } > - if (*ref_count != multiple) { > - log_err( _("Directory #%llu (0x%llx) has an " > - "invalid number of pointers to " > - "leaf #%llu (0x%llx)\n\tFound: %u, " > - "which is not a factor of 2.\n"), > - (unsigned long long)ip->i_di.di_num.no_addr, > - (unsigned long long)ip->i_di.di_num.no_addr, > - (unsigned long long)old_leaf, > - (unsigned long long)old_leaf, *ref_count); > - if (!query( _("Attempt to fix it? (y/n) "))) { > - log_err( _("Directory inode was not fixed.\n")); > - return 1; > - } > - error = fix_leaf_pointers(ip, lindex, *ref_count, multiple); > - if (error) > - return error; > - *ref_count = multiple; > - log_err( _("Directory inode was fixed.\n")); > - } > - /* Check to see if the counted number of leaf pointers is what we > - expect. */ > - if (*ref_count != *exp_count) { > - log_err( _("Directory #%llu (0x%llx) has an " > - "incorrect number of pointers to " > - "leaf #%llu (0x%llx)\n\tFound: " > - "%u, Expected: %u\n"), > - (unsigned long long)ip->i_di.di_num.no_addr, > - (unsigned long long)ip->i_di.di_num.no_addr, > - (unsigned long long)old_leaf, > - (unsigned long long)old_leaf, *ref_count, *exp_count); > - if (!query( _("Attempt to fix it? (y/n) "))) { > - log_err( _("Directory leaf was not fixed.\n")); > - return 1; > - } > - lbh = bread(ip->i_sbd, old_leaf); > - gfs2_leaf_in(oldleaf, lbh); > - log_err( _("Leaf depth was %d, changed to %d\n"), > - oldleaf->lf_depth, ip->i_di.di_depth - factor); > - oldleaf->lf_depth = ip->i_di.di_depth - factor; > - gfs2_leaf_out(oldleaf, lbh); > - brelse(lbh); > - *exp_count = *ref_count; > - log_err( _("Directory leaf was fixed.\n")); > - } > - return 0; > -} > - > /* Checks exhash directory entries */ > static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass) > { > @@ -638,10 +521,14 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass) > } > > do { > - if (!valid_block(ip->i_sbd, old_leaf) == 0) { > - error = check_num_ptrs(ip, old_leaf, > - &ref_count, &exp_count, > - &lindex, &oldleaf); > + if (fsck_abort) > + return 0; > + if (pass->check_num_ptrs && > + valid_block(ip->i_sbd, old_leaf)) { > + error = pass->check_num_ptrs(ip, old_leaf, > + &ref_count, > + &lindex, > + &oldleaf); > if (error) > return error; > } > diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h > index d705726..7a8ae4c 100644 > --- a/gfs2/fsck/metawalk.h > +++ b/gfs2/fsck/metawalk.h > @@ -89,6 +89,9 @@ struct metawalk_fxns { > int (*finish_eattr_indir) (struct gfs2_inode *ip, int leaf_pointers, > int leaf_pointer_errors, void *private); > void (*big_file_msg) (struct gfs2_inode *ip, uint64_t blks_checked); > + int (*check_num_ptrs) (struct gfs2_inode *ip, uint64_t leafno, > + int *ref_count, int *lindex, > + struct gfs2_leaf *leaf); > }; > > #endif /* _METAWALK_H */ > diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c > index 2b04227..8e6bec3 100644 > --- a/gfs2/fsck/pass1.c > +++ b/gfs2/fsck/pass1.c > @@ -34,7 +34,7 @@ struct block_count { > uint64_t ea_count; > }; > > -static int leaf(struct gfs2_inode *ip, uint64_t block, void *private); > +static int check_leaf(struct gfs2_inode *ip, uint64_t block, void *private); > static int check_metalist(struct gfs2_inode *ip, uint64_t block, > struct gfs2_buffer_head **bh, int h, void *private); > static int undo_check_metalist(struct gfs2_inode *ip, uint64_t block, > @@ -59,6 +59,8 @@ static int check_extended_leaf_eattr(struct gfs2_inode *ip, uint64_t *data_ptr, > struct gfs2_ea_header *ea_hdr, > struct gfs2_ea_header *ea_hdr_prev, > void *private); > +static int check_num_ptrs(struct gfs2_inode *ip, uint64_t leafno, > + int *ref_count, int *lindex, struct gfs2_leaf *leaf); > 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, > @@ -79,7 +81,7 @@ static int handle_ip(struct gfs2_sbd *sdp, struct gfs2_inode *ip); > > struct metawalk_fxns pass1_fxns = { > .private = NULL, > - .check_leaf = leaf, > + .check_leaf = check_leaf, > .check_metalist = check_metalist, > .check_data = check_data, > .check_eattr_indir = check_eattr_indir, > @@ -89,6 +91,7 @@ struct metawalk_fxns pass1_fxns = { > .check_eattr_extentry = check_extended_leaf_eattr, > .finish_eattr_indir = finish_eattr_indir, > .big_file_msg = big_file_comfort, > + .check_num_ptrs = check_num_ptrs, > }; > > struct metawalk_fxns undo_fxns = { > @@ -199,7 +202,132 @@ struct metawalk_fxns sysdir_fxns = { > .check_dentry = resuscitate_dentry, > }; > > -static int leaf(struct gfs2_inode *ip, uint64_t block, void *private) > +/* > + * fix_leaf_pointers - fix a directory dinode that has a number of pointers > + * that is not a multiple of 2. > + * dip - the directory inode having the problem > + * lindex - the index of the leaf right after the problem (need to back up) > + * cur_numleafs - current (incorrect) number of instances of the leaf block > + * correct_numleafs - the correct number instances of the leaf block > + */ > +static int fix_leaf_pointers(struct gfs2_inode *dip, int *lindex, > + int cur_numleafs, int correct_numleafs) > +{ > + int count; > + char *ptrbuf; > + int start_lindex = *lindex - cur_numleafs; /* start of bad ptrs */ > + int tot_num_ptrs = (1 << dip->i_di.di_depth) - start_lindex; > + int bufsize = tot_num_ptrs * sizeof(uint64_t); > + int off_by = cur_numleafs - correct_numleafs; > + > + ptrbuf = malloc(bufsize); > + if (!ptrbuf) { > + log_err( _("Error: Cannot allocate memory to fix the leaf " > + "pointers.\n")); > + return -1; > + } > + /* Read all the pointers, starting with the first bad one */ > + count = gfs2_readi(dip, ptrbuf, start_lindex * sizeof(uint64_t), > + bufsize); > + if (count != bufsize) { > + log_err( _("Error: bad read while fixing leaf pointers.\n")); > + free(ptrbuf); > + return -1; > + } > + > + bufsize -= off_by * sizeof(uint64_t); /* We need to write fewer */ > + /* Write the same pointers, but offset them so they fit within the > + smaller factor of 2. So if we have 12 pointers, write out only > + the last 8 of them. If we have 7, write the last 4, etc. > + We need to write these starting at the current lindex and adjust > + lindex accordingly. */ > + count = gfs2_writei(dip, ptrbuf + (off_by * sizeof(uint64_t)), > + start_lindex * sizeof(uint64_t), bufsize); > + if (count != bufsize) { > + log_err( _("Error: bad read while fixing leaf pointers.\n")); > + free(ptrbuf); > + return -1; > + } > + /* Now zero out the hole left at the end */ > + memset(ptrbuf, 0, off_by * sizeof(uint64_t)); > + gfs2_writei(dip, ptrbuf, (start_lindex * sizeof(uint64_t)) + > + bufsize, off_by * sizeof(uint64_t)); > + free(ptrbuf); > + *lindex -= off_by; /* adjust leaf index to account for the change */ > + return 0; > +} > + > +/** > + * check_num_ptrs - check a previously processed leaf's pointer count > + * > + * ip - pointer to the in-core inode structure > + * leafno - the leaf number we're operating on > + * ref_count - the number of pointers to this leaf we actually counted. > + * exp_count - the number of pointers to this leaf we expect based on > + * ip depth minus leaf depth. > + * lindex - leaf index number > + * leaf - the leaf structure for the leaf block to check > + */ > +static int check_num_ptrs(struct gfs2_inode *ip, uint64_t leafno, > + int *ref_count, int *lindex, struct gfs2_leaf *leaf) > +{ > + int factor = 0, divisor = *ref_count, multiple = 1, error = 0; > + struct gfs2_buffer_head *lbh; > + int exp_count; > + > + /* Check to see if the number of pointers we found is a power of 2. > + It needs to be and if it's not we need to fix it.*/ > + while (divisor > 1) { > + factor++; > + divisor /= 2; > + multiple = multiple << 1; > + } > + if (*ref_count != multiple) { > + log_err( _("Directory #%llu (0x%llx) has an invalid number of " > + "pointers to leaf #%llu (0x%llx)\n\tFound: %u, " > + "which is not a factor of 2.\n"), > + (unsigned long long)ip->i_di.di_num.no_addr, > + (unsigned long long)ip->i_di.di_num.no_addr, > + (unsigned long long)leafno, > + (unsigned long long)leafno, *ref_count); > + if (!query( _("Attempt to fix it? (y/n) "))) { > + log_err( _("Directory inode was not fixed.\n")); > + return 1; > + } > + error = fix_leaf_pointers(ip, lindex, *ref_count, multiple); > + if (error) > + return error; > + *ref_count = multiple; > + log_err( _("Directory inode was fixed.\n")); > + } > + /* Check to see if the counted number of leaf pointers is what we > + expect based on the leaf depth. */ > + exp_count = (1 << (ip->i_di.di_depth - leaf->lf_depth)); > + if (*ref_count != exp_count) { > + log_err( _("Directory #%llu (0x%llx) has an incorrect number " > + "of pointers to leaf #%llu (0x%llx)\n\tFound: " > + "%u, Expected: %u\n"), > + (unsigned long long)ip->i_di.di_num.no_addr, > + (unsigned long long)ip->i_di.di_num.no_addr, > + (unsigned long long)leafno, > + (unsigned long long)leafno, *ref_count, exp_count); > + if (!query( _("Attempt to fix it? (y/n) "))) { > + log_err( _("Directory leaf was not fixed.\n")); > + return 1; > + } > + lbh = bread(ip->i_sbd, leafno); > + gfs2_leaf_in(leaf, lbh); > + log_err( _("Leaf depth was %d, changed to %d\n"), > + leaf->lf_depth, ip->i_di.di_depth - factor); > + leaf->lf_depth = ip->i_di.di_depth - factor; > + gfs2_leaf_out(leaf, lbh); > + brelse(lbh); > + log_err( _("Directory leaf was fixed.\n")); > + } > + return 0; > +} > + > +static int check_leaf(struct gfs2_inode *ip, uint64_t block, void *private) > { > struct block_count *bc = (struct block_count *) private; >