filefrag: count optimize non-verbose mode; count 0 extents properly when verbose

PATCH Superseded Eric Sandeen 3 participants 8 messages Nov 23, 2010 – May 8, 2011

Share & tools

Eric Sandeen

author

2010-11-23 23:21 UTC #0 lore raw

PATCH 1/1 filefrag: count optimize non-verbose mode; count 0 extents properly when verbose

# 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 <[email protected]>

---

misc/filefrag.c +16 -1

index bd4486d..a48b9b0 100644
@@ -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");

Eric Sandeen

author

2010-12-09 17:19 UTC #1 lore raw

Quoted 8 lines · Eric Sandeen

# 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.

Quoted 7 lines

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 <[email protected]>

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

Quoted 41 lines

---

misc/filefrag.c +16 -2

index bd4486d..a48b9b0 100644
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Eric Sandeen

author

2011-03-11 15:01 UTC #2 lore raw

In reply to Eric Sandeen

Quoted 8 lines · Eric Sandeen

# 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.

Quoted 7 lines

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 <[email protected]>

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

Quoted 38 lines

---

misc/filefrag.c +16 -2

index bd4486d..a48b9b0 100644
@@ -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");
--

Quoted 3 lines

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

Eric Sandeen

author

2011-04-08 16:44 UTC #3 lore raw

In reply to Eric Sandeen

Quoted 8 lines · Eric Sandeen

# 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.

Quoted 7 lines

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 <[email protected]>

ping^3

Quoted 41 lines

---

misc/filefrag.c +16 -2

index bd4486d..a48b9b0 100644
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Andreas Dilger

2011-04-08 23:40 UTC #4 lore raw

Quoted 30 lines · Eric Sandeen

Quoted 8 lines · Eric Sandeen
# 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.
Quoted 7 lines
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 <[email protected]>
ping^3
Quoted 11 lines
---
misc/filefrag.c +0 -1
index bd4486d..a48b9b0 100644
@@ -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;

Quoted 8 lines

misc/filefrag.c static int filefrag_fiemap(int fd, int blk_shift, int *num_extents) · line 204
Quoted 8 lines
misc/filefrag.c +8 -0
+		/*
+		 * 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.

Quoted 15 lines

Quoted 5 lines

misc/filefrag.c +1 -0

@@ -206,6 +213,14 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents)
 		}
 
 		if (verbose && !fiemap_header_printed) {
+			/*

misc/filefrag.c static int filefrag_fiemap(int fd, int blk_shift, int *num_extents) · line 226

Quoted 10 lines

misc/filefrag.c +7 -0

+			 * 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

Eric Sandeen

author

2011-05-05 18:21 UTC #5 lore raw

PATCH v2 1/1 filefrag: count 0 extents properly when verbose

# 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 <[email protected]>

misc/filefrag.c +12 -0

index d604b6c..2795e15 100644
@@ -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");

Ted Ts'o

Applied

2011-05-07 22:45 UTC #6 lore raw

Re: [PATCH V2] filefrag: count 0 extents properly when verbose

Quoted 18 lines · Eric Sandeen

# 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.

Quoted 6 lines

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 <[email protected]>

Thanks,

applied to the maint branch (which will then get pulled to master/next) of e2fsprogs.

- Ted

Andreas Dilger

2011-05-08 03:55 UTC #7 lore raw

Re: [PATCH V2] filefrag: count 0 extents properly when verbose

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

Quoted 32 lines · Ted Ts'o

Quoted 18 lines · Eric Sandeen
# 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.
Quoted 6 lines
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 <[email protected]>
Thanks,
applied to the maint branch (which will then get pulled to master/next) of e2fsprogs.
- Ted