diff mbox

[v1,05/22] libext2fs: handle inline_data in block iterator function

Message ID 1375436989-18948-6-git-send-email-wenqing.lz@taobao.com
State Superseded, archived
Headers show

Commit Message

Zheng Liu Aug. 2, 2013, 9:49 a.m. UTC 
From: Zheng Liu <wenqing.lz@taobao.com>

We needn't traverse blocks for an inode which has inline data because no
block belongs to it.  After applied this patch, the following commands
in debugfs can handle inline_data feature:
	- icheck
	- blocks
	- filefrag

In some places, block iterator functions are used to traverse blocks of
directory.  In this case, we need to check whether inode has inline_data
flag or not, and handle it manually.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
---
 lib/ext2fs/block.c |    7 +++++++
 1 file changed, 7 insertions(+)

Comments

Theodore Ts'o Oct. 13, 2013, 3:55 a.m. UTC | #1 
> diff --git a/lib/ext2fs/block.c b/lib/ext2fs/block.c
> index b8c6879..b194ca8 100644
> --- a/lib/ext2fs/block.c
> +++ b/lib/ext2fs/block.c
> @@ -345,6 +345,13 @@ errcode_t ext2fs_block_iterate3(ext2_filsys fs,
>  		return ctx.errcode;
>  
>  	/*
> +	 * If an inode has inline data, we needn't traverse its blocks
> +	 * because no block belong to this inode.
> +	 */
> +	if (inode.i_flags & EXT4_INLINE_DATA_FL)
> +		return ctx.errcode;

ctx.errcode is guaranteed to be zero here.  So it would be better to
return zero explicitly --- except I wonder if we might be better to
have ext2fs_block_iterate3() return an error code if it is called on a
file that has inline data.  If we did this, then in nearly all of the
places where we call ext2fs_has_inline_data(), the call could be
obviated, and replaced with a check for the error code from
ext2fs_block_iterate3() --- which gets called in nearly every single
place where we call ext2fs_has_inline_data().

I'm not that fond of ext2fs_has_inline_data() because it doesn't
actually do that much, and it also might encourage application
programmers to use it even when they have access to inode data
structure, but instead of doing something like this:

	   if (inode.i_flags & EXT4_INLINE_DATA_FL)

they'll out of laziness, do this instead

	if (ext2fs_has_inline_data(fs, ino)

Which is less efficient, since it means an extra call to
ext2fs_read_inode()

							- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/lib/ext2fs/block.c b/lib/ext2fs/block.c
index b8c6879..b194ca8 100644
--- a/lib/ext2fs/block.c
+++ b/lib/ext2fs/block.c
@@ -345,6 +345,13 @@  errcode_t ext2fs_block_iterate3(ext2_filsys fs,
 		return ctx.errcode;
 
 	/*
+	 * If an inode has inline data, we needn't traverse its blocks
+	 * because no block belong to this inode.
+	 */
+	if (inode.i_flags & EXT4_INLINE_DATA_FL)
+		return ctx.errcode;
+
+	/*
 	 * Check to see if we need to limit large files
 	 */
 	if (flags & BLOCK_FLAG_NO_LARGE) {