| Message ID | 1503501082-16983-1-git-send-email-nborisov@suse.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 8/23/17 10:11 AM, Nikolay Borisov wrote: > Move the blocksize and max_extent variables to the top of the file since they > are globals. Also blocksize never changes to mark it const. Also stop passing > max_extents around and refer to it directly. I think this is fine, though it could probably go further. If blocksize is always 512, we could just use the BTOBBT() macro, which converts bytes to "basic blocks" (512 units), and do away with the variable entirely... > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > --- > io/fiemap.c | 14 +++++--------- > 1 file changed, 5 insertions(+), 9 deletions(-) > > diff --git a/io/fiemap.c b/io/fiemap.c > index 75e882057362..d1584aba7818 100644 > --- a/io/fiemap.c > +++ b/io/fiemap.c > @@ -24,6 +24,8 @@ > #include "io.h" > > static cmdinfo_t fiemap_cmd; > +static const __u64 blocksize = 512; > +static int max_extents = 0; > > static void > fiemap_help(void) > @@ -57,7 +59,6 @@ print_verbose( > int boff_w, > int tot_w, > int flg_w, > - int max_extents, > int *cur_extent, > __u64 *last_logical) > { > @@ -113,7 +114,6 @@ print_plain( > struct fiemap_extent *extent, > int lflag, > int blocksize, > - int max_extents, > int *cur_extent, > __u64 *last_logical) > { > @@ -165,7 +165,7 @@ calc_print_format( > int *tot_w, > int *flg_w) > { > - int i; > + int i; > char lbuf[32]; > char bbuf[32]; > __u64 logical; > @@ -199,7 +199,6 @@ fiemap_f( > char **argv) > { > struct fiemap *fiemap; > - int max_extents = 0; > int num_extents = 32; > int last = 0; > int lflag = 0; > @@ -214,7 +213,6 @@ fiemap_f( > int boff_w = 16; > int tot_w = 5; /* 5 since its just one number */ > int flg_w = 5; > - __u64 blocksize = 512; > __u64 last_logical = 0; > struct stat st; > > @@ -288,12 +286,10 @@ fiemap_f( > > print_verbose(extent, blocksize, foff_w, > boff_w, tot_w, flg_w, > - max_extents, &cur_extent, > - &last_logical); > + &cur_extent, &last_logical); > } else > print_plain(extent, lflag, blocksize, > - max_extents, &cur_extent, > - &last_logical); > + &cur_extent, &last_logical); > > if (extent->fe_flags & FIEMAP_EXTENT_LAST) { > last = 1; > -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 8/23/17 10:11 AM, Nikolay Borisov wrote: > Move the blocksize and max_extent variables to the top of the file since they > are globals. Also blocksize never changes to mark it const. Also stop passing > max_extents around and refer to it directly. Having looked at this for way too long, here are things that I think could be done, in more or less this order as separate patches, but rearrange or add/remove as you see fit... - Get rid of the blocksize variable, and use BTOBBT() - Turn num_extents into #define EXTENT_BATCH 32, fixed query map size - make max_extents global if you like - Clean up the control loop(s) and exit point(s), there are now exits on: + fm_mapped_extents == 0 (we got none from our query) + FIEMAP_EXTENT_LAST set (kernel tells us this one is last) + max_extents hit (we hit the user's limit) + end of requested range hit (after range is implemented) ... there's a lot in there that is messy. :/ - Fix the "-n" behavior (a minimally as possible) and update xfs_bmap(8) to match finally: - Implement your range query (I think this can be done by simply modifying the initial fm_start and fm_length, no? Or, by only modifying fm_start, and breaking out of the loop when we retrieve an extent that goes beyond the end of the requested region?) Other things I noticed: - print_plain and print_verbose are pretty nasty the way they hide increments of cur_extent & last_logical. I think it would be better to: + pass cur_extent & last_logical by value + return nr of extents printed, so the caller can increment cur_extent + update last_logical in fiemap_f after, not in, the print functions - The last hunk that handles a hole at EOF is ick; something like this may be better, re-using the print functions with a special "EOF extent"; the print functions would then return after printing a hole if they are handed a 0 length extent with LAST set, or something. /* Handle a hole at EOF if there is one */ if (cur_extent && last_logical < st.st_size) { struct fiemap_extent hole = { 0 }; /* special 0-length extent to show formatters it's hole @ EOF */ hole.fe_logical = st.st_size; hole.fe_length = 0; hole.fe_flags = FIEMAP_EXTENT_LAST; if (vflag) print_verbose(&hole, foff_w, boff_w, tot_w, flg_w, max_extents, cur_extent, last_logical); else print_plain(&hole, lflag, max_extents, cur_extent, last_logical); } -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/io/fiemap.c b/io/fiemap.c index 75e882057362..d1584aba7818 100644 --- a/io/fiemap.c +++ b/io/fiemap.c @@ -24,6 +24,8 @@ #include "io.h" static cmdinfo_t fiemap_cmd; +static const __u64 blocksize = 512; +static int max_extents = 0; static void fiemap_help(void) @@ -57,7 +59,6 @@ print_verbose( int boff_w, int tot_w, int flg_w, - int max_extents, int *cur_extent, __u64 *last_logical) { @@ -113,7 +114,6 @@ print_plain( struct fiemap_extent *extent, int lflag, int blocksize, - int max_extents, int *cur_extent, __u64 *last_logical) { @@ -165,7 +165,7 @@ calc_print_format( int *tot_w, int *flg_w) { - int i; + int i; char lbuf[32]; char bbuf[32]; __u64 logical; @@ -199,7 +199,6 @@ fiemap_f( char **argv) { struct fiemap *fiemap; - int max_extents = 0; int num_extents = 32; int last = 0; int lflag = 0; @@ -214,7 +213,6 @@ fiemap_f( int boff_w = 16; int tot_w = 5; /* 5 since its just one number */ int flg_w = 5; - __u64 blocksize = 512; __u64 last_logical = 0; struct stat st; @@ -288,12 +286,10 @@ fiemap_f( print_verbose(extent, blocksize, foff_w, boff_w, tot_w, flg_w, - max_extents, &cur_extent, - &last_logical); + &cur_extent, &last_logical); } else print_plain(extent, lflag, blocksize, - max_extents, &cur_extent, - &last_logical); + &cur_extent, &last_logical); if (extent->fe_flags & FIEMAP_EXTENT_LAST) { last = 1;
Move the blocksize and max_extent variables to the top of the file since they are globals. Also blocksize never changes to mark it const. Also stop passing max_extents around and refer to it directly. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- io/fiemap.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)