[PATCH][BUG][TAKE2] ext4: unify each meaning of the offset in ext4_check_dir_entry calling from some functions.

[PATCH][BUG][TAKE2] ext4: unify each meaning of the offset in ext4_check_dir_entry calling from some functions.

[Toshiyuki Okajima]

From: Toshiyuki Okajima <toshi.okajima@jp.fujitsu.com>

"offset" of the error message of ext4_check_dir_entry() might change the
meaning by the caller. There are 2 meanings:
- "File offset"
  called by:
   ext4_readdir, htree_dirblock_to_tree, search_dirblock, ext4_dx_find_entry,
   empty_dir
- "Buffer offset"
  called by:
   add_dirent_to_buf, ext4_delete_entry

The best way to solve this problem is to change the meaning of
"Buffer offset" into "File offset" but it is not easy.
However, we can solve this problem easily if we unify the meanings into
"Buffer offset". So, instead of "File Offset" meaning, we add the block number
information to this message.

--- Examples ---
Original error message:
 EXT4-fs error (device loop0): htree_dirblock_to_tree: bad entry in directory \
 #12: rec_len is too small for name_len - offset=12288, inode=216, rec_len=12,\
  name_len=11

Error message which is changed by this patch:
 EXT4-fs error (device loop0): htree_dirblock_to_tree: bad entry in directory \
 #12: rec_len is too small for name_len - block=78, offset=0, inode=216, \
 rec_len=12, name_len=11

Signed-off-by: Toshiyuki Okajima <toshi.okajima@jp.fujitsu.com>
---
 fs/ext4/dir.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index 9dc9316..65da7e5 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -84,9 +84,11 @@ int ext4_check_dir_entry(const char *function, struct inode *dir,
 
 	if (error_msg != NULL)
 		ext4_error(dir->i_sb, function,
-			"bad entry in directory #%lu: %s - "
+			"bad entry in directory #%lu: %s - block=%llu, "
 			"offset=%u, inode=%u, rec_len=%d, name_len=%d",
-			dir->i_ino, error_msg, offset,
+			dir->i_ino, error_msg, 
+			(unsigned long long)bh->b_blocknr, 
+			(unsigned)(offset%bh->b_size),
 			le32_to_cpu(de->inode),
 			rlen, de->name_len);
 	return error_msg == NULL ? 1 : 0;
-- 
1.5.5.6

Re: [PATCH][BUG][TAKE2] ext4: unify each meaning of the offset in ext4_check_dir_entry calling from some functions.

[tytso]

On Wed, Feb 03, 2010 at 04:13:48PM +0900, Toshiyuki Okajima wrote:
> From: Toshiyuki Okajima <toshi.okajima@jp.fujitsu.com>
> 
> "offset" of the error message of ext4_check_dir_entry() might change the
> meaning by the caller. There are 2 meanings:
> - "File offset"
>   called by:
>    ext4_readdir, htree_dirblock_to_tree, search_dirblock, ext4_dx_find_entry,
>    empty_dir
> - "Buffer offset"
>   called by:
>    add_dirent_to_buf, ext4_delete_entry
> 
> The best way to solve this problem is to change the meaning of
> "Buffer offset" into "File offset" but it is not easy.

It would be desirable for this to be made consistent, but a physical
block number plus the buffer offset is actually much less useful for
someone trying to find the bad directory entry.

So what I've checked into the ext4 patch queue prints both the
physical block nubmer and the relative offset, as well as the
passed-in offset which in many cases is the file offset.

	  	       	       	     	- Ted

commit e5a2233178ebf8b6538dc19a0dcf721d11590ea0
Author: Toshiyuki Okajima <toshi.okajima@jp.fujitsu.com>
Date:   Tue Mar 2 00:21:35 2010 -0500

    ext4: make "offset" consistent in ext4_check_dir_entry()
    
    The callers of ext4_check_dir_entry() usually pass in the "file
    offset" (ext4_readdir, htree_dirblock_to_tree, search_dirblock,
    ext4_dx_find_entry, empty_dir), but a few callers (add_dirent_to_buf,
    ext4_delete_entry) only pass in the buffer offset.
    
    To accomodate those last two (which would be hard to fix otherwise),
    this patch changes ext4_check_dir_entry() to print the physical block
    number and the relative offset as well as the passed-in offset.
    
    Signed-off-by: Toshiyuki Okajima <toshi.okajima@jp.fujitsu.com>
    Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index 0e0bef3..29857dd 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -84,9 +84,11 @@ int ext4_check_dir_entry(const char *function, struct inode *dir,
 
 	if (error_msg != NULL)
 		__ext4_error(dir->i_sb, function,
-			"bad entry in directory #%lu: %s - "
-			"offset=%u, inode=%u, rec_len=%d, name_len=%d",
-			dir->i_ino, error_msg, offset,
+			"bad entry in directory #%lu: %s - block=%llu"
+			"offset=%u(%u), inode=%u, rec_len=%d, name_len=%d",
+			dir->i_ino, error_msg, 
+			(unsigned long long) bh->b_blocknr,     
+			(unsigned) (offset%bh->b_size), offset,
 			le32_to_cpu(de->inode),
 			rlen, de->name_len);
 	return error_msg == NULL ? 1 : 0;