* [Cluster-devel] [fsck.gfs2 PATCH] fsck.gfs2: Reprocess nodes if anything changed
2015-01-15 16:55 ` [Cluster-devel] [fsck.gfs2 PATCH] fsck.gfs2: Reprocess nodes if anything changed Bob Peterson
@ 2015-01-15 18:08 ` Abhijith Das
0 siblings, 0 replies; 2+ messages in thread
From: Abhijith Das @ 2015-01-15 18:08 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
ACK.
Cheers!
--Abhi
----- Original Message -----
> From: "Bob Peterson" <rpeterso@redhat.com>
> To: "cluster-devel" <cluster-devel@redhat.com>
> Sent: Thursday, January 15, 2015 10:55:21 AM
> Subject: [Cluster-devel] [fsck.gfs2 PATCH] fsck.gfs2: Reprocess nodes if anything changed
>
> Hi,
>
> Before this patch, fsck.gfs2 called "reprocess_inode" in several
> places, especially when the number of blocks had changed from the
> original. That works in almost all cases. However, there's a corner
> case where a block may be deleted, due to errors (for example, a
> bad directory leaf), and another block takes its place. In that
> case the count of the number of blocks is still the same, but the
> inode should be reprocessed anyway, because the deleted blocks
> and replacement blocks may be at different locations or maybe of
> different types. A hash table block may be "data" when it's freed,
> but if the block is reused, it may be repurposed as a "leaf" block.
> That may confuse subsequent processing. Another reason to call
> reprocess_inode is when the goal block changes to a value outside
> the resource group, due to block allocations for repairs.
>
> Regards,
>
> Bob Peterson
> Red Hat File Systems
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
> diff --git a/gfs2/fsck/lost_n_found.c b/gfs2/fsck/lost_n_found.c
> index 9672c7a..b2e35e2 100644
> --- a/gfs2/fsck/lost_n_found.c
> +++ b/gfs2/fsck/lost_n_found.c
> @@ -174,7 +174,6 @@ void make_sure_lf_exists(struct gfs2_inode *ip)
> int add_inode_to_lf(struct gfs2_inode *ip){
> char tmp_name[256];
> __be32 inode_type;
> - uint64_t lf_blocks;
> struct gfs2_sbd *sdp = ip->i_sbd;
> int err = 0;
> uint32_t mode;
> @@ -184,7 +183,6 @@ int add_inode_to_lf(struct gfs2_inode *ip){
> log_err( _("Trying to add lost+found to itself...skipping"));
> return 0;
> }
> - lf_blocks = lf_dip->i_di.di_blocks;
>
> if (sdp->gfs1)
> mode = gfs_to_gfs2_mode(ip);
> @@ -242,10 +240,6 @@ int add_inode_to_lf(struct gfs2_inode *ip){
> tmp_name, strerror(errno));
> exit(FSCK_ERROR);
> }
> - /* If the lf directory had new blocks added we have to mark them
> - properly in the bitmap so they're not freed. */
> - if (lf_dip->i_di.di_blocks != lf_blocks)
> - reprocess_inode(lf_dip, "lost+found");
>
> /* This inode is linked from lost+found */
> incr_link_count(ip->i_di.di_num, lf_dip, _("from lost+found"));
> diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
> index 16faafb..217bb07 100644
> --- a/gfs2/fsck/metawalk.c
> +++ b/gfs2/fsck/metawalk.c
> @@ -1693,11 +1693,11 @@ int check_dir(struct gfs2_sbd *sdp, uint64_t block,
> struct metawalk_fxns *pass)
> {
> struct gfs2_inode *ip;
> int error = 0;
> - uint64_t cur_blks;
> + struct alloc_state as;
>
> ip = fsck_load_inode(sdp, block);
>
> - cur_blks = ip->i_di.di_blocks;
> + astate_save(ip, &as);
>
> if (ip->i_di.di_flags & GFS2_DIF_EXHASH)
> error = check_leaf_blks(ip, pass);
> @@ -1707,7 +1707,7 @@ int check_dir(struct gfs2_sbd *sdp, uint64_t block,
> struct metawalk_fxns *pass)
> if (error < 0)
> stack;
>
> - if (ip->i_di.di_blocks != cur_blks)
> + if (astate_changed(ip, &as))
> reprocess_inode(ip, _("Current"));
>
> fsck_inode_put(&ip); /* does a brelse */
> diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c
> index 4ea322a..74bcc26 100644
> --- a/gfs2/fsck/pass2.c
> +++ b/gfs2/fsck/pass2.c
> @@ -1723,7 +1723,8 @@ build_it:
> static int check_system_dir(struct gfs2_inode *sysinode, const char
> *dirname,
> int builder(struct gfs2_sbd *sdp))
> {
> - uint64_t iblock = 0, cur_blks;
> + uint64_t iblock = 0;
> + struct alloc_state as;
> struct dir_status ds = {0};
> int error = 0;
>
> @@ -1740,14 +1741,14 @@ static int check_system_dir(struct gfs2_inode
> *sysinode, const char *dirname,
>
> pass2_fxns.private = (void *) &ds;
> if (ds.q == gfs2_bad_block) {
> - cur_blks = sysinode->i_di.di_blocks;
> + astate_save(sysinode, &as);
> /* First check that the directory's metatree is valid */
> error = check_metatree(sysinode, &pass2_fxns);
> if (error < 0) {
> stack;
> return error;
> }
> - if (sysinode->i_di.di_blocks != cur_blks)
> + if (astate_changed(sysinode, &as))
> reprocess_inode(sysinode, _("System inode"));
> }
> error = check_dir(sysinode->i_sbd, iblock, &pass2_fxns);
> @@ -1768,7 +1769,7 @@ static int check_system_dir(struct gfs2_inode
> *sysinode, const char *dirname,
> if (!ds.dotdir) {
> log_err( _("No '.' entry found for %s directory.\n"), dirname);
> if (query( _("Is it okay to add '.' entry? (y/n) "))) {
> - cur_blks = sysinode->i_di.di_blocks;
> + astate_save(sysinode, &as);
> log_warn( _("Adding '.' entry\n"));
> error = dir_add(sysinode, ".", 1, &(sysinode->i_di.di_num),
> (sysinode->i_sbd->gfs1 ? GFS_FILE_DIR : DT_DIR));
> @@ -1777,7 +1778,7 @@ static int check_system_dir(struct gfs2_inode
> *sysinode, const char *dirname,
> strerror(errno));
> return -errno;
> }
> - if (cur_blks != sysinode->i_di.di_blocks)
> + if (astate_changed(sysinode, &as))
> reprocess_inode(sysinode, dirname);
> /* This system inode is linked to itself via '.' */
> incr_link_count(sysinode->i_di.di_num, sysinode,
> @@ -1859,7 +1860,8 @@ static inline int is_system_dir(struct gfs2_sbd *sdp,
> uint64_t block)
> */
> int pass2(struct gfs2_sbd *sdp)
> {
> - uint64_t dirblk, cur_blks;
> + uint64_t dirblk;
> + struct alloc_state as;
> uint8_t q;
> struct dir_status ds = {0};
> struct gfs2_inode *ip;
> @@ -1926,14 +1928,14 @@ int pass2(struct gfs2_sbd *sdp)
> /* First check that the directory's metatree
> * is valid */
> ip = fsck_load_inode(sdp, dirblk);
> - cur_blks = ip->i_di.di_blocks;
> + astate_save(ip, &as);
> error = check_metatree(ip, &pass2_fxns);
> if (error < 0) {
> stack;
> fsck_inode_put(&ip);
> return error;
> }
> - if (ip->i_di.di_blocks != cur_blks)
> + if (astate_changed(ip, &as))
> reprocess_inode(ip, "current");
> fsck_inode_put(&ip);
> }
> @@ -1994,7 +1996,7 @@ int pass2(struct gfs2_sbd *sdp)
> (unsigned long long)dirblk);
>
> if (query( _("Is it okay to add '.' entry? (y/n) "))) {
> - cur_blks = ip->i_di.di_blocks;
> + astate_save(ip, &as);
> error = dir_add(ip, ".", 1, &(ip->i_di.di_num),
> (sdp->gfs1 ? GFS_FILE_DIR : DT_DIR));
> if (error) {
> @@ -2003,7 +2005,7 @@ int pass2(struct gfs2_sbd *sdp)
> fsck_inode_put(&ip);
> return -errno;
> }
> - if (cur_blks != ip->i_di.di_blocks) {
> + if (astate_changed(ip, &as)) {
> char dirname[80];
>
> sprintf(dirname, _("Directory at %lld "
> diff --git a/gfs2/fsck/pass3.c b/gfs2/fsck/pass3.c
> index 33865df..3e183e5 100644
> --- a/gfs2/fsck/pass3.c
> +++ b/gfs2/fsck/pass3.c
> @@ -24,7 +24,7 @@ static int attach_dotdot_to(struct gfs2_sbd *sdp, uint64_t
> newdotdot,
> int filename_len = 2;
> int err;
> struct gfs2_inode *ip, *pip;
> - uint64_t cur_blks;
> + struct alloc_state as;
>
> ip = fsck_load_inode(sdp, block);
> pip = fsck_load_inode(sdp, newdotdot);
> @@ -39,7 +39,7 @@ static int attach_dotdot_to(struct gfs2_sbd *sdp, uint64_t
> newdotdot,
> log_warn( _("Unable to remove \"..\" directory entry.\n"));
> else
> decr_link_count(olddotdot, block, _("old \"..\""));
> - cur_blks = ip->i_di.di_blocks;
> + astate_save(ip, &as);
> err = dir_add(ip, filename, filename_len, &pip->i_di.di_num,
> (sdp->gfs1 ? GFS_FILE_DIR : DT_DIR));
> if (err) {
> @@ -47,7 +47,7 @@ static int attach_dotdot_to(struct gfs2_sbd *sdp, uint64_t
> newdotdot,
> filename, strerror(errno));
> exit(FSCK_ERROR);
> }
> - if (cur_blks != ip->i_di.di_blocks) {
> + if (astate_changed(ip, &as)) {
> char dirname[80];
>
> sprintf(dirname, _("Directory at %lld (0x%llx)"),
> @@ -179,6 +179,7 @@ int pass3(struct gfs2_sbd *sdp)
> struct dir_info *di, *tdi;
> struct gfs2_inode *ip;
> uint8_t q;
> + struct alloc_state lf_as = {.as_blocks = 0, .as_meta_goal = 0};
>
> di = dirtree_find(sdp->md.rooti->i_di.di_num.no_addr);
> if (di) {
> @@ -225,6 +226,8 @@ int pass3(struct gfs2_sbd *sdp)
> */
> log_info( _("Checking directory linkage.\n"));
> for (tmp = osi_first(&dirtree); tmp; tmp = next) {
> + if (lf_dip && lf_as.as_blocks == 0)
> + astate_save(lf_dip, &lf_as);
> next = osi_next(tmp);
> di = (struct dir_info *)tmp;
> while (!di->checked) {
> @@ -330,8 +333,14 @@ int pass3(struct gfs2_sbd *sdp)
> break;
> }
> }
> - if (lf_dip)
> + if (lf_dip) {
> + /* If the lf directory had new blocks added we have to mark
> + them properly in the blockmap so they're not freed. */
> + if (astate_changed(lf_dip, &lf_as))
> + reprocess_inode(lf_dip, "lost+found");
> +
> log_debug( _("At end of pass3, lost+found entries is %u\n"),
> lf_dip->i_di.di_entries);
> + }
> return FSCK_OK;
> }
> diff --git a/gfs2/fsck/pass4.c b/gfs2/fsck/pass4.c
> index f9d08dc..324ea9f 100644
> --- a/gfs2/fsck/pass4.c
> +++ b/gfs2/fsck/pass4.c
> @@ -48,12 +48,15 @@ static int scan_inode_list(struct gfs2_sbd *sdp) {
> struct gfs2_inode *ip;
> int lf_addition = 0;
> uint8_t q;
> + struct alloc_state lf_as = {.as_blocks = 0, .as_meta_goal = 0};
>
> /* FIXME: should probably factor this out into a generic
> * scanning fxn */
> for (tmp = osi_first(&inodetree); tmp; tmp = next) {
> if (skip_this_pass || fsck_abort) /* if asked to skip the rest */
> return 0;
> + if (lf_dip && lf_as.as_blocks == 0)
> + astate_save(lf_dip, &lf_as);
> next = osi_next(tmp);
> ii = (struct inode_info *)tmp;
> /* Don't check reference counts on the special gfs files */
> @@ -179,6 +182,9 @@ static int scan_inode_list(struct gfs2_sbd *sdp) {
> (unsigned long long)ii->di_num.no_addr, ii->di_nlink);
> } /* osi_list_foreach(tmp, list) */
>
> + if (lf_dip && astate_changed(lf_dip, &lf_as))
> + reprocess_inode(lf_dip, "lost+found");
> +
> if (lf_addition) {
> if (!(ii = inodetree_find(lf_dip->i_di.di_num.no_addr))) {
> log_crit( _("Unable to find lost+found inode in inode_hash!!\n"));
> diff --git a/gfs2/fsck/util.h b/gfs2/fsck/util.h
> index 276c726..66b9c7a 100644
> --- a/gfs2/fsck/util.h
> +++ b/gfs2/fsck/util.h
> @@ -12,6 +12,11 @@
> #define INODE_VALID 1
> #define INODE_INVALID 0
>
> +struct alloc_state {
> + uint64_t as_blocks;
> + uint64_t as_meta_goal;
> +};
> +
> struct di_info *search_list(osi_list_t *list, uint64_t addr);
> void big_file_comfort(struct gfs2_inode *ip, uint64_t blks_checked);
> void warm_fuzzy_stuff(uint64_t block);
> @@ -27,6 +32,21 @@ extern const char *reftypes[ref_types + 1];
> #define BLOCKMAP_BYTE_OFFSET4(x) (((x) & 0x0000000000000001) << 2)
> #define BLOCKMAP_MASK4 (0xf)
>
> +static inline void astate_save(struct gfs2_inode *ip, struct alloc_state
> *as)
> +{
> + as->as_blocks = ip->i_di.di_blocks;
> + as->as_meta_goal = ip->i_di.di_goal_meta;
> +}
> +
> +static inline int astate_changed(struct gfs2_inode *ip, struct alloc_state
> *as)
> +{
> + if (as->as_blocks != ip->i_di.di_blocks)
> + return 1;
> + if (as->as_meta_goal != ip->i_di.di_goal_meta)
> + return 1;
> + return 0;
> +}
> +
> static inline uint8_t block_type(uint64_t bblock)
> {
> static unsigned char *byte;
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread