* Re: [PATCH] filefrag: count optimize non-verbose mode; count 0 extents properly when verbose
2010-11-23 23:21 [PATCH] filefrag: count optimize non-verbose mode; count 0 extents properly when verbose Eric Sandeen
@ 2010-12-09 17:19 ` Eric Sandeen
2011-03-11 15:01 ` Eric Sandeen
2011-04-08 16:44 ` Eric Sandeen
2 siblings, 0 replies; 8+ messages in thread
From: Eric Sandeen @ 2010-12-09 17:19 UTC (permalink / raw)
To: ext4 development
On 11/23/10 5:21 PM, Eric Sandeen wrote:
> # rm -f a; touch a; filefrag -v a
>
> yields 1 extent when it should be 0. Without -v, 0 is returned.
>
> Fix this up by special-casing no extents returned in verbose
> mode; skip printing the header for the columns too, since there
> are no columns to print.
>
> Also, in nonverbose mode we can set fm_extent_count to 0
> so that FIEMAP will just query the extent count without gathering
> details; as it is today I think a non-verbose query may under-report
> the extent count once "count" extents have been filled in.
>
> Addresses-redhat-bugzilla: 653234
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Ted, wondering if you can push this one in as well, if it looks ok;
I'd like to get it upstream before adding it to our package for the
above bug.
Thanks,
-Eric
> ---
>
> diff --git a/misc/filefrag.c b/misc/filefrag.c
> index bd4486d..a48b9b0 100644
> --- a/misc/filefrag.c
> +++ b/misc/filefrag.c
> @@ -194,7 +194,14 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents)
> do {
> fiemap->fm_length = ~0ULL;
> fiemap->fm_flags = flags;
> - fiemap->fm_extent_count = count;
> + /*
> + * If fm_extent_count == 0, FIEMAP returns count of
> + * extents found without filling in details.
> + */
> + if (!verbose)
> + fiemap->fm_extent_count = 0;
> + else
> + fiemap->fm_extent_count = count;
> rc = ioctl(fd, FS_IOC_FIEMAP, (unsigned long) fiemap);
> if (rc < 0) {
> if (errno == EBADR && fiemap_incompat_printed == 0) {
> @@ -206,6 +213,14 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents)
> }
>
> if (verbose && !fiemap_header_printed) {
> + /*
> + * No extents on first call?
> + * Skip header and show 0 extents.
> + */
> + if (fiemap->fm_mapped_extents == 0) {
> + *num_extents = 0;
> + goto out;
> + }
> printf(" ext %*s %*s %*s length flags\n", logical_width,
> "logical", physical_width, "physical",
> physical_width, "expected");
> --
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] filefrag: count optimize non-verbose mode; count 0 extents properly when verbose
2010-11-23 23:21 [PATCH] filefrag: count optimize non-verbose mode; count 0 extents properly when verbose Eric Sandeen
2010-12-09 17:19 ` Eric Sandeen
@ 2011-03-11 15:01 ` Eric Sandeen
2011-04-08 16:44 ` Eric Sandeen
2 siblings, 0 replies; 8+ messages in thread
From: Eric Sandeen @ 2011-03-11 15:01 UTC (permalink / raw)
To: ext4 development; +Cc: p
On 11/23/10 5:21 PM, Eric Sandeen wrote:
> # rm -f a; touch a; filefrag -v a
>
> yields 1 extent when it should be 0. Without -v, 0 is returned.
>
> Fix this up by special-casing no extents returned in verbose
> mode; skip printing the header for the columns too, since there
> are no columns to print.
>
> Also, in nonverbose mode we can set fm_extent_count to 0
> so that FIEMAP will just query the extent count without gathering
> details; as it is today I think a non-verbose query may under-report
> the extent count once "count" extents have been filled in.
>
> Addresses-redhat-bugzilla: 653234
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Ted, ping on this one? Looks like it got lost. I noticed this when
Pádraig Brady filed another red hat bug for the same issue,
saying that he had also sent you a patch...
-Eric
> ---
>
> diff --git a/misc/filefrag.c b/misc/filefrag.c
> index bd4486d..a48b9b0 100644
> --- a/misc/filefrag.c
> +++ b/misc/filefrag.c
> @@ -194,7 +194,14 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents)
> do {
> fiemap->fm_length = ~0ULL;
> fiemap->fm_flags = flags;
> - fiemap->fm_extent_count = count;
> + /*
> + * If fm_extent_count == 0, FIEMAP returns count of
> + * extents found without filling in details.
> + */
> + if (!verbose)
> + fiemap->fm_extent_count = 0;
> + else
> + fiemap->fm_extent_count = count;
> rc = ioctl(fd, FS_IOC_FIEMAP, (unsigned long) fiemap);
> if (rc < 0) {
> if (errno == EBADR && fiemap_incompat_printed == 0) {
> @@ -206,6 +213,14 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents)
> }
>
> if (verbose && !fiemap_header_printed) {
> + /*
> + * No extents on first call?
> + * Skip header and show 0 extents.
> + */
> + if (fiemap->fm_mapped_extents == 0) {
> + *num_extents = 0;
> + goto out;
> + }
> printf(" ext %*s %*s %*s length flags\n", logical_width,
> "logical", physical_width, "physical",
> physical_width, "expected");
> --
> 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
--
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] filefrag: count optimize non-verbose mode; count 0 extents properly when verbose
2010-11-23 23:21 [PATCH] filefrag: count optimize non-verbose mode; count 0 extents properly when verbose Eric Sandeen
2010-12-09 17:19 ` Eric Sandeen
2011-03-11 15:01 ` Eric Sandeen
@ 2011-04-08 16:44 ` Eric Sandeen
2011-04-08 23:40 ` Andreas Dilger
2 siblings, 1 reply; 8+ messages in thread
From: Eric Sandeen @ 2011-04-08 16:44 UTC (permalink / raw)
To: ext4 development
On 11/23/10 3:21 PM, Eric Sandeen wrote:
> # rm -f a; touch a; filefrag -v a
>
> yields 1 extent when it should be 0. Without -v, 0 is returned.
>
> Fix this up by special-casing no extents returned in verbose
> mode; skip printing the header for the columns too, since there
> are no columns to print.
>
> Also, in nonverbose mode we can set fm_extent_count to 0
> so that FIEMAP will just query the extent count without gathering
> details; as it is today I think a non-verbose query may under-report
> the extent count once "count" extents have been filled in.
>
> Addresses-redhat-bugzilla: 653234
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
ping^3
> ---
>
> diff --git a/misc/filefrag.c b/misc/filefrag.c
> index bd4486d..a48b9b0 100644
> --- a/misc/filefrag.c
> +++ b/misc/filefrag.c
> @@ -194,7 +194,14 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents)
> do {
> fiemap->fm_length = ~0ULL;
> fiemap->fm_flags = flags;
> - fiemap->fm_extent_count = count;
> + /*
> + * If fm_extent_count == 0, FIEMAP returns count of
> + * extents found without filling in details.
> + */
> + if (!verbose)
> + fiemap->fm_extent_count = 0;
> + else
> + fiemap->fm_extent_count = count;
> rc = ioctl(fd, FS_IOC_FIEMAP, (unsigned long) fiemap);
> if (rc < 0) {
> if (errno == EBADR && fiemap_incompat_printed == 0) {
> @@ -206,6 +213,14 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents)
> }
>
> if (verbose && !fiemap_header_printed) {
> + /*
> + * No extents on first call?
> + * Skip header and show 0 extents.
> + */
> + if (fiemap->fm_mapped_extents == 0) {
> + *num_extents = 0;
> + goto out;
> + }
> printf(" ext %*s %*s %*s length flags\n", logical_width,
> "logical", physical_width, "physical",
> physical_width, "expected");
> --
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] filefrag: count optimize non-verbose mode; count 0 extents properly when verbose
2011-04-08 16:44 ` Eric Sandeen
@ 2011-04-08 23:40 ` Andreas Dilger
2011-05-05 18:21 ` [PATCH V2] filefrag: " Eric Sandeen
0 siblings, 1 reply; 8+ messages in thread
From: Andreas Dilger @ 2011-04-08 23:40 UTC (permalink / raw)
To: Eric Sandeen; +Cc: ext4 development
On 2011-04-08, at 10:44 AM, Eric Sandeen wrote:
> On 11/23/10 3:21 PM, Eric Sandeen wrote:
>> # rm -f a; touch a; filefrag -v a
>>
>> yields 1 extent when it should be 0. Without -v, 0 is returned.
>>
>> Fix this up by special-casing no extents returned in verbose
>> mode; skip printing the header for the columns too, since there
>> are no columns to print.
>>
>> Also, in nonverbose mode we can set fm_extent_count to 0
>> so that FIEMAP will just query the extent count without gathering
>> details; as it is today I think a non-verbose query may under-report
>> the extent count once "count" extents have been filled in.
>>
>> Addresses-redhat-bugzilla: 653234
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>
> ping^3
>
>> ---
>>
>> diff --git a/misc/filefrag.c b/misc/filefrag.c
>> index bd4486d..a48b9b0 100644
>> --- a/misc/filefrag.c
>> +++ b/misc/filefrag.c
>> @@ -194,7 +194,14 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents)
>> do {
>> fiemap->fm_length = ~0ULL;
>> fiemap->fm_flags = flags;
>> - fiemap->fm_extent_count = count;
>> + /*
>> + * If fm_extent_count == 0, FIEMAP returns count of
>> + * extents found without filling in details.
>> + */
>> + if (!verbose)
>> + fiemap->fm_extent_count = 0;
>> + else
>> + fiemap->fm_extent_count = count;
This is already checked before the start of the do {} while() loop, just above the context of this patch:
if (!verbose)
count = 0;
No point in checking (!verbose) inside the loop.
>> @@ -206,6 +213,14 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents)
>> }
>>
>> if (verbose && !fiemap_header_printed) {
>> + /*
>> + * No extents on first call?
>> + * Skip header and show 0 extents.
>> + */
>> + if (fiemap->fm_mapped_extents == 0) {
>> + *num_extents = 0;
>> + goto out;
>> + }
>> printf(" ext %*s %*s %*s length flags\n", logical_width,
>> "logical", physical_width, "physical",
>> physical_width, "expected");
This part definitely makes sense.
Cheers, Andreas
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH V2] filefrag: count 0 extents properly when verbose
2011-04-08 23:40 ` Andreas Dilger
@ 2011-05-05 18:21 ` Eric Sandeen
2011-05-07 22:45 ` Ted Ts'o
0 siblings, 1 reply; 8+ messages in thread
From: Eric Sandeen @ 2011-05-05 18:21 UTC (permalink / raw)
To: Andreas Dilger; +Cc: ext4 development
# rm -f a; touch a; filefrag a
/boot/a: 0 extents found
works properly, but
# rm -f a; touch a; filefrag -v a
Filesystem type is: ef53
Filesystem cylinder groups is approximately 61
File size of a is 0 (0 blocks, blocksize 1024)
ext logical physical expected length flags
a: 1 extent found
yields 1 extent when it should be 0.
Fix this up by special-casing no extents returned in verbose
mode; skip printing the header for the columns too, since there
are no columns to print.
Also, in nonverbose mode we can set fm_extent_count to 0
so that FIEMAP will just query the extent count without gathering
details; clarify this with a comment.
Addresses-redhat-bugzilla: 653234
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
diff --git a/misc/filefrag.c b/misc/filefrag.c
index d604b6c..2795e15 100644
--- a/misc/filefrag.c
+++ b/misc/filefrag.c
@@ -180,6 +180,10 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents)
memset(fiemap, 0, sizeof(struct fiemap));
+ /*
+ * If count (and therefore fm_extent_count) == 0, FIEMAP
+ * returns count of extents found without filling in details.
+ */
if (!verbose)
count = 0;
@@ -204,6 +208,14 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents)
}
if (verbose && !fiemap_header_printed) {
+ /*
+ * No extents on first call?
+ * Skip header and show 0 extents.
+ */
+ if (fiemap->fm_mapped_extents == 0) {
+ *num_extents = 0;
+ goto out;
+ }
printf(" ext %*s %*s %*s length flags\n", logical_width,
"logical", physical_width, "physical",
physical_width, "expected");
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH V2] filefrag: count 0 extents properly when verbose
2011-05-05 18:21 ` [PATCH V2] filefrag: " Eric Sandeen
@ 2011-05-07 22:45 ` Ted Ts'o
2011-05-08 3:35 ` Andreas Dilger
0 siblings, 1 reply; 8+ messages in thread
From: Ted Ts'o @ 2011-05-07 22:45 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Andreas Dilger, ext4 development
On Thu, May 05, 2011 at 01:21:08PM -0500, Eric Sandeen wrote:
> # rm -f a; touch a; filefrag a
> /boot/a: 0 extents found
>
> works properly, but
>
> # rm -f a; touch a; filefrag -v a
> Filesystem type is: ef53
> Filesystem cylinder groups is approximately 61
> File size of a is 0 (0 blocks, blocksize 1024)
> ext logical physical expected length flags
> a: 1 extent found
>
> yields 1 extent when it should be 0.
>
> Fix this up by special-casing no extents returned in verbose
> mode; skip printing the header for the columns too, since there
> are no columns to print.
>
> Also, in nonverbose mode we can set fm_extent_count to 0
> so that FIEMAP will just query the extent count without gathering
> details; clarify this with a comment.
>
> Addresses-redhat-bugzilla: 653234
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Thanks,
applied to the maint branch (which will then get pulled to
master/next) of e2fsprogs.
- Ted
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] filefrag: count 0 extents properly when verbose
2011-05-07 22:45 ` Ted Ts'o
@ 2011-05-08 3:35 ` Andreas Dilger
0 siblings, 0 replies; 8+ messages in thread
From: Andreas Dilger @ 2011-05-08 3:35 UTC (permalink / raw)
To: Ted Ts'o; +Cc: Eric Sandeen, ext4 development
I was going to comment on this on Friday and supply a new patch but got distracted. There is actually a cleaner way to implement this, by initializing the extent count to zero, and always incrementing it inside the inner loop. That simplifies the case when there are no extents returned by FIEMAP.
I'm sure I'll get around to sending an updated patch at some point, and will clean up this section at the same time.
Cheers, Andreas
On 2011-05-07, at 4:45 PM, Ted Ts'o <tytso@mit.edu> wrote:
> On Thu, May 05, 2011 at 01:21:08PM -0500, Eric Sandeen wrote:
>> # rm -f a; touch a; filefrag a
>> /boot/a: 0 extents found
>>
>> works properly, but
>>
>> # rm -f a; touch a; filefrag -v a
>> Filesystem type is: ef53
>> Filesystem cylinder groups is approximately 61
>> File size of a is 0 (0 blocks, blocksize 1024)
>> ext logical physical expected length flags
>> a: 1 extent found
>>
>> yields 1 extent when it should be 0.
>>
>> Fix this up by special-casing no extents returned in verbose
>> mode; skip printing the header for the columns too, since there
>> are no columns to print.
>>
>> Also, in nonverbose mode we can set fm_extent_count to 0
>> so that FIEMAP will just query the extent count without gathering
>> details; clarify this with a comment.
>>
>> Addresses-redhat-bugzilla: 653234
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>
> Thanks,
>
> applied to the maint branch (which will then get pulled to
> master/next) of e2fsprogs.
>
> - Ted
^ permalink raw reply [flat|nested] 8+ messages in thread