[Cluster-devel] [fsck.gfs2 patch] fsck.gfs2: remove duplicate designation during undo

[Cluster-devel] [fsck.gfs2 patch] fsck.gfs2: remove duplicate designation during undo

[Bob Peterson]

Hi,

This patch fixes a problem whereby fsck.gfs2's pass1 would perform this
sequence of events:
1. Metadata block X is identified as being referenced from dinode D1.
2. Metadata block X is identified as being referenced from another dinode, D2,
   which makes it a duplicate reference, but so far, no serious errors were
   found for that dinode.
3. Dinode D2 is found later to be irreparably damaged, and needs to be removed.
When D2 is deleted, the duplicate reference from D2 is removed and block X is
not freed because D1 still references it. However, it's still marked as a
duplicate and requires processing in pass1b.

Later, pass1b resolves the duplicate and determine's there is really only one
reference remaining, so it makes the correct decision. However, it should not
be necessary. The "undo" functions should remove the duplicate reference if
(and only if) the only reference was from D2. Note, though, that if the
corruption is found later in the cycle (after "undo" is possible) the duplicate
reference MUST remain and be resolved by pass1b.

Regards,

Bob Peterson
Red Hat File Systems

Signed-off-by: Bob Peterson <rpeterso@redhat.com> 
---
diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c
index a4ba04c..73b054c 100644
--- a/gfs2/fsck/pass1.c
+++ b/gfs2/fsck/pass1.c
@@ -364,6 +364,11 @@ static int undo_reference(struct gfs2_inode *ip, uint64_t block, int meta,
 				  "from another inode; not freeing.\n"),
 				(unsigned long long)block,
 				(unsigned long long)block);
+			if (dt->refs == 1) {
+				log_err(_("This was the only duplicate "
+					  "reference so far; removing it.\n"));
+				dup_delete(dt);
+			}
 			return 1;
 		}
 	}
@@ -1055,8 +1060,13 @@ static int rangecheck_block(struct gfs2_inode *ip, uint64_t block,
 			  (unsigned long long)ip->i_di.di_num.no_addr);
 		if ((*bad_pointers) <= BAD_POINTER_TOLERANCE)
 			return meta_is_good;
-		else
+		else {
+			log_debug(_("Inode 0x%llx bad pointer tolerance "
+				    "exceeded: block 0x%llx.\n"),
+				  (unsigned long long)ip->i_di.di_num.no_addr,
+				  (unsigned long long)block);
 			return meta_error; /* Exits check_metatree quicker */
+		}
 	}
 	return meta_is_good;
 }

[Cluster-devel] [fsck.gfs2 patch] fsck.gfs2: remove duplicate designation during undo

[Andrew Price]

Hi,

Looks good to me, ACK to all 5 patches.

Andy

On 09/01/15 19:34, Bob Peterson wrote:
> Hi,
>
> This patch fixes a problem whereby fsck.gfs2's pass1 would perform this
> sequence of events:
> 1. Metadata block X is identified as being referenced from dinode D1.
> 2. Metadata block X is identified as being referenced from another dinode, D2,
>     which makes it a duplicate reference, but so far, no serious errors were
>     found for that dinode.
> 3. Dinode D2 is found later to be irreparably damaged, and needs to be removed.
> When D2 is deleted, the duplicate reference from D2 is removed and block X is
> not freed because D1 still references it. However, it's still marked as a
> duplicate and requires processing in pass1b.
>
> Later, pass1b resolves the duplicate and determine's there is really only one
> reference remaining, so it makes the correct decision. However, it should not
> be necessary. The "undo" functions should remove the duplicate reference if
> (and only if) the only reference was from D2. Note, though, that if the
> corruption is found later in the cycle (after "undo" is possible) the duplicate
> reference MUST remain and be resolved by pass1b.
>
> Regards,
>
> Bob Peterson
> Red Hat File Systems
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
> diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c
> index a4ba04c..73b054c 100644
> --- a/gfs2/fsck/pass1.c
> +++ b/gfs2/fsck/pass1.c
> @@ -364,6 +364,11 @@ static int undo_reference(struct gfs2_inode *ip, uint64_t block, int meta,
>   				  "from another inode; not freeing.\n"),
>   				(unsigned long long)block,
>   				(unsigned long long)block);
> +			if (dt->refs == 1) {
> +				log_err(_("This was the only duplicate "
> +					  "reference so far; removing it.\n"));
> +				dup_delete(dt);
> +			}
>   			return 1;
>   		}
>   	}
> @@ -1055,8 +1060,13 @@ static int rangecheck_block(struct gfs2_inode *ip, uint64_t block,
>   			  (unsigned long long)ip->i_di.di_num.no_addr);
>   		if ((*bad_pointers) <= BAD_POINTER_TOLERANCE)
>   			return meta_is_good;
> -		else
> +		else {
> +			log_debug(_("Inode 0x%llx bad pointer tolerance "
> +				    "exceeded: block 0x%llx.\n"),
> +				  (unsigned long long)ip->i_di.di_num.no_addr,
> +				  (unsigned long long)block);
>   			return meta_error; /* Exits check_metatree quicker */
> +		}
>   	}
>   	return meta_is_good;
>   }
>