| Submitter | liubo |
|---|---|
| Date | June 14, 2012, 8:44 a.m. |
| Message ID | <1339663486-5417-1-git-send-email-liubo2009@cn.fujitsu.com> |
| Download | mbox | patch |
| Permalink | /patch/164868/ |
| State | Accepted |
| Headers | show |
Comments
On 6/14/12 3:44 AM, Liu Bo wrote: > filefrag has several bugs: > > 1. > $ touch f1 > $ filefrag f1 > f1: 1 extent found ----> bug! > $ filefrag -v f1 > Filesystem type is: ef53 > File size of f1 is 0 (0 blocks, blocksize 4096) > f1: 0 extents found > > 2. > $ truncate -s 1m f2 > $ filefrag f2 > f2: 1 extent found ----> bug! > $ filefrag -v f2 > Filesystem type is: ef53 > File size of f2 is 1048576 (256 blocks, blocksize 4096) > f2: 0 extents found > > 3. > $ for i in `seq 11 -2 0`; do dd if=/dev/zero of=f4 bs=4k count=1 seek=$i conv=notrunc oflag=sync &>/dev/null; done > $ ll f4 > -rw-r--r-- 1 root root 49152 Jun 9 15:09 f4 > $ filefrag f4 > f4: 7 extents found > $ filefrag -v f4 > Filesystem type is: ef53 > File size of f4 is 49152 (12 blocks, blocksize 4096) > ext logical physical expected length flags > 0 1 1109993 1 > 1 3 1109992 1109994 1 > 2 5 1109991 1109993 1 > 3 7 1109990 1109992 1 > 4 9 1109989 1109991 1 > 5 11 1108207 1109990 1 eof > f4: 7 extents found --------------------------> but we only have 6 extents, bug! > > All of these bugs come from the fact that we've made a mistake on calculating > total extents: > o we set 1 as default for 'total extents', and this will report 1 extent found > even when we don't get any extent from fiemap. > o if our first extent does not start from 0(logical addr), total extents will > be one more than what it should be. > > Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com> Looks right to me. FWIW, there's a Fedora bug open for this, #840848 Maybe another one for merging, Ted? Reviewed-by: Eric Sandeen <sandeen@redhat.com> > --- > misc/filefrag.c | 25 ++++++++++--------------- > 1 files changed, 10 insertions(+), 15 deletions(-) > > diff --git a/misc/filefrag.c b/misc/filefrag.c > index 0583e91..af008fb 100644 > --- a/misc/filefrag.c > +++ b/misc/filefrag.c > @@ -175,7 +175,7 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents) > unsigned int i; > static int fiemap_incompat_printed; > int fiemap_header_printed = 0; > - int tot_extents = 1, n = 0; > + int tot_extents = 0, n = 0; > int last = 0; > int rc; > > @@ -201,25 +201,17 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents) > return rc; > } > > + /* If 0 extents are returned, then more ioctls are not needed */ > + if (fiemap->fm_mapped_extents == 0) > + break; > + > 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"); > fiemap_header_printed = 1; > } > > - /* If 0 extents are returned, then more ioctls are not needed */ > - if (fiemap->fm_mapped_extents == 0) > - break; > - > for (i = 0; i < fiemap->fm_mapped_extents; i++) { > __u64 phy_blk, logical_blk; > unsigned long ext_len; > @@ -228,10 +220,13 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents) > ext_len = fm_ext[i].fe_length >> blk_shift; > logical_blk = fm_ext[i].fe_logical >> blk_shift; > > - if (logical_blk && phy_blk != expected) > + if (logical_blk && phy_blk != expected) { > tot_extents++; > - else > + } else { > expected = 0; > + if (!tot_extents) > + tot_extents = 1; > + } > if (verbose) > print_extent_info(&fm_ext[i], n, expected, > blk_shift); > -- 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
On Thu, Jun 14, 2012 at 04:44:46PM +0800, Liu Bo wrote: > filefrag has several bugs > ... > All of these bugs come from the fact that we've made a mistake on calculating > total extents: > o we set 1 as default for 'total extents', and this will report 1 extent found > even when we don't get any extent from fiemap. > o if our first extent does not start from 0(logical addr), total extents will > be one more than what it should be. > > Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com> Thanks, applied. - 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
Patch
diff --git a/misc/filefrag.c b/misc/filefrag.c index 0583e91..af008fb 100644 --- a/misc/filefrag.c +++ b/misc/filefrag.c @@ -175,7 +175,7 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents) unsigned int i; static int fiemap_incompat_printed; int fiemap_header_printed = 0; - int tot_extents = 1, n = 0; + int tot_extents = 0, n = 0; int last = 0; int rc; @@ -201,25 +201,17 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents) return rc; } + /* If 0 extents are returned, then more ioctls are not needed */ + if (fiemap->fm_mapped_extents == 0) + break; + 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"); fiemap_header_printed = 1; } - /* If 0 extents are returned, then more ioctls are not needed */ - if (fiemap->fm_mapped_extents == 0) - break; - for (i = 0; i < fiemap->fm_mapped_extents; i++) { __u64 phy_blk, logical_blk; unsigned long ext_len; @@ -228,10 +220,13 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents) ext_len = fm_ext[i].fe_length >> blk_shift; logical_blk = fm_ext[i].fe_logical >> blk_shift; - if (logical_blk && phy_blk != expected) + if (logical_blk && phy_blk != expected) { tot_extents++; - else + } else { expected = 0; + if (!tot_extents) + tot_extents = 1; + } if (verbose) print_extent_info(&fm_ext[i], n, expected, blk_shift);
filefrag has several bugs: 1. $ touch f1 $ filefrag f1 f1: 1 extent found ----> bug! $ filefrag -v f1 Filesystem type is: ef53 File size of f1 is 0 (0 blocks, blocksize 4096) f1: 0 extents found 2. $ truncate -s 1m f2 $ filefrag f2 f2: 1 extent found ----> bug! $ filefrag -v f2 Filesystem type is: ef53 File size of f2 is 1048576 (256 blocks, blocksize 4096) f2: 0 extents found 3. $ for i in `seq 11 -2 0`; do dd if=/dev/zero of=f4 bs=4k count=1 seek=$i conv=notrunc oflag=sync &>/dev/null; done $ ll f4 -rw-r--r-- 1 root root 49152 Jun 9 15:09 f4 $ filefrag f4 f4: 7 extents found $ filefrag -v f4 Filesystem type is: ef53 File size of f4 is 49152 (12 blocks, blocksize 4096) ext logical physical expected length flags 0 1 1109993 1 1 3 1109992 1109994 1 2 5 1109991 1109993 1 3 7 1109990 1109992 1 4 9 1109989 1109991 1 5 11 1108207 1109990 1 eof f4: 7 extents found --------------------------> but we only have 6 extents, bug! All of these bugs come from the fact that we've made a mistake on calculating total extents: o we set 1 as default for 'total extents', and this will report 1 extent found even when we don't get any extent from fiemap. o if our first extent does not start from 0(logical addr), total extents will be one more than what it should be. Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com> --- misc/filefrag.c | 25 ++++++++++--------------- 1 files changed, 10 insertions(+), 15 deletions(-)