[23/28] ocfs2: check/fix inode block for online file check
diff mbox

Message ID 55de39c6.NQqMm5hA00TLdLDU%akpm@linux-foundation.org
State New
Headers show

Commit Message

Andrew Morton Aug. 26, 2015, 10:12 p.m. UTC
From: Gang He <ghe@suse.com>
Subject: ocfs2: check/fix inode block for online file check

Implement online check or fix inode block during reading a inode block to
memory.

Signed-off-by: Gang He <ghe@suse.com>
Reviewed-by: Goldwyn Rodrigues <rgoldwyn@suse.de>
Cc: Mark Fasheh <mfasheh@suse.com>
Cc: Joel Becker <jlbec@evilplan.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 fs/ocfs2/inode.c       |  196 +++++++++++++++++++++++++++++++++++++--
 fs/ocfs2/ocfs2_trace.h |    2 
 2 files changed, 192 insertions(+), 6 deletions(-)

Comments

Mark Fasheh Aug. 31, 2015, 8:56 p.m. UTC | #1
On Wed, Aug 26, 2015 at 03:12:22PM -0700, Andrew Morton wrote:
> From: Gang He <ghe@suse.com>
> Subject: ocfs2: check/fix inode block for online file check
> 
> Implement online check or fix inode block during reading a inode block to
> memory.
> 
> Signed-off-by: Gang He <ghe@suse.com>
> Reviewed-by: Goldwyn Rodrigues <rgoldwyn@suse.de>
> Cc: Mark Fasheh <mfasheh@suse.com>
> Cc: Joel Becker <jlbec@evilplan.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  fs/ocfs2/inode.c       |  196 +++++++++++++++++++++++++++++++++++++--
>  fs/ocfs2/ocfs2_trace.h |    2 
>  2 files changed, 192 insertions(+), 6 deletions(-)
> 
> diff -puN fs/ocfs2/inode.c~ocfs2-check-fix-inode-block-for-online-file-check fs/ocfs2/inode.c
> --- a/fs/ocfs2/inode.c~ocfs2-check-fix-inode-block-for-online-file-check
> +++ a/fs/ocfs2/inode.c
> @@ -53,6 +53,7 @@
>  #include "xattr.h"
>  #include "refcounttree.h"
>  #include "ocfs2_trace.h"
> +#include "filecheck.h"
>  
>  #include "buffer_head_io.h"
>  
> @@ -74,6 +75,13 @@ static int ocfs2_truncate_for_delete(str
>  				    struct inode *inode,
>  				    struct buffer_head *fe_bh);
>  
> +static int ocfs2_filecheck_read_inode_block_full(struct inode *inode,
> +			struct buffer_head **bh, int flags, int type);
> +static int ocfs2_filecheck_validate_inode_block(struct super_block *sb,
> +			struct buffer_head *bh);
> +static int ocfs2_filecheck_repair_inode_block(struct super_block *sb,
> +			struct buffer_head *bh);
> +
>  void ocfs2_set_inode_flags(struct inode *inode)
>  {
>  	unsigned int flags = OCFS2_I(inode)->ip_attr;
> @@ -127,6 +135,7 @@ struct inode *ocfs2_ilookup(struct super
>  struct inode *ocfs2_iget(struct ocfs2_super *osb, u64 blkno, unsigned flags,
>  			 int sysfile_type)
>  {
> +	int rc = 0;
>  	struct inode *inode = NULL;
>  	struct super_block *sb = osb->sb;
>  	struct ocfs2_find_inode_args args;
> @@ -161,12 +170,17 @@ struct inode *ocfs2_iget(struct ocfs2_su
>  	}
>  	trace_ocfs2_iget5_locked(inode->i_state);
>  	if (inode->i_state & I_NEW) {
> -		ocfs2_read_locked_inode(inode, &args);
> +		rc = ocfs2_read_locked_inode(inode, &args);
>  		unlock_new_inode(inode);
>  	}
>  	if (is_bad_inode(inode)) {
>  		iput(inode);
> -		inode = ERR_PTR(-ESTALE);
> +		if ((flags & OCFS2_FI_FLAG_FILECHECK_CHK) ||
> +			(flags & OCFS2_FI_FLAG_FILECHECK_FIX))
> +			/* Return OCFS2_FILECHECK_ERR_XXX related errno */
> +			inode = ERR_PTR(rc);
> +		else
> +			inode = ERR_PTR(-ESTALE);
>  		goto bail;
>  	}
>  
> @@ -494,16 +508,32 @@ static int ocfs2_read_locked_inode(struc
>  	}
>  
>  	if (can_lock) {
> -		status = ocfs2_read_inode_block_full(inode, &bh,
> -						     OCFS2_BH_IGNORE_CACHE);
> +		if (args->fi_flags & OCFS2_FI_FLAG_FILECHECK_CHK)
> +			status = ocfs2_filecheck_read_inode_block_full(inode,
> +						&bh, OCFS2_BH_IGNORE_CACHE, 0);
> +		else if (args->fi_flags & OCFS2_FI_FLAG_FILECHECK_FIX)
> +			status = ocfs2_filecheck_read_inode_block_full(inode,
> +						&bh, OCFS2_BH_IGNORE_CACHE, 1);
> +		else
> +			status = ocfs2_read_inode_block_full(inode,
> +						&bh, OCFS2_BH_IGNORE_CACHE);

NAK, at first glance this is very hacky - I don't like that we've hidden these checks
and fixups down in iget(). If there's a reason it has to be this way that
should be explained, but otherwise I would expect the check/repair code to
be less intertwined with ocfs2_iget(). Otherwise I fear we're setting ourselves up
for finding some ugly bugs down the road.

Btw, how does the code handle the case where the inode is already in our
cache? In that case you'd never get to read_locked_inode()...

Thanks,
	--Mark

--
Mark Fasheh
Gang He Sept. 1, 2015, 3:35 a.m. UTC | #2
Hello Mark,

Thanks for your reviewing, please see my comments inline.


>>> 
> On Wed, Aug 26, 2015 at 03:12:22PM -0700, Andrew Morton wrote:
>> From: Gang He <ghe@suse.com>
>> Subject: ocfs2: check/fix inode block for online file check
>> 
>> Implement online check or fix inode block during reading a inode block to
>> memory.
>> 
>> Signed-off-by: Gang He <ghe@suse.com>
>> Reviewed-by: Goldwyn Rodrigues <rgoldwyn@suse.de>
>> Cc: Mark Fasheh <mfasheh@suse.com>
>> Cc: Joel Becker <jlbec@evilplan.org>
>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>> ---
>> 
>>  fs/ocfs2/inode.c       |  196 +++++++++++++++++++++++++++++++++++++--
>>  fs/ocfs2/ocfs2_trace.h |    2 
>>  2 files changed, 192 insertions(+), 6 deletions(-)
>> 
>> diff -puN fs/ocfs2/inode.c~ocfs2-check-fix-inode-block-for-online-file-check 
> fs/ocfs2/inode.c
>> --- a/fs/ocfs2/inode.c~ocfs2-check-fix-inode-block-for-online-file-check
>> +++ a/fs/ocfs2/inode.c
>> @@ -53,6 +53,7 @@
>>  #include "xattr.h"
>>  #include "refcounttree.h"
>>  #include "ocfs2_trace.h"
>> +#include "filecheck.h"
>>  
>>  #include "buffer_head_io.h"
>>  
>> @@ -74,6 +75,13 @@ static int ocfs2_truncate_for_delete(str
>>  				    struct inode *inode,
>>  				    struct buffer_head *fe_bh);
>>  
>> +static int ocfs2_filecheck_read_inode_block_full(struct inode *inode,
>> +			struct buffer_head **bh, int flags, int type);
>> +static int ocfs2_filecheck_validate_inode_block(struct super_block *sb,
>> +			struct buffer_head *bh);
>> +static int ocfs2_filecheck_repair_inode_block(struct super_block *sb,
>> +			struct buffer_head *bh);
>> +
>>  void ocfs2_set_inode_flags(struct inode *inode)
>>  {
>>  	unsigned int flags = OCFS2_I(inode)->ip_attr;
>> @@ -127,6 +135,7 @@ struct inode *ocfs2_ilookup(struct super
>>  struct inode *ocfs2_iget(struct ocfs2_super *osb, u64 blkno, unsigned 
> flags,
>>  			 int sysfile_type)
>>  {
>> +	int rc = 0;
>>  	struct inode *inode = NULL;
>>  	struct super_block *sb = osb->sb;
>>  	struct ocfs2_find_inode_args args;
>> @@ -161,12 +170,17 @@ struct inode *ocfs2_iget(struct ocfs2_su
>>  	}
>>  	trace_ocfs2_iget5_locked(inode->i_state);
>>  	if (inode->i_state & I_NEW) {
>> -		ocfs2_read_locked_inode(inode, &args);
>> +		rc = ocfs2_read_locked_inode(inode, &args);
>>  		unlock_new_inode(inode);
>>  	}
>>  	if (is_bad_inode(inode)) {
>>  		iput(inode);
>> -		inode = ERR_PTR(-ESTALE);
>> +		if ((flags & OCFS2_FI_FLAG_FILECHECK_CHK) ||
>> +			(flags & OCFS2_FI_FLAG_FILECHECK_FIX))
>> +			/* Return OCFS2_FILECHECK_ERR_XXX related errno */
>> +			inode = ERR_PTR(rc);
>> +		else
>> +			inode = ERR_PTR(-ESTALE);
>>  		goto bail;
>>  	}
>>  
>> @@ -494,16 +508,32 @@ static int ocfs2_read_locked_inode(struc
>>  	}
>>  
>>  	if (can_lock) {
>> -		status = ocfs2_read_inode_block_full(inode, &bh,
>> -						     OCFS2_BH_IGNORE_CACHE);
>> +		if (args->fi_flags & OCFS2_FI_FLAG_FILECHECK_CHK)
>> +			status = ocfs2_filecheck_read_inode_block_full(inode,
>> +						&bh, OCFS2_BH_IGNORE_CACHE, 0);
>> +		else if (args->fi_flags & OCFS2_FI_FLAG_FILECHECK_FIX)
>> +			status = ocfs2_filecheck_read_inode_block_full(inode,
>> +						&bh, OCFS2_BH_IGNORE_CACHE, 1);
>> +		else
>> +			status = ocfs2_read_inode_block_full(inode,
>> +						&bh, OCFS2_BH_IGNORE_CACHE);
> 
> NAK, at first glance this is very hacky - I don't like that we've hidden 
> these checks
> and fixups down in iget(). If there's a reason it has to be this way that
> should be explained, but otherwise I would expect the check/repair code to
> be less intertwined with ocfs2_iget(). Otherwise I fear we're setting 
> ourselves up
> for finding some ugly bugs down the road.
Firstly, I want to read inode block separately, but the feature is working with a online file system, 
we can't avoid the inodes cache and the cluster environment, I think that I should not handle a inode 
block separately without considering the current inodes cache and potential cluster access from other node.
Then I tried to integrate this light-level online check/fix with iget() function, make the online check/fix 
operations is compatible with the inodes cache and the cluster environment.
This is why I use iget() to integrate this feature. if there is a more graceful way to implement this feature,
please help to give some suggestions. 

> 
> Btw, how does the code handle the case where the inode is already in our
> cache? In that case you'd never get to read_locked_inode()...
This online file check/fix is light-level meta-data block check/fix, the check/fix fields usually correspond 
with ocfs2_validate_inode_block(), for more serious problem, we have to use fsck by offline.
If the inode is already in our cache, that means this inode block passed ocfs2_validate_inode_block()
verification during loading inode block from the disk, this inode block is sane, we need not check it again.

Thanks
Gang 
 

> 
> Thanks,
> 	--Mark
> 
> --
> Mark Fasheh
Gang He Sept. 18, 2015, 9:22 a.m. UTC | #3
Hello Mark and All,

The implementation code looks a little tricky, but It looks to have to hack like that way in a online file system environment. I also want to find a more graceful/concise way to implement this feature.
Please help to take some time in thinking about this problem, find a compromised way, and make the thing move forward.

Thanks a lot.
Gang


>>> 
> Hello Mark,
> 
> Thanks for your reviewing, please see my comments inline.
> 
> 
> >>> 
> > On Wed, Aug 26, 2015 at 03:12:22PM -0700, Andrew Morton wrote:
> >> From: Gang He <ghe@suse.com>
> >> Subject: ocfs2: check/fix inode block for online file check
> >> 
> >> Implement online check or fix inode block during reading a inode block to
> >> memory.
> >> 
> >> Signed-off-by: Gang He <ghe@suse.com>
> >> Reviewed-by: Goldwyn Rodrigues <rgoldwyn@suse.de>
> >> Cc: Mark Fasheh <mfasheh@suse.com>
> >> Cc: Joel Becker <jlbec@evilplan.org>
> >> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> >> ---
> >> 
> >>  fs/ocfs2/inode.c       |  196 +++++++++++++++++++++++++++++++++++++--
> >>  fs/ocfs2/ocfs2_trace.h |    2 
> >>  2 files changed, 192 insertions(+), 6 deletions(-)
> >> 
> >> diff -puN fs/ocfs2/inode.c~ocfs2-check-fix-inode-block-for-online-file-check 
> > fs/ocfs2/inode.c
> >> --- a/fs/ocfs2/inode.c~ocfs2-check-fix-inode-block-for-online-file-check
> >> +++ a/fs/ocfs2/inode.c
> >> @@ -53,6 +53,7 @@
> >>  #include "xattr.h"
> >>  #include "refcounttree.h"
> >>  #include "ocfs2_trace.h"
> >> +#include "filecheck.h"
> >>  
> >>  #include "buffer_head_io.h"
> >>  
> >> @@ -74,6 +75,13 @@ static int ocfs2_truncate_for_delete(str
> >>  				    struct inode *inode,
> >>  				    struct buffer_head *fe_bh);
> >>  
> >> +static int ocfs2_filecheck_read_inode_block_full(struct inode *inode,
> >> +			struct buffer_head **bh, int flags, int type);
> >> +static int ocfs2_filecheck_validate_inode_block(struct super_block *sb,
> >> +			struct buffer_head *bh);
> >> +static int ocfs2_filecheck_repair_inode_block(struct super_block *sb,
> >> +			struct buffer_head *bh);
> >> +
> >>  void ocfs2_set_inode_flags(struct inode *inode)
> >>  {
> >>  	unsigned int flags = OCFS2_I(inode)->ip_attr;
> >> @@ -127,6 +135,7 @@ struct inode *ocfs2_ilookup(struct super
> >>  struct inode *ocfs2_iget(struct ocfs2_super *osb, u64 blkno, unsigned 
> > flags,
> >>  			 int sysfile_type)
> >>  {
> >> +	int rc = 0;
> >>  	struct inode *inode = NULL;
> >>  	struct super_block *sb = osb->sb;
> >>  	struct ocfs2_find_inode_args args;
> >> @@ -161,12 +170,17 @@ struct inode *ocfs2_iget(struct ocfs2_su
> >>  	}
> >>  	trace_ocfs2_iget5_locked(inode->i_state);
> >>  	if (inode->i_state & I_NEW) {
> >> -		ocfs2_read_locked_inode(inode, &args);
> >> +		rc = ocfs2_read_locked_inode(inode, &args);
> >>  		unlock_new_inode(inode);
> >>  	}
> >>  	if (is_bad_inode(inode)) {
> >>  		iput(inode);
> >> -		inode = ERR_PTR(-ESTALE);
> >> +		if ((flags & OCFS2_FI_FLAG_FILECHECK_CHK) ||
> >> +			(flags & OCFS2_FI_FLAG_FILECHECK_FIX))
> >> +			/* Return OCFS2_FILECHECK_ERR_XXX related errno */
> >> +			inode = ERR_PTR(rc);
> >> +		else
> >> +			inode = ERR_PTR(-ESTALE);
> >>  		goto bail;
> >>  	}
> >>  
> >> @@ -494,16 +508,32 @@ static int ocfs2_read_locked_inode(struc
> >>  	}
> >>  
> >>  	if (can_lock) {
> >> -		status = ocfs2_read_inode_block_full(inode, &bh,
> >> -						     OCFS2_BH_IGNORE_CACHE);
> >> +		if (args->fi_flags & OCFS2_FI_FLAG_FILECHECK_CHK)
> >> +			status = ocfs2_filecheck_read_inode_block_full(inode,
> >> +						&bh, OCFS2_BH_IGNORE_CACHE, 0);
> >> +		else if (args->fi_flags & OCFS2_FI_FLAG_FILECHECK_FIX)
> >> +			status = ocfs2_filecheck_read_inode_block_full(inode,
> >> +						&bh, OCFS2_BH_IGNORE_CACHE, 1);
> >> +		else
> >> +			status = ocfs2_read_inode_block_full(inode,
> >> +						&bh, OCFS2_BH_IGNORE_CACHE);
> > 
> > NAK, at first glance this is very hacky - I don't like that we've hidden 
> > these checks
> > and fixups down in iget(). If there's a reason it has to be this way that
> > should be explained, but otherwise I would expect the check/repair code to
> > be less intertwined with ocfs2_iget(). Otherwise I fear we're setting 
> > ourselves up
> > for finding some ugly bugs down the road.
> Firstly, I want to read inode block separately, but the feature is working 
> with a online file system, 
> we can't avoid the inodes cache and the cluster environment, I think that I 
> should not handle a inode 
> block separately without considering the current inodes cache and potential 
> cluster access from other node.
> Then I tried to integrate this light-level online check/fix with iget() 
> function, make the online check/fix 
> operations is compatible with the inodes cache and the cluster environment.
> This is why I use iget() to integrate this feature. if there is a more 
> graceful way to implement this feature,
> please help to give some suggestions. 
> 
> > 
> > Btw, how does the code handle the case where the inode is already in our
> > cache? In that case you'd never get to read_locked_inode()...
> This online file check/fix is light-level meta-data block check/fix, the 
> check/fix fields usually correspond 
> with ocfs2_validate_inode_block(), for more serious problem, we have to use 
> fsck by offline.
> If the inode is already in our cache, that means this inode block passed 
> ocfs2_validate_inode_block()
> verification during loading inode block from the disk, this inode block is 
> sane, we need not check it again.
> 
> Thanks
> Gang 
>  
> 
> > 
> > Thanks,
> > 	--Mark
> > 
> > --
> > Mark Fasheh

Patch
diff mbox

diff -puN fs/ocfs2/inode.c~ocfs2-check-fix-inode-block-for-online-file-check fs/ocfs2/inode.c
--- a/fs/ocfs2/inode.c~ocfs2-check-fix-inode-block-for-online-file-check
+++ a/fs/ocfs2/inode.c
@@ -53,6 +53,7 @@ 
 #include "xattr.h"
 #include "refcounttree.h"
 #include "ocfs2_trace.h"
+#include "filecheck.h"
 
 #include "buffer_head_io.h"
 
@@ -74,6 +75,13 @@  static int ocfs2_truncate_for_delete(str
 				    struct inode *inode,
 				    struct buffer_head *fe_bh);
 
+static int ocfs2_filecheck_read_inode_block_full(struct inode *inode,
+			struct buffer_head **bh, int flags, int type);
+static int ocfs2_filecheck_validate_inode_block(struct super_block *sb,
+			struct buffer_head *bh);
+static int ocfs2_filecheck_repair_inode_block(struct super_block *sb,
+			struct buffer_head *bh);
+
 void ocfs2_set_inode_flags(struct inode *inode)
 {
 	unsigned int flags = OCFS2_I(inode)->ip_attr;
@@ -127,6 +135,7 @@  struct inode *ocfs2_ilookup(struct super
 struct inode *ocfs2_iget(struct ocfs2_super *osb, u64 blkno, unsigned flags,
 			 int sysfile_type)
 {
+	int rc = 0;
 	struct inode *inode = NULL;
 	struct super_block *sb = osb->sb;
 	struct ocfs2_find_inode_args args;
@@ -161,12 +170,17 @@  struct inode *ocfs2_iget(struct ocfs2_su
 	}
 	trace_ocfs2_iget5_locked(inode->i_state);
 	if (inode->i_state & I_NEW) {
-		ocfs2_read_locked_inode(inode, &args);
+		rc = ocfs2_read_locked_inode(inode, &args);
 		unlock_new_inode(inode);
 	}
 	if (is_bad_inode(inode)) {
 		iput(inode);
-		inode = ERR_PTR(-ESTALE);
+		if ((flags & OCFS2_FI_FLAG_FILECHECK_CHK) ||
+			(flags & OCFS2_FI_FLAG_FILECHECK_FIX))
+			/* Return OCFS2_FILECHECK_ERR_XXX related errno */
+			inode = ERR_PTR(rc);
+		else
+			inode = ERR_PTR(-ESTALE);
 		goto bail;
 	}
 
@@ -494,16 +508,32 @@  static int ocfs2_read_locked_inode(struc
 	}
 
 	if (can_lock) {
-		status = ocfs2_read_inode_block_full(inode, &bh,
-						     OCFS2_BH_IGNORE_CACHE);
+		if (args->fi_flags & OCFS2_FI_FLAG_FILECHECK_CHK)
+			status = ocfs2_filecheck_read_inode_block_full(inode,
+						&bh, OCFS2_BH_IGNORE_CACHE, 0);
+		else if (args->fi_flags & OCFS2_FI_FLAG_FILECHECK_FIX)
+			status = ocfs2_filecheck_read_inode_block_full(inode,
+						&bh, OCFS2_BH_IGNORE_CACHE, 1);
+		else
+			status = ocfs2_read_inode_block_full(inode,
+						&bh, OCFS2_BH_IGNORE_CACHE);
 	} else {
 		status = ocfs2_read_blocks_sync(osb, args->fi_blkno, 1, &bh);
 		/*
 		 * If buffer is in jbd, then its checksum may not have been
 		 * computed as yet.
 		 */
-		if (!status && !buffer_jbd(bh))
-			status = ocfs2_validate_inode_block(osb->sb, bh);
+		if (!status && !buffer_jbd(bh)) {
+			if (args->fi_flags & OCFS2_FI_FLAG_FILECHECK_CHK)
+				status = ocfs2_filecheck_validate_inode_block(
+								osb->sb, bh);
+			else if (args->fi_flags & OCFS2_FI_FLAG_FILECHECK_FIX)
+				status = ocfs2_filecheck_repair_inode_block(
+								osb->sb, bh);
+			else
+				status = ocfs2_validate_inode_block(
+								osb->sb, bh);
+		}
 	}
 	if (status < 0) {
 		mlog_errno(status);
@@ -531,6 +561,14 @@  static int ocfs2_read_locked_inode(struc
 
 	BUG_ON(args->fi_blkno != le64_to_cpu(fe->i_blkno));
 
+	if (buffer_dirty(bh)) {
+		status = ocfs2_write_block(osb, bh, INODE_CACHE(inode));
+		if (status < 0) {
+			mlog_errno(status);
+			goto bail;
+		}
+	}
+
 	status = 0;
 
 bail:
@@ -1396,6 +1434,152 @@  bail:
 	return rc;
 }
 
+static int ocfs2_filecheck_validate_inode_block(struct super_block *sb,
+			       struct buffer_head *bh)
+{
+	int rc = 0;
+	struct ocfs2_dinode *di = (struct ocfs2_dinode *)bh->b_data;
+
+	trace_ocfs2_filecheck_validate_inode_block(
+		(unsigned long long)bh->b_blocknr);
+
+	BUG_ON(!buffer_uptodate(bh));
+
+	if (!OCFS2_IS_VALID_DINODE(di)) {
+		mlog(ML_ERROR,
+			"Filecheck: invalid dinode #%llu: signature = %.*s\n",
+			(unsigned long long)bh->b_blocknr, 7, di->i_signature);
+		rc = -OCFS2_FILECHECK_ERR_INVALIDINO;
+		goto bail;
+	}
+
+	rc = ocfs2_validate_meta_ecc(sb, bh->b_data, &di->i_check);
+	if (rc) {
+		mlog(ML_ERROR,
+			"Filecheck: checksum failed for dinode %llu\n",
+			(unsigned long long)bh->b_blocknr);
+		rc = -OCFS2_FILECHECK_ERR_BLOCKECC;
+		goto bail;
+	}
+
+	if (le64_to_cpu(di->i_blkno) != bh->b_blocknr) {
+		mlog(ML_ERROR,
+			"Filecheck: invalid dinode #%llu: i_blkno is %llu\n",
+			(unsigned long long)bh->b_blocknr,
+			(unsigned long long)le64_to_cpu(di->i_blkno));
+		rc = -OCFS2_FILECHECK_ERR_BLOCKNO;
+		goto bail;
+	}
+
+	if (!(di->i_flags & cpu_to_le32(OCFS2_VALID_FL))) {
+		mlog(ML_ERROR,
+			"Filecheck: invalid dinode #%llu: OCFS2_VALID_FL not set\n",
+			(unsigned long long)bh->b_blocknr);
+		rc = -OCFS2_FILECHECK_ERR_VALIDFLAG;
+		goto bail;
+	}
+
+	if (le32_to_cpu(di->i_fs_generation) !=
+	    OCFS2_SB(sb)->fs_generation) {
+		mlog(ML_ERROR,
+			"Filecheck: invalid dinode #%llu: fs_generation is %u\n",
+			(unsigned long long)bh->b_blocknr,
+			le32_to_cpu(di->i_fs_generation));
+		rc = -OCFS2_FILECHECK_ERR_GENERATION;
+		goto bail;
+	}
+
+bail:
+	return rc;
+}
+
+static int ocfs2_filecheck_repair_inode_block(struct super_block *sb,
+			       struct buffer_head *bh)
+{
+	int rc;
+	int changed = 0;
+	struct ocfs2_dinode *di = (struct ocfs2_dinode *)bh->b_data;
+
+	rc = ocfs2_filecheck_validate_inode_block(sb, bh);
+	/* Can't fix invalid inode block */
+	if (!rc || rc == -OCFS2_FILECHECK_ERR_INVALIDINO)
+		return rc;
+
+	trace_ocfs2_filecheck_repair_inode_block(
+		(unsigned long long)bh->b_blocknr);
+
+	if (ocfs2_is_hard_readonly(OCFS2_SB(sb)) ||
+		ocfs2_is_soft_readonly(OCFS2_SB(sb))) {
+		mlog(ML_ERROR,
+			"Filecheck: try to repair dinode #%llu on readonly filesystem\n",
+			(unsigned long long)bh->b_blocknr);
+		return -OCFS2_FILECHECK_ERR_READONLY;
+	}
+
+	if (le64_to_cpu(di->i_blkno) != bh->b_blocknr) {
+		di->i_blkno = cpu_to_le64(bh->b_blocknr);
+		changed = 1;
+		mlog(ML_ERROR,
+			"Filecheck: reset dinode #%llu: i_blkno to %llu\n",
+			(unsigned long long)bh->b_blocknr,
+			(unsigned long long)le64_to_cpu(di->i_blkno));
+	}
+
+	if (!(di->i_flags & cpu_to_le32(OCFS2_VALID_FL))) {
+		di->i_flags |= cpu_to_le32(OCFS2_VALID_FL);
+		changed = 1;
+		mlog(ML_ERROR,
+			"Filecheck: reset dinode #%llu: OCFS2_VALID_FL is set\n",
+			(unsigned long long)bh->b_blocknr);
+	}
+
+	if (le32_to_cpu(di->i_fs_generation) !=
+	    OCFS2_SB(sb)->fs_generation) {
+		di->i_fs_generation = cpu_to_le32(OCFS2_SB(sb)->fs_generation);
+		changed = 1;
+		mlog(ML_ERROR,
+			"Filecheck: reset dinode #%llu: fs_generation to %u\n",
+			(unsigned long long)bh->b_blocknr,
+			le32_to_cpu(di->i_fs_generation));
+	}
+
+	if (changed ||
+		ocfs2_validate_meta_ecc(sb, bh->b_data, &di->i_check)) {
+		ocfs2_compute_meta_ecc(sb, bh->b_data, &di->i_check);
+		mark_buffer_dirty(bh);
+		mlog(ML_ERROR,
+			"Filecheck: reset dinode #%llu: compute meta ecc\n",
+			(unsigned long long)bh->b_blocknr);
+	}
+
+	return 0;
+}
+
+static int
+ocfs2_filecheck_read_inode_block_full(struct inode *inode,
+		struct buffer_head **bh, int flags, int type)
+{
+	int rc;
+	struct buffer_head *tmp = *bh;
+
+	if (!type) /* Check inode block */
+		rc = ocfs2_read_blocks(INODE_CACHE(inode),
+				OCFS2_I(inode)->ip_blkno,
+				1, &tmp, flags,
+				ocfs2_filecheck_validate_inode_block);
+	else /* Repair inode block */
+		rc = ocfs2_read_blocks(INODE_CACHE(inode),
+				OCFS2_I(inode)->ip_blkno,
+				1, &tmp, flags,
+				ocfs2_filecheck_repair_inode_block);
+
+	/* If ocfs2_read_blocks() got us a new bh, pass it up. */
+	if (!rc && !*bh)
+		*bh = tmp;
+
+	return rc;
+}
+
 int ocfs2_read_inode_block_full(struct inode *inode, struct buffer_head **bh,
 				int flags)
 {
diff -puN fs/ocfs2/ocfs2_trace.h~ocfs2-check-fix-inode-block-for-online-file-check fs/ocfs2/ocfs2_trace.h
--- a/fs/ocfs2/ocfs2_trace.h~ocfs2-check-fix-inode-block-for-online-file-check
+++ a/fs/ocfs2/ocfs2_trace.h
@@ -1540,6 +1540,8 @@  DEFINE_OCFS2_ULL_INT_EVENT(ocfs2_read_lo
 DEFINE_OCFS2_INT_INT_EVENT(ocfs2_check_orphan_recovery_state);
 
 DEFINE_OCFS2_ULL_EVENT(ocfs2_validate_inode_block);
+DEFINE_OCFS2_ULL_EVENT(ocfs2_filecheck_validate_inode_block);
+DEFINE_OCFS2_ULL_EVENT(ocfs2_filecheck_repair_inode_block);
 
 TRACE_EVENT(ocfs2_inode_is_valid_to_delete,
 	TP_PROTO(void *task, void *dc_task, unsigned long long ino,