Before this patch, the output woulld be(e4defrag -c -v testfile):
<File>
[ext 1]: start 33796: logical 0: len 1
[ext 2]: start 9267: logical 1: len 29
[ext 3]: start 9296: logical 30: len 1
[ext 4]: start 9297: logical 31: len 29
[ext 5]: start 9326: logical 60: len 1
[ext 6]: start 9327: logical 61: len 29
...
[ext 32]: start 9265: logical 800: len 1
[ext 33]: start 9266: logical 900: len 1
Total/best extents 33/1
Average size per extent 61 KB
Fragmentation score 51
[0-30 no problem: 31-55 a little bit fragmented: 56- needs defrag]
This file (/mnt/xfstests/scratch/test.10) does not need defragmentation.
Done.
After this patch, the output would be:
<File>
[ext 1]: start 33796: logical 0: len 1 [written]
[ext 2]: start 9267: logical 1: len 29 [unwritten]
[ext 3]: start 9296: logical 30: len 1 [written]
[ext 4]: start 9297: logical 31: len 29 [unwritten]
[ext 5]: start 9326: logical 60: len 1 [written]
[ext 6]: start 9327: logical 61: len 29 [unwritten]
...
[ext 32]: start 9265: logical 800: len 1 [unwritten]
[ext 33]: start 9266: logical 900: len 1 [unwritten]
Total/best extents 33/1
Average size per extent 61 KB
Fragmentation score 51
[0-30 no problem: 31-55 a little bit fragmented: 56- needs defrag]
This file (/mnt/xfstests/scratch/test.10) does not need defragmentation.
Done.
Signed-off-by: Xiaoguang Wang <[email protected]>
---
misc/e4defrag.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/misc/e4defrag.c b/misc/e4defrag.c
index d0eac60..b39e263 100644
--- a/misc/e4defrag.c
+++ b/misc/e4defrag.c
@@ -129,6 +129,7 @@ struct fiemap_extent_data {
__u64 len; /* blocks count */
__u64 logical; /* start logical block number */
ext4_fsblk_t physical; /* start physical block number */
+ __u32 fe_flags; /* FIEMAP_EXTENT_* flags for this extent */
};
struct fiemap_extent_list {
@@ -831,6 +832,7 @@ static int get_file_extents(int fd, struct fiemap_extent_list **ext_list_head)
/ block_size;
ext_list->data.len = ext_buf[i].fe_length
/ block_size;
+ ext_list->data.fe_flags = ext_buf[i].fe_flags;
ret = insert_extent_by_physical(
ext_list_head, ext_list);
@@ -1170,12 +1172,16 @@ static int file_statistic(const char *file, const struct stat64 *buf,
/* Print extents info */
do {
+ char *status = ext_list_tmp->data.fe_flags &
+ FIEMAP_EXTENT_UNWRITTEN ?
+ "unwritten" : "written";
count++;
printf("[ext %d]:\tstart %llu:\tlogical "
- "%llu:\tlen %llu\n", count,
- ext_list_tmp->data.physical,
- ext_list_tmp->data.logical,
- ext_list_tmp->data.len);
+ "%llu:\tlen %llu \t[%s]\n", count,
+ ext_list_tmp->data.physical,
+ ext_list_tmp->data.logical,
+ ext_list_tmp->data.len,
+ status);
ext_list_tmp = ext_list_tmp->next;
} while (ext_list_tmp != logical_list_head);
--
1.8.2.1
It would be nice if the output would match the format used by filefrag.
Cheers, Andreas
> On Sep 23, 2014, at 12:41, Xiaoguang Wang <[email protected]> wrote:
>
> Before this patch, the output woulld be(e4defrag -c -v testfile):
> <File>
> [ext 1]: start 33796: logical 0: len 1
> [ext 2]: start 9267: logical 1: len 29
> [ext 3]: start 9296: logical 30: len 1
> [ext 4]: start 9297: logical 31: len 29
> [ext 5]: start 9326: logical 60: len 1
> [ext 6]: start 9327: logical 61: len 29
> ...
> [ext 32]: start 9265: logical 800: len 1
> [ext 33]: start 9266: logical 900: len 1
>
> Total/best extents 33/1
> Average size per extent 61 KB
> Fragmentation score 51
> [0-30 no problem: 31-55 a little bit fragmented: 56- needs defrag]
> This file (/mnt/xfstests/scratch/test.10) does not need defragmentation.
> Done.
>
> After this patch, the output would be:
> <File>
> [ext 1]: start 33796: logical 0: len 1 [written]
> [ext 2]: start 9267: logical 1: len 29 [unwritten]
> [ext 3]: start 9296: logical 30: len 1 [written]
> [ext 4]: start 9297: logical 31: len 29 [unwritten]
> [ext 5]: start 9326: logical 60: len 1 [written]
> [ext 6]: start 9327: logical 61: len 29 [unwritten]
> ...
> [ext 32]: start 9265: logical 800: len 1 [unwritten]
> [ext 33]: start 9266: logical 900: len 1 [unwritten]
>
> Total/best extents 33/1
> Average size per extent 61 KB
> Fragmentation score 51
> [0-30 no problem: 31-55 a little bit fragmented: 56- needs defrag]
> This file (/mnt/xfstests/scratch/test.10) does not need defragmentation.
> Done.
>
> Signed-off-by: Xiaoguang Wang <[email protected]>
> ---
> misc/e4defrag.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/misc/e4defrag.c b/misc/e4defrag.c
> index d0eac60..b39e263 100644
> --- a/misc/e4defrag.c
> +++ b/misc/e4defrag.c
> @@ -129,6 +129,7 @@ struct fiemap_extent_data {
> __u64 len; /* blocks count */
> __u64 logical; /* start logical block number */
> ext4_fsblk_t physical; /* start physical block number */
> + __u32 fe_flags; /* FIEMAP_EXTENT_* flags for this extent */
> };
>
> struct fiemap_extent_list {
> @@ -831,6 +832,7 @@ static int get_file_extents(int fd, struct fiemap_extent_list **ext_list_head)
> / block_size;
> ext_list->data.len = ext_buf[i].fe_length
> / block_size;
> + ext_list->data.fe_flags = ext_buf[i].fe_flags;
>
> ret = insert_extent_by_physical(
> ext_list_head, ext_list);
> @@ -1170,12 +1172,16 @@ static int file_statistic(const char *file, const struct stat64 *buf,
>
> /* Print extents info */
> do {
> + char *status = ext_list_tmp->data.fe_flags &
> + FIEMAP_EXTENT_UNWRITTEN ?
> + "unwritten" : "written";
> count++;
> printf("[ext %d]:\tstart %llu:\tlogical "
> - "%llu:\tlen %llu\n", count,
> - ext_list_tmp->data.physical,
> - ext_list_tmp->data.logical,
> - ext_list_tmp->data.len);
> + "%llu:\tlen %llu \t[%s]\n", count,
> + ext_list_tmp->data.physical,
> + ext_list_tmp->data.logical,
> + ext_list_tmp->data.len,
> + status);
> ext_list_tmp = ext_list_tmp->next;
> } while (ext_list_tmp != logical_list_head);
>
> --
> 1.8.2.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi,
On 09/24/2014 03:02 AM, Andreas Dilger wrote:
> It would be nice if the output would match the format used by filefrag.
OK, will send a V2 version, thanks!
Regards,
Xiaoguang Wang
>
> Cheers, Andreas
>
>> On Sep 23, 2014, at 12:41, Xiaoguang Wang <[email protected]> wrote:
>>
>> Before this patch, the output woulld be(e4defrag -c -v testfile):
>> <File>
>> [ext 1]: start 33796: logical 0: len 1
>> [ext 2]: start 9267: logical 1: len 29
>> [ext 3]: start 9296: logical 30: len 1
>> [ext 4]: start 9297: logical 31: len 29
>> [ext 5]: start 9326: logical 60: len 1
>> [ext 6]: start 9327: logical 61: len 29
>> ...
>> [ext 32]: start 9265: logical 800: len 1
>> [ext 33]: start 9266: logical 900: len 1
>>
>> Total/best extents 33/1
>> Average size per extent 61 KB
>> Fragmentation score 51
>> [0-30 no problem: 31-55 a little bit fragmented: 56- needs defrag]
>> This file (/mnt/xfstests/scratch/test.10) does not need defragmentation.
>> Done.
>>
>> After this patch, the output would be:
>> <File>
>> [ext 1]: start 33796: logical 0: len 1 [written]
>> [ext 2]: start 9267: logical 1: len 29 [unwritten]
>> [ext 3]: start 9296: logical 30: len 1 [written]
>> [ext 4]: start 9297: logical 31: len 29 [unwritten]
>> [ext 5]: start 9326: logical 60: len 1 [written]
>> [ext 6]: start 9327: logical 61: len 29 [unwritten]
>> ...
>> [ext 32]: start 9265: logical 800: len 1 [unwritten]
>> [ext 33]: start 9266: logical 900: len 1 [unwritten]
>>
>> Total/best extents 33/1
>> Average size per extent 61 KB
>> Fragmentation score 51
>> [0-30 no problem: 31-55 a little bit fragmented: 56- needs defrag]
>> This file (/mnt/xfstests/scratch/test.10) does not need defragmentation.
>> Done.
>>
>> Signed-off-by: Xiaoguang Wang <[email protected]>
>> ---
>> misc/e4defrag.c | 14 ++++++++++----
>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/misc/e4defrag.c b/misc/e4defrag.c
>> index d0eac60..b39e263 100644
>> --- a/misc/e4defrag.c
>> +++ b/misc/e4defrag.c
>> @@ -129,6 +129,7 @@ struct fiemap_extent_data {
>> __u64 len; /* blocks count */
>> __u64 logical; /* start logical block number */
>> ext4_fsblk_t physical; /* start physical block number */
>> + __u32 fe_flags; /* FIEMAP_EXTENT_* flags for this extent */
>> };
>>
>> struct fiemap_extent_list {
>> @@ -831,6 +832,7 @@ static int get_file_extents(int fd, struct fiemap_extent_list **ext_list_head)
>> / block_size;
>> ext_list->data.len = ext_buf[i].fe_length
>> / block_size;
>> + ext_list->data.fe_flags = ext_buf[i].fe_flags;
>>
>> ret = insert_extent_by_physical(
>> ext_list_head, ext_list);
>> @@ -1170,12 +1172,16 @@ static int file_statistic(const char *file, const struct stat64 *buf,
>>
>> /* Print extents info */
>> do {
>> + char *status = ext_list_tmp->data.fe_flags &
>> + FIEMAP_EXTENT_UNWRITTEN ?
>> + "unwritten" : "written";
>> count++;
>> printf("[ext %d]:\tstart %llu:\tlogical "
>> - "%llu:\tlen %llu\n", count,
>> - ext_list_tmp->data.physical,
>> - ext_list_tmp->data.logical,
>> - ext_list_tmp->data.len);
>> + "%llu:\tlen %llu \t[%s]\n", count,
>> + ext_list_tmp->data.physical,
>> + ext_list_tmp->data.logical,
>> + ext_list_tmp->data.len,
>> + status);
>> ext_list_tmp = ext_list_tmp->next;
>> } while (ext_list_tmp != logical_list_head);
>>
>> --
>> 1.8.2.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> .
>
On 9/23/14 8:11 PM, Xiaoguang Wang wrote:
> Hi,
>
> On 09/24/2014 03:02 AM, Andreas Dilger wrote:
>> It would be nice if the output would match the format used by filefrag.
> OK, will send a V2 version, thanks!
And could they share code? I seem to remember that it's taken several
iterations to get filefrag working properly, or some reason.
-Eric
> Regards,
> Xiaoguang Wang
>
Hi,
On 09/24/2014 09:44 PM, Eric Sandeen wrote:
> On 9/23/14 8:11 PM, Xiaoguang Wang wrote:
>> Hi,
>>
>> On 09/24/2014 03:02 AM, Andreas Dilger wrote:
>>> It would be nice if the output would match the format used by filefrag.
>> OK, will send a V2 version, thanks!
>
> And could they share code? I seem to remember that it's taken several
> iterations to get filefrag working properly, or some reason.
Sorry for much late response, I was away from the internet for some time.
Yes, we can pull some shared code together, but I found filefrag couldn't
work correctly when using FIBMAP, I'd like to fix it first. Please check
my next mail that fixes this bug, thanks!
Regards,
Xiaoguang Wang
>
> -Eric
>
>
>> Regards,
>> Xiaoguang Wang
>>
>
> .
>
When using FIBMAP and '-e' option is specified, the calculation for fiemap_extent
is wrong, we wrongly updated fm_ext.fe_logical for every iteration, please see the
code in the end of 'for' loop in fm_ext.fe_logical().
In an ext2 file system(block size is 1024 bytes),
dd if=/dev/zero of=testfile bs=1k count=15
Using debugfs, corresponding physical blocks are "2049 2050 2051 2052 2053 2054
2055 2056 2057 2058 2059 2060 1025 2061 2062 2063", 1025 is this indirect block.
Before this patch, filefrag's output would be:
filefrag -B -e testfile
Filesystem type is: ef53
Filesystem cylinder groups approximately 16
File size of testfile is 15360 (15 blocks of 1024 bytes)
ext: logical_offset: physical_offset: length: expected: flags:
0: 1.. 2: 2050.. 2051: 2: 2051: merged
1: 3.. 4: 2052.. 2053: 2: 2053: merged
2: 5.. 6: 2054.. 2055: 2: 2055: merged
3: 7.. 8: 2056.. 2057: 2: 2057: merged
4: 9.. 10: 2058.. 2059: 2: 2059: merged
5: 11.. 12: 2060.. 2061: 2: 2062: merged
6: 13.. 14: 2062.. 2063: 2: 2063: merged,eof
7: 14.. 14: 2063.. 2063: 1: 2063: merged,eof
This output is not reasonable.
Fix this bug and try to make it readable. After this patch, the output would be:
./filefrag -B -e mntpoint/testfile
Filesystem type is: ef53
Filesystem cylinder groups approximately 16
File size of mntpoint/testfile is 15360 (15 blocks of 1024 bytes)
ext: logical_offset: physical_offset: length: expected: flags:
0: 0.. 11: 2049.. 2060: 12: 2062: merged
1: 12.. 14: 2061.. 2063: 3: 2063: merged,eof
mntpoint/testfile: 2 extents found, perfection would be 1 extent
Signed-off-by: Xiaoguang Wang <[email protected]>
---
misc/filefrag.c | 37 +++++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/misc/filefrag.c b/misc/filefrag.c
index c1a8684..e9f7e68 100644
--- a/misc/filefrag.c
+++ b/misc/filefrag.c
@@ -315,32 +315,41 @@ static int filefrag_fibmap(int fd, int blk_shift, int *num_extents,
return rc;
if (block == 0)
continue;
+ count++;
+
if (*num_extents == 0) {
(*num_extents)++;
if (force_extent) {
print_extent_header();
+ fm_ext.fe_logical = logical;
fm_ext.fe_physical = block * st->st_blksize;
+ fm_ext.fe_length = st->st_blksize;
}
+ last_block = block;
+ continue;
}
- count++;
- if (force_extent && last_block != 0 &&
- (block != last_block + 1 ||
- fm_ext.fe_logical + fm_ext.fe_length != logical)) {
- print_extent_info(&fm_ext, *num_extents - 1,
- (last_block + 1) * st->st_blksize,
- blk_shift, st);
- fm_ext.fe_length = 0;
- (*num_extents)++;
+
+ if (force_extent) {
+ if (block != last_block + 1 ||
+ fm_ext.fe_length + fm_ext.fe_logical != logical) {
+ print_extent_info(&fm_ext, *num_extents - 1,
+ (last_block + 1) *
+ st->st_blksize,
+ blk_shift, st);
+ fm_ext.fe_logical = logical;
+ fm_ext.fe_physical = block * st->st_blksize;
+ fm_ext.fe_length = st->st_blksize;
+ (*num_extents)++;
+ } else {
+ fm_ext.fe_length += st->st_blksize;
+ }
} else if (last_block && (block != last_block + 1)) {
- if (verbose)
+ if (verbose) {
printf("Discontinuity: Block %ld is at %lu (was "
"%lu)\n", i, block, last_block + 1);
- fm_ext.fe_length = 0;
+ }
(*num_extents)++;
}
- fm_ext.fe_logical = logical;
- fm_ext.fe_physical = block * st->st_blksize;
- fm_ext.fe_length += st->st_blksize;
last_block = block;
}
--
1.8.2.1
On Oct 6, 2014, at 5:05 AM, Xiaoguang Wang <[email protected]> wrote:
> When using FIBMAP and '-e' option is specified, the calculation for
> fiemap_extent is wrong, we wrongly updated fm_ext.fe_logical for every iteration, please see the code in the end of 'for' loop in
> fm_ext.fe_logical().
>
> In an ext2 file system(block size is 1024 bytes),
> dd if=/dev/zero of=testfile bs=1k count=15
> Using debugfs, corresponding physical blocks are "2049 2050 2051 2052 2053 2054
> 2055 2056 2057 2058 2059 2060 1025 2061 2062 2063", 1025 is this indirect block.
> Before this patch, filefrag's output would be:
> filefrag -B -e testfile
> Filesystem type is: ef53
> Filesystem cylinder groups approximately 16
> File size of testfile is 15360 (15 blocks of 1024 bytes)
> ext: logical_offset: physical_offset: length: expected: flags:
> 0: 1.. 2: 2050.. 2051: 2: 2051: merged
> 1: 3.. 4: 2052.. 2053: 2: 2053: merged
> 2: 5.. 6: 2054.. 2055: 2: 2055: merged
> 3: 7.. 8: 2056.. 2057: 2: 2057: merged
> 4: 9.. 10: 2058.. 2059: 2: 2059: merged
> 5: 11.. 12: 2060.. 2061: 2: 2062: merged
> 6: 13.. 14: 2062.. 2063: 2: 2063: merged,eof
> 7: 14.. 14: 2063.. 2063: 1: 2063: merged,eof
> This output is not reasonable.
Definitely agree.
> Fix this bug and try to make it readable. After this patch, the output would be:
> ./filefrag -B -e mntpoint/testfile
> Filesystem type is: ef53
> Filesystem cylinder groups approximately 16
> File size of mntpoint/testfile is 15360 (15 blocks of 1024 bytes)
> ext: logical_offset: physical_offset: length: expected: flags:
> 0: 0.. 11: 2049.. 2060: 12: 2062: merged
> 1: 12.. 14: 2061.. 2063: 3: 2063: merged,eof
> mntpoint/testfile: 2 extents found, perfection would be 1 extent
Why would this not be shown as a single extent:
0: 0..14: 2049..2063: 15: merged,eof
It isn't clear why "expected" is 2062 instead of 2061.
Cheers, Andreas
> Signed-off-by: Xiaoguang Wang <[email protected]>
> ---
> misc/filefrag.c | 37 +++++++++++++++++++++++--------------
> 1 file changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/misc/filefrag.c b/misc/filefrag.c
> index c1a8684..e9f7e68 100644
> --- a/misc/filefrag.c
> +++ b/misc/filefrag.c
> @@ -315,32 +315,41 @@ static int filefrag_fibmap(int fd, int blk_shift, int *num_extents,
> return rc;
> if (block == 0)
> continue;
> + count++;
> +
> if (*num_extents == 0) {
> (*num_extents)++;
> if (force_extent) {
> print_extent_header();
> + fm_ext.fe_logical = logical;
> fm_ext.fe_physical = block * st->st_blksize;
> + fm_ext.fe_length = st->st_blksize;
> }
> + last_block = block;
> + continue;
> }
> - count++;
> - if (force_extent && last_block != 0 &&
> - (block != last_block + 1 ||
> - fm_ext.fe_logical + fm_ext.fe_length != logical)) {
> - print_extent_info(&fm_ext, *num_extents - 1,
> - (last_block + 1) * st->st_blksize,
> - blk_shift, st);
> - fm_ext.fe_length = 0;
> - (*num_extents)++;
> +
> + if (force_extent) {
> + if (block != last_block + 1 ||
> + fm_ext.fe_length + fm_ext.fe_logical != logical) {
> + print_extent_info(&fm_ext, *num_extents - 1,
> + (last_block + 1) *
> + st->st_blksize,
> + blk_shift, st);
> + fm_ext.fe_logical = logical;
> + fm_ext.fe_physical = block * st->st_blksize;
> + fm_ext.fe_length = st->st_blksize;
> + (*num_extents)++;
> + } else {
> + fm_ext.fe_length += st->st_blksize;
> + }
> } else if (last_block && (block != last_block + 1)) {
> - if (verbose)
> + if (verbose) {
> printf("Discontinuity: Block %ld is at %lu (was "
> "%lu)\n", i, block, last_block + 1);
> - fm_ext.fe_length = 0;
> + }
> (*num_extents)++;
> }
> - fm_ext.fe_logical = logical;
> - fm_ext.fe_physical = block * st->st_blksize;
> - fm_ext.fe_length += st->st_blksize;
> last_block = block;
> }
>
> --
> 1.8.2.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers, Andreas
Hi,
On 10/07/2014 06:29 AM, Andreas Dilger wrote:
> On Oct 6, 2014, at 5:05 AM, Xiaoguang Wang <[email protected]> wrote:
>> When using FIBMAP and '-e' option is specified, the calculation for
>> fiemap_extent is wrong, we wrongly updated fm_ext.fe_logical for every iteration, please see the code in the end of 'for' loop in
>> fm_ext.fe_logical().
>>
>> In an ext2 file system(block size is 1024 bytes),
>> dd if=/dev/zero of=testfile bs=1k count=15
>> Using debugfs, corresponding physical blocks are "2049 2050 2051 2052 2053 2054
>> 2055 2056 2057 2058 2059 2060 1025 2061 2062 2063", 1025 is this indirect block.
>> Before this patch, filefrag's output would be:
>> filefrag -B -e testfile
>> Filesystem type is: ef53
>> Filesystem cylinder groups approximately 16
>> File size of testfile is 15360 (15 blocks of 1024 bytes)
>> ext: logical_offset: physical_offset: length: expected: flags:
>> 0: 1.. 2: 2050.. 2051: 2: 2051: merged
>> 1: 3.. 4: 2052.. 2053: 2: 2053: merged
>> 2: 5.. 6: 2054.. 2055: 2: 2055: merged
>> 3: 7.. 8: 2056.. 2057: 2: 2057: merged
>> 4: 9.. 10: 2058.. 2059: 2: 2059: merged
>> 5: 11.. 12: 2060.. 2061: 2: 2062: merged
>> 6: 13.. 14: 2062.. 2063: 2: 2063: merged,eof
>> 7: 14.. 14: 2063.. 2063: 1: 2063: merged,eof
>> This output is not reasonable.
>
> Definitely agree.
>
>> Fix this bug and try to make it readable. After this patch, the output would be:
>> ./filefrag -B -e mntpoint/testfile
>> Filesystem type is: ef53
>> Filesystem cylinder groups approximately 16
>> File size of mntpoint/testfile is 15360 (15 blocks of 1024 bytes)
>> ext: logical_offset: physical_offset: length: expected: flags:
>> 0: 0.. 11: 2049.. 2060: 12: 2062: merged
>> 1: 12.. 14: 2061.. 2063: 3: 2063: merged,eof
>> mntpoint/testfile: 2 extents found, perfection would be 1 extent
>
> Why would this not be shown as a single extent:
>
> 0: 0..14: 2049..2063: 15: merged,eof
>
> It isn't clear why "expected" is 2062 instead of 2061.
This is because for a logical region, only all corresponding physical data blocks and
possible indirect blocks are both continuous, we can say that it's an extent. There is such
code in filefrag_fibmap():
if (is_ext2 && last_block) {
if (((i - EXT2_DIRECT) % bpib) == 0)
last_block++;
if (((i - EXT2_DIRECT - bpib) % (bpib * bpib)) == 0)
last_block++;
if (((i - EXT2_DIRECT - bpib - bpib * bpib) %
(((unsigned long long)bpib) * bpib * bpib)) == 0)
last_block++;
}
For logical block 12, it starts to need an indirect block, and its expected physical
block is 2062(expected indirect block is 2061), so we do not consider it continuous.
Regards,
Xiaoguang Wang
>
> Cheers, Andreas
>
>> Signed-off-by: Xiaoguang Wang <[email protected]>
>> ---
>> misc/filefrag.c | 37 +++++++++++++++++++++++--------------
>> 1 file changed, 23 insertions(+), 14 deletions(-)
>>
>> diff --git a/misc/filefrag.c b/misc/filefrag.c
>> index c1a8684..e9f7e68 100644
>> --- a/misc/filefrag.c
>> +++ b/misc/filefrag.c
>> @@ -315,32 +315,41 @@ static int filefrag_fibmap(int fd, int blk_shift, int *num_extents,
>> return rc;
>> if (block == 0)
>> continue;
>> + count++;
>> +
>> if (*num_extents == 0) {
>> (*num_extents)++;
>> if (force_extent) {
>> print_extent_header();
>> + fm_ext.fe_logical = logical;
>> fm_ext.fe_physical = block * st->st_blksize;
>> + fm_ext.fe_length = st->st_blksize;
>> }
>> + last_block = block;
>> + continue;
>> }
>> - count++;
>> - if (force_extent && last_block != 0 &&
>> - (block != last_block + 1 ||
>> - fm_ext.fe_logical + fm_ext.fe_length != logical)) {
>> - print_extent_info(&fm_ext, *num_extents - 1,
>> - (last_block + 1) * st->st_blksize,
>> - blk_shift, st);
>> - fm_ext.fe_length = 0;
>> - (*num_extents)++;
>> +
>> + if (force_extent) {
>> + if (block != last_block + 1 ||
>> + fm_ext.fe_length + fm_ext.fe_logical != logical) {
>> + print_extent_info(&fm_ext, *num_extents - 1,
>> + (last_block + 1) *
>> + st->st_blksize,
>> + blk_shift, st);
>> + fm_ext.fe_logical = logical;
>> + fm_ext.fe_physical = block * st->st_blksize;
>> + fm_ext.fe_length = st->st_blksize;
>> + (*num_extents)++;
>> + } else {
>> + fm_ext.fe_length += st->st_blksize;
>> + }
>> } else if (last_block && (block != last_block + 1)) {
>> - if (verbose)
>> + if (verbose) {
>> printf("Discontinuity: Block %ld is at %lu (was "
>> "%lu)\n", i, block, last_block + 1);
>> - fm_ext.fe_length = 0;
>> + }
>> (*num_extents)++;
>> }
>> - fm_ext.fe_logical = logical;
>> - fm_ext.fe_physical = block * st->st_blksize;
>> - fm_ext.fe_length += st->st_blksize;
>> last_block = block;
>> }
>>
>> --
>> 1.8.2.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
> Cheers, Andreas
>
>
>
>
>
On Mon, Oct 06, 2014 at 07:05:39PM +0800, Xiaoguang Wang wrote:
> When using FIBMAP and '-e' option is specified, the calculation for fiemap_extent
> is wrong, we wrongly updated fm_ext.fe_logical for every iteration, please see the
> code in the end of 'for' loop in fm_ext.fe_logical().
>
> In an ext2 file system(block size is 1024 bytes),
> dd if=/dev/zero of=testfile bs=1k count=15
> Using debugfs, corresponding physical blocks are "2049 2050 2051 2052 2053 2054
> 2055 2056 2057 2058 2059 2060 1025 2061 2062 2063", 1025 is this indirect block.
> Before this patch, filefrag's output would be:
> filefrag -B -e testfile
> Filesystem type is: ef53
> Filesystem cylinder groups approximately 16
> File size of testfile is 15360 (15 blocks of 1024 bytes)
> ext: logical_offset: physical_offset: length: expected: flags:
> 0: 1.. 2: 2050.. 2051: 2: 2051: merged
> 1: 3.. 4: 2052.. 2053: 2: 2053: merged
> 2: 5.. 6: 2054.. 2055: 2: 2055: merged
> 3: 7.. 8: 2056.. 2057: 2: 2057: merged
> 4: 9.. 10: 2058.. 2059: 2: 2059: merged
> 5: 11.. 12: 2060.. 2061: 2: 2062: merged
> 6: 13.. 14: 2062.. 2063: 2: 2063: merged,eof
> 7: 14.. 14: 2063.. 2063: 1: 2063: merged,eof
> This output is not reasonable.
It's just plain whacky. Why would logical offset 14 be listed twice? :)
> Fix this bug and try to make it readable. After this patch, the output would be:
> ./filefrag -B -e mntpoint/testfile
> Filesystem type is: ef53
> Filesystem cylinder groups approximately 16
> File size of mntpoint/testfile is 15360 (15 blocks of 1024 bytes)
> ext: logical_offset: physical_offset: length: expected: flags:
> 0: 0.. 11: 2049.. 2060: 12: 2062: merged
> 1: 12.. 14: 2061.. 2063: 3: 2063: merged,eof
> mntpoint/testfile: 2 extents found, perfection would be 1 extent
Where'd the indirect block end up, if not in the middle of 2049-2063?
> Signed-off-by: Xiaoguang Wang <[email protected]>
> ---
> misc/filefrag.c | 37 +++++++++++++++++++++++--------------
> 1 file changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/misc/filefrag.c b/misc/filefrag.c
> index c1a8684..e9f7e68 100644
> --- a/misc/filefrag.c
> +++ b/misc/filefrag.c
> @@ -315,32 +315,41 @@ static int filefrag_fibmap(int fd, int blk_shift, int *num_extents,
> return rc;
> if (block == 0)
> continue;
> + count++;
> +
> if (*num_extents == 0) {
> (*num_extents)++;
> if (force_extent) {
> print_extent_header();
> + fm_ext.fe_logical = logical;
> fm_ext.fe_physical = block * st->st_blksize;
> + fm_ext.fe_length = st->st_blksize;
> }
> + last_block = block;
> + continue;
> }
> - count++;
> - if (force_extent && last_block != 0 &&
> - (block != last_block + 1 ||
> - fm_ext.fe_logical + fm_ext.fe_length != logical)) {
> - print_extent_info(&fm_ext, *num_extents - 1,
> - (last_block + 1) * st->st_blksize,
> - blk_shift, st);
> - fm_ext.fe_length = 0;
> - (*num_extents)++;
> +
> + if (force_extent) {
> + if (block != last_block + 1 ||
> + fm_ext.fe_length + fm_ext.fe_logical != logical) {
> + print_extent_info(&fm_ext, *num_extents - 1,
> + (last_block + 1) *
> + st->st_blksize,
> + blk_shift, st);
> + fm_ext.fe_logical = logical;
> + fm_ext.fe_physical = block * st->st_blksize;
> + fm_ext.fe_length = st->st_blksize;
> + (*num_extents)++;
> + } else {
> + fm_ext.fe_length += st->st_blksize;
> + }
> } else if (last_block && (block != last_block + 1)) {
> - if (verbose)
> + if (verbose) {
> printf("Discontinuity: Block %ld is at %lu (was "
> "%lu)\n", i, block, last_block + 1);
> - fm_ext.fe_length = 0;
> + }
The { } is not needed for a single line if statement.
> (*num_extents)++;
> }
> - fm_ext.fe_logical = logical;
> - fm_ext.fe_physical = block * st->st_blksize;
> - fm_ext.fe_length += st->st_blksize;
> last_block = block;
Otherwise, I think this looks decent.
--D
> }
>
> --
> 1.8.2.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi,
On 10/07/2014 10:43 AM, Darrick J. Wong wrote:
> On Mon, Oct 06, 2014 at 07:05:39PM +0800, Xiaoguang Wang wrote:
>> When using FIBMAP and '-e' option is specified, the calculation for fiemap_extent
>> is wrong, we wrongly updated fm_ext.fe_logical for every iteration, please see the
>> code in the end of 'for' loop in fm_ext.fe_logical().
>>
>> In an ext2 file system(block size is 1024 bytes),
>> dd if=/dev/zero of=testfile bs=1k count=15
>> Using debugfs, corresponding physical blocks are "2049 2050 2051 2052 2053 2054
>> 2055 2056 2057 2058 2059 2060 1025 2061 2062 2063", 1025 is this indirect block.
>> Before this patch, filefrag's output would be:
>> filefrag -B -e testfile
>> Filesystem type is: ef53
>> Filesystem cylinder groups approximately 16
>> File size of testfile is 15360 (15 blocks of 1024 bytes)
>> ext: logical_offset: physical_offset: length: expected: flags:
>> 0: 1.. 2: 2050.. 2051: 2: 2051: merged
>> 1: 3.. 4: 2052.. 2053: 2: 2053: merged
>> 2: 5.. 6: 2054.. 2055: 2: 2055: merged
>> 3: 7.. 8: 2056.. 2057: 2: 2057: merged
>> 4: 9.. 10: 2058.. 2059: 2: 2059: merged
>> 5: 11.. 12: 2060.. 2061: 2: 2062: merged
>> 6: 13.. 14: 2062.. 2063: 2: 2063: merged,eof
>> 7: 14.. 14: 2063.. 2063: 1: 2063: merged,eof
>> This output is not reasonable.
>
> It's just plain whacky. Why would logical offset 14 be listed twice? :)
Because of the wrong code :) It updates fm_ext.fe_logical and fm_ext.fe_physical for every
iteration :) and i think the original code is not that readable.
>
>> Fix this bug and try to make it readable. After this patch, the output would be:
>> ./filefrag -B -e mntpoint/testfile
>> Filesystem type is: ef53
>> Filesystem cylinder groups approximately 16
>> File size of mntpoint/testfile is 15360 (15 blocks of 1024 bytes)
>> ext: logical_offset: physical_offset: length: expected: flags:
>> 0: 0.. 11: 2049.. 2060: 12: 2062: merged
>> 1: 12.. 14: 2061.. 2063: 3: 2063: merged,eof
>> mntpoint/testfile: 2 extents found, perfection would be 1 extent
>
> Where'd the indirect block end up, if not in the middle of 2049-2063?
Indirect block's information is used to judge whether logical region is continuous physically.
For example, the above testfile's first indirect block is 1025, and will not shown in the output.
>
>> Signed-off-by: Xiaoguang Wang <[email protected]>
>> ---
>> misc/filefrag.c | 37 +++++++++++++++++++++++--------------
>> 1 file changed, 23 insertions(+), 14 deletions(-)
>>
>> diff --git a/misc/filefrag.c b/misc/filefrag.c
>> index c1a8684..e9f7e68 100644
>> --- a/misc/filefrag.c
>> +++ b/misc/filefrag.c
>> @@ -315,32 +315,41 @@ static int filefrag_fibmap(int fd, int blk_shift, int *num_extents,
>> return rc;
>> if (block == 0)
>> continue;
>> + count++;
>> +
>> if (*num_extents == 0) {
>> (*num_extents)++;
>> if (force_extent) {
>> print_extent_header();
>> + fm_ext.fe_logical = logical;
>> fm_ext.fe_physical = block * st->st_blksize;
>> + fm_ext.fe_length = st->st_blksize;
>> }
>> + last_block = block;
>> + continue;
>> }
>> - count++;
>> - if (force_extent && last_block != 0 &&
>> - (block != last_block + 1 ||
>> - fm_ext.fe_logical + fm_ext.fe_length != logical)) {
>> - print_extent_info(&fm_ext, *num_extents - 1,
>> - (last_block + 1) * st->st_blksize,
>> - blk_shift, st);
>> - fm_ext.fe_length = 0;
>> - (*num_extents)++;
>> +
>> + if (force_extent) {
>> + if (block != last_block + 1 ||
>> + fm_ext.fe_length + fm_ext.fe_logical != logical) {
>> + print_extent_info(&fm_ext, *num_extents - 1,
>> + (last_block + 1) *
>> + st->st_blksize,
>> + blk_shift, st);
>> + fm_ext.fe_logical = logical;
>> + fm_ext.fe_physical = block * st->st_blksize;
>> + fm_ext.fe_length = st->st_blksize;
>> + (*num_extents)++;
>> + } else {
>> + fm_ext.fe_length += st->st_blksize;
>> + }
>> } else if (last_block && (block != last_block + 1)) {
>> - if (verbose)
>> + if (verbose) {
>> printf("Discontinuity: Block %ld is at %lu (was "
>> "%lu)\n", i, block, last_block + 1);
>> - fm_ext.fe_length = 0;
>> + }
>
> The { } is not needed for a single line if statement.
OK, I'll send a new version to remove it, thanks!
Regards,
Xiaoguang Wang
>
>> (*num_extents)++;
>> }
>> - fm_ext.fe_logical = logical;
>> - fm_ext.fe_physical = block * st->st_blksize;
>> - fm_ext.fe_length += st->st_blksize;
>> last_block = block;
>
> Otherwise, I think this looks decent.
>
> --D
>
>> }
>>
>> --
>> 1.8.2.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> .
>
On Tue, Oct 07, 2014 at 11:50:34AM +0800, Xiaoguang Wang wrote:
> Hi,
>
> On 10/07/2014 10:43 AM, Darrick J. Wong wrote:
> > On Mon, Oct 06, 2014 at 07:05:39PM +0800, Xiaoguang Wang wrote:
> >> When using FIBMAP and '-e' option is specified, the calculation for fiemap_extent
> >> is wrong, we wrongly updated fm_ext.fe_logical for every iteration, please see the
> >> code in the end of 'for' loop in fm_ext.fe_logical().
> >>
> >> In an ext2 file system(block size is 1024 bytes),
> >> dd if=/dev/zero of=testfile bs=1k count=15
> >> Using debugfs, corresponding physical blocks are "2049 2050 2051 2052 2053 2054
> >> 2055 2056 2057 2058 2059 2060 1025 2061 2062 2063", 1025 is this indirect block.
> >> Before this patch, filefrag's output would be:
> >> filefrag -B -e testfile
> >> Filesystem type is: ef53
> >> Filesystem cylinder groups approximately 16
> >> File size of testfile is 15360 (15 blocks of 1024 bytes)
> >> ext: logical_offset: physical_offset: length: expected: flags:
> >> 0: 1.. 2: 2050.. 2051: 2: 2051: merged
> >> 1: 3.. 4: 2052.. 2053: 2: 2053: merged
> >> 2: 5.. 6: 2054.. 2055: 2: 2055: merged
> >> 3: 7.. 8: 2056.. 2057: 2: 2057: merged
> >> 4: 9.. 10: 2058.. 2059: 2: 2059: merged
> >> 5: 11.. 12: 2060.. 2061: 2: 2062: merged
> >> 6: 13.. 14: 2062.. 2063: 2: 2063: merged,eof
> >> 7: 14.. 14: 2063.. 2063: 1: 2063: merged,eof
> >> This output is not reasonable.
> >
> > It's just plain whacky. Why would logical offset 14 be listed twice? :)
>
> Because of the wrong code :) It updates fm_ext.fe_logical and fm_ext.fe_physical for every
> iteration :) and i think the original code is not that readable.
> >
> >> Fix this bug and try to make it readable. After this patch, the output would be:
> >> ./filefrag -B -e mntpoint/testfile
> >> Filesystem type is: ef53
> >> Filesystem cylinder groups approximately 16
> >> File size of mntpoint/testfile is 15360 (15 blocks of 1024 bytes)
> >> ext: logical_offset: physical_offset: length: expected: flags:
> >> 0: 0.. 11: 2049.. 2060: 12: 2062: merged
> >> 1: 12.. 14: 2061.. 2063: 3: 2063: merged,eof
> >> mntpoint/testfile: 2 extents found, perfection would be 1 extent
> >
> > Where'd the indirect block end up, if not in the middle of 2049-2063?
>
> Indirect block's information is used to judge whether logical region is continuous physically.
> For example, the above testfile's first indirect block is 1025, and will not shown in the output.
1025? Huh. Is this an old FS, or is there something wrong with the block
allocator?
--D
>
> >
> >> Signed-off-by: Xiaoguang Wang <[email protected]>
> >> ---
> >> misc/filefrag.c | 37 +++++++++++++++++++++++--------------
> >> 1 file changed, 23 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/misc/filefrag.c b/misc/filefrag.c
> >> index c1a8684..e9f7e68 100644
> >> --- a/misc/filefrag.c
> >> +++ b/misc/filefrag.c
> >> @@ -315,32 +315,41 @@ static int filefrag_fibmap(int fd, int blk_shift, int *num_extents,
> >> return rc;
> >> if (block == 0)
> >> continue;
> >> + count++;
> >> +
> >> if (*num_extents == 0) {
> >> (*num_extents)++;
> >> if (force_extent) {
> >> print_extent_header();
> >> + fm_ext.fe_logical = logical;
> >> fm_ext.fe_physical = block * st->st_blksize;
> >> + fm_ext.fe_length = st->st_blksize;
> >> }
> >> + last_block = block;
> >> + continue;
> >> }
> >> - count++;
> >> - if (force_extent && last_block != 0 &&
> >> - (block != last_block + 1 ||
> >> - fm_ext.fe_logical + fm_ext.fe_length != logical)) {
> >> - print_extent_info(&fm_ext, *num_extents - 1,
> >> - (last_block + 1) * st->st_blksize,
> >> - blk_shift, st);
> >> - fm_ext.fe_length = 0;
> >> - (*num_extents)++;
> >> +
> >> + if (force_extent) {
> >> + if (block != last_block + 1 ||
> >> + fm_ext.fe_length + fm_ext.fe_logical != logical) {
> >> + print_extent_info(&fm_ext, *num_extents - 1,
> >> + (last_block + 1) *
> >> + st->st_blksize,
> >> + blk_shift, st);
> >> + fm_ext.fe_logical = logical;
> >> + fm_ext.fe_physical = block * st->st_blksize;
> >> + fm_ext.fe_length = st->st_blksize;
> >> + (*num_extents)++;
> >> + } else {
> >> + fm_ext.fe_length += st->st_blksize;
> >> + }
> >> } else if (last_block && (block != last_block + 1)) {
> >> - if (verbose)
> >> + if (verbose) {
> >> printf("Discontinuity: Block %ld is at %lu (was "
> >> "%lu)\n", i, block, last_block + 1);
> >> - fm_ext.fe_length = 0;
> >> + }
> >
> > The { } is not needed for a single line if statement.
>
> OK, I'll send a new version to remove it, thanks!
>
> Regards,
> Xiaoguang Wang
> >
> >> (*num_extents)++;
> >> }
> >> - fm_ext.fe_logical = logical;
> >> - fm_ext.fe_physical = block * st->st_blksize;
> >> - fm_ext.fe_length += st->st_blksize;
> >> last_block = block;
> >
> > Otherwise, I think this looks decent.
> >
> > --D
> >
> >> }
> >>
> >> --
> >> 1.8.2.1
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> >> the body of a message to [email protected]
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> > .
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi,
On 10/07/2014 11:59 AM, Darrick J. Wong wrote:
> On Tue, Oct 07, 2014 at 11:50:34AM +0800, Xiaoguang Wang wrote:
>> Hi,
>>
>> On 10/07/2014 10:43 AM, Darrick J. Wong wrote:
>>> On Mon, Oct 06, 2014 at 07:05:39PM +0800, Xiaoguang Wang wrote:
>>>> When using FIBMAP and '-e' option is specified, the calculation for fiemap_extent
>>>> is wrong, we wrongly updated fm_ext.fe_logical for every iteration, please see the
>>>> code in the end of 'for' loop in fm_ext.fe_logical().
>>>>
>>>> In an ext2 file system(block size is 1024 bytes),
>>>> dd if=/dev/zero of=testfile bs=1k count=15
>>>> Using debugfs, corresponding physical blocks are "2049 2050 2051 2052 2053 2054
>>>> 2055 2056 2057 2058 2059 2060 1025 2061 2062 2063", 1025 is this indirect block.
>>>> Before this patch, filefrag's output would be:
>>>> filefrag -B -e testfile
>>>> Filesystem type is: ef53
>>>> Filesystem cylinder groups approximately 16
>>>> File size of testfile is 15360 (15 blocks of 1024 bytes)
>>>> ext: logical_offset: physical_offset: length: expected: flags:
>>>> 0: 1.. 2: 2050.. 2051: 2: 2051: merged
>>>> 1: 3.. 4: 2052.. 2053: 2: 2053: merged
>>>> 2: 5.. 6: 2054.. 2055: 2: 2055: merged
>>>> 3: 7.. 8: 2056.. 2057: 2: 2057: merged
>>>> 4: 9.. 10: 2058.. 2059: 2: 2059: merged
>>>> 5: 11.. 12: 2060.. 2061: 2: 2062: merged
>>>> 6: 13.. 14: 2062.. 2063: 2: 2063: merged,eof
>>>> 7: 14.. 14: 2063.. 2063: 1: 2063: merged,eof
>>>> This output is not reasonable.
>>>
>>> It's just plain whacky. Why would logical offset 14 be listed twice? :)
>>
>> Because of the wrong code :) It updates fm_ext.fe_logical and fm_ext.fe_physical for every
>> iteration :) and i think the original code is not that readable.
>>>
>>>> Fix this bug and try to make it readable. After this patch, the output would be:
>>>> ./filefrag -B -e mntpoint/testfile
>>>> Filesystem type is: ef53
>>>> Filesystem cylinder groups approximately 16
>>>> File size of mntpoint/testfile is 15360 (15 blocks of 1024 bytes)
>>>> ext: logical_offset: physical_offset: length: expected: flags:
>>>> 0: 0.. 11: 2049.. 2060: 12: 2062: merged
>>>> 1: 12.. 14: 2061.. 2063: 3: 2063: merged,eof
>>>> mntpoint/testfile: 2 extents found, perfection would be 1 extent
>>>
>>> Where'd the indirect block end up, if not in the middle of 2049-2063?
>>
>> Indirect block's information is used to judge whether logical region is continuous physically.
>> For example, the above testfile's first indirect block is 1025, and will not shown in the output.
>
> 1025? Huh. Is this an old FS, or is there something wrong with the block
> allocator?
Yeah, it's strange. I created testfile multiple times, data blocks are basically continuous, but
indirect block always not. It seems that block allocator does not work well.
I have this test on a 128MB loop device(mkfs to ext2), OS: Fedora 19(3.9.5-301.fc19.x86_64)
Regards,
Xiaoguang Wang
>
> --D
>
>>
>>>
>>>> Signed-off-by: Xiaoguang Wang <[email protected]>
>>>> ---
>>>> misc/filefrag.c | 37 +++++++++++++++++++++++--------------
>>>> 1 file changed, 23 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/misc/filefrag.c b/misc/filefrag.c
>>>> index c1a8684..e9f7e68 100644
>>>> --- a/misc/filefrag.c
>>>> +++ b/misc/filefrag.c
>>>> @@ -315,32 +315,41 @@ static int filefrag_fibmap(int fd, int blk_shift, int *num_extents,
>>>> return rc;
>>>> if (block == 0)
>>>> continue;
>>>> + count++;
>>>> +
>>>> if (*num_extents == 0) {
>>>> (*num_extents)++;
>>>> if (force_extent) {
>>>> print_extent_header();
>>>> + fm_ext.fe_logical = logical;
>>>> fm_ext.fe_physical = block * st->st_blksize;
>>>> + fm_ext.fe_length = st->st_blksize;
>>>> }
>>>> + last_block = block;
>>>> + continue;
>>>> }
>>>> - count++;
>>>> - if (force_extent && last_block != 0 &&
>>>> - (block != last_block + 1 ||
>>>> - fm_ext.fe_logical + fm_ext.fe_length != logical)) {
>>>> - print_extent_info(&fm_ext, *num_extents - 1,
>>>> - (last_block + 1) * st->st_blksize,
>>>> - blk_shift, st);
>>>> - fm_ext.fe_length = 0;
>>>> - (*num_extents)++;
>>>> +
>>>> + if (force_extent) {
>>>> + if (block != last_block + 1 ||
>>>> + fm_ext.fe_length + fm_ext.fe_logical != logical) {
>>>> + print_extent_info(&fm_ext, *num_extents - 1,
>>>> + (last_block + 1) *
>>>> + st->st_blksize,
>>>> + blk_shift, st);
>>>> + fm_ext.fe_logical = logical;
>>>> + fm_ext.fe_physical = block * st->st_blksize;
>>>> + fm_ext.fe_length = st->st_blksize;
>>>> + (*num_extents)++;
>>>> + } else {
>>>> + fm_ext.fe_length += st->st_blksize;
>>>> + }
>>>> } else if (last_block && (block != last_block + 1)) {
>>>> - if (verbose)
>>>> + if (verbose) {
>>>> printf("Discontinuity: Block %ld is at %lu (was "
>>>> "%lu)\n", i, block, last_block + 1);
>>>> - fm_ext.fe_length = 0;
>>>> + }
>>>
>>> The { } is not needed for a single line if statement.
>>
>> OK, I'll send a new version to remove it, thanks!
>>
>> Regards,
>> Xiaoguang Wang
>>>
>>>> (*num_extents)++;
>>>> }
>>>> - fm_ext.fe_logical = logical;
>>>> - fm_ext.fe_physical = block * st->st_blksize;
>>>> - fm_ext.fe_length += st->st_blksize;
>>>> last_block = block;
>>>
>>> Otherwise, I think this looks decent.
>>>
>>> --D
>>>
>>>> }
>>>>
>>>> --
>>>> 1.8.2.1
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>>>> the body of a message to [email protected]
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> .
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> .
>
When using FIBMAP and '-e' option is specified, the calculation for fiemap_extent
is wrong, we wrongly updated fm_ext.fe_logical for every iteration, please see the
code in the end of 'for' loop in fm_ext.fe_logical().
In an ext2 file system(block size is 1024 bytes),
dd if=/dev/zero of=testfile bs=1k count=15
Using debugfs, corresponding physical blocks are "2049 2050 2051 2052 2053 2054
2055 2056 2057 2058 2059 2060 1025 2061 2062 2063", 1025 is this indirect block.
Before this patch, filefrag's output would be:
filefrag -B -e testfile
Filesystem type is: ef53
Filesystem cylinder groups approximately 16
File size of testfile is 15360 (15 blocks of 1024 bytes)
ext: logical_offset: physical_offset: length: expected: flags:
0: 1.. 2: 2050.. 2051: 2: 2051: merged
1: 3.. 4: 2052.. 2053: 2: 2053: merged
2: 5.. 6: 2054.. 2055: 2: 2055: merged
3: 7.. 8: 2056.. 2057: 2: 2057: merged
4: 9.. 10: 2058.. 2059: 2: 2059: merged
5: 11.. 12: 2060.. 2061: 2: 2062: merged
6: 13.. 14: 2062.. 2063: 2: 2063: merged,eof
7: 14.. 14: 2063.. 2063: 1: 2063: merged,eof
This output is not reasonable.
Fix this bug and try to make it readable. With this patch, the output would be:
./filefrag -B -e mntpoint/testfile
Filesystem type is: ef53
Filesystem cylinder groups approximately 16
File size of mntpoint/testfile is 15360 (15 blocks of 1024 bytes)
ext: logical_offset: physical_offset: length: expected: flags:
0: 0.. 11: 2049.. 2060: 12: 2062: merged
1: 12.. 14: 2061.. 2063: 3: 2063: merged,eof
mntpoint/testfile: 2 extents found, perfection would be 1 extent
Changes since v1->v2
- remove the unnecessary {} for if statement.
Signed-off-by: Xiaoguang Wang <[email protected]>
Reviewed-by: Darrick J. Wong <[email protected]>
---
misc/filefrag.c | 34 +++++++++++++++++++++-------------
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/misc/filefrag.c b/misc/filefrag.c
index c1a8684..76e613f 100644
--- a/misc/filefrag.c
+++ b/misc/filefrag.c
@@ -315,32 +315,40 @@ static int filefrag_fibmap(int fd, int blk_shift, int *num_extents,
return rc;
if (block == 0)
continue;
+ count++;
+
if (*num_extents == 0) {
(*num_extents)++;
if (force_extent) {
print_extent_header();
+ fm_ext.fe_logical = logical;
fm_ext.fe_physical = block * st->st_blksize;
+ fm_ext.fe_length = st->st_blksize;
}
+ last_block = block;
+ continue;
}
- count++;
- if (force_extent && last_block != 0 &&
- (block != last_block + 1 ||
- fm_ext.fe_logical + fm_ext.fe_length != logical)) {
- print_extent_info(&fm_ext, *num_extents - 1,
- (last_block + 1) * st->st_blksize,
- blk_shift, st);
- fm_ext.fe_length = 0;
- (*num_extents)++;
+
+ if (force_extent) {
+ if (block != last_block + 1 ||
+ fm_ext.fe_length + fm_ext.fe_logical != logical) {
+ print_extent_info(&fm_ext, *num_extents - 1,
+ (last_block + 1) *
+ st->st_blksize,
+ blk_shift, st);
+ fm_ext.fe_logical = logical;
+ fm_ext.fe_physical = block * st->st_blksize;
+ fm_ext.fe_length = st->st_blksize;
+ (*num_extents)++;
+ } else {
+ fm_ext.fe_length += st->st_blksize;
+ }
} else if (last_block && (block != last_block + 1)) {
if (verbose)
printf("Discontinuity: Block %ld is at %lu (was "
"%lu)\n", i, block, last_block + 1);
- fm_ext.fe_length = 0;
(*num_extents)++;
}
- fm_ext.fe_logical = logical;
- fm_ext.fe_physical = block * st->st_blksize;
- fm_ext.fe_length += st->st_blksize;
last_block = block;
}
--
1.8.2.1