| Message ID | 1502368241-8928-1-git-send-email-nborisov@suse.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Thu, Aug 10, 2017 at 03:30:41PM +0300, Nikolay Borisov wrote: > Add 2 additional, optional, arguments to the embedded fiemap command, > that way one can specify exact ranges to be fiemapped. This will be used > for a btrfs test. Since the arguments are optional, omitting them just > retains the old behavior. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > --- > > Changes since v1: > * Added help in man describing the new options > * Removed whitespace damage > * Simplified/dedup the code used to get the parameters in > * Updated one line help to indicate that len param can't exist on its own > > io/fiemap.c | 28 ++++++++++++++++++++++++++-- > man/man8/xfs_io.8 | 5 +++-- > 2 files changed, 29 insertions(+), 4 deletions(-) > > diff --git a/io/fiemap.c b/io/fiemap.c > index 75e882057362..2a0698e9d1e9 100644 > --- a/io/fiemap.c > +++ b/io/fiemap.c > @@ -34,6 +34,7 @@ fiemap_help(void) > "\n" > " Example:\n" > " 'fiemap -v' - tabular format verbose map\n" > +" 'fiemap 0 4k' - print fiemap extents for 0-4k range\n" > "\n" > " fiemap prints the map of disk blocks used by the current file.\n" > " The map lists each extent used by the file, as well as regions in the\n" > @@ -216,7 +217,11 @@ fiemap_f( > int flg_w = 5; > __u64 blocksize = 512; > __u64 last_logical = 0; > + __u64 len = -1LL; > struct stat st; > + size_t fsblocksize, fssectsize; > + > + init_cvtnum(&fsblocksize, &fssectsize); > > while ((c = getopt(argc, argv, "aln:v")) != EOF) { > switch (c) { > @@ -237,6 +242,25 @@ fiemap_f( > } > } > > + if (optind < argc) { > + off64_t start_offset = cvtnum(fsblocksize, fssectsize, argv[optind]); > + if (start_offset < 0) { > + printf("non-numeric offset argument -- %s\n", argv[optind]); > + return 0; > + } > + last_logical = start_offset; > + optind++; > + } > + > + if (optind < argc) { > + off64_t length = cvtnum(fsblocksize, fssectsize, argv[optind]); > + if (length < 0) { > + printf("non-numeric len argument -- %s\n", argv[optind]); > + return 0; > + } > + len = length; > + } > + > if (max_extents) > num_extents = min(num_extents, max_extents); > map_size = sizeof(struct fiemap) + > @@ -259,7 +283,7 @@ fiemap_f( > memset(fiemap, 0, map_size); > fiemap->fm_flags = fiemap_flags; > fiemap->fm_start = last_logical; > - fiemap->fm_length = -1LL; > + fiemap->fm_length = len; We don't decrement len each time through the ioctl loop? Doesn't that cause us to request and retrieve rows for ranges we don't want? --D > fiemap->fm_extent_count = num_extents; > > ret = ioctl(file->fd, FS_IOC_FIEMAP, (unsigned long)fiemap); > @@ -350,7 +374,7 @@ fiemap_init(void) > fiemap_cmd.argmin = 0; > fiemap_cmd.argmax = -1; > fiemap_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK; > - fiemap_cmd.args = _("[-alv] [-n nx]"); > + fiemap_cmd.args = _("[-alv] [-n nx] [start offset [len]]"); > fiemap_cmd.oneline = _("print block mapping for a file"); > fiemap_cmd.help = fiemap_help; > > diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8 > index 273b9c54c52d..125db9181851 100644 > --- a/man/man8/xfs_io.8 > +++ b/man/man8/xfs_io.8 > @@ -295,11 +295,12 @@ Prints the block mapping for the current open file. Refer to the > .BR xfs_bmap (8) > manual page for complete documentation. > .TP > -.BI "fiemap [ \-alv ] [ \-n " nx " ]" > +.BI "fiemap [ \-alv ] [ \-n " nx " ] [ " offset " [ " len " ]]" > Prints the block mapping for the current open file using the fiemap > ioctl. Options behave as described in the > .BR xfs_bmap (8) > -manual page. > +manual page. Optionally, this command also supports passing the start offset > +from where to begin the fiemap and the length of that region. > .TP > .BI "fsmap [ \-d | \-l | \-r ] [ \-m | \-v ] [ \-n " nx " ] [ " start " ] [ " end " ] > Prints the mapping of disk blocks used by the filesystem hosting the current > -- > 2.7.4 > > -- > 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 -- 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 10.08.2017 19:13, Darrick J. Wong wrote: > We don't decrement len each time through the ioctl loop? Doesn't that > cause us to request and retrieve rows for ranges we don't want? Hm, you are right, I believe on every iteration we should do: len -= last_logical - start_offset -- 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 Thu, Aug 10, 2017 at 07:46:35PM +0300, Nikolay Borisov wrote: > > > On 10.08.2017 19:13, Darrick J. Wong wrote: > > We don't decrement len each time through the ioctl loop? Doesn't that > > cause us to request and retrieve rows for ranges we don't want? > > Hm, you are right, I believe on every iteration we should do: > > len -= last_logical - start_offset start_offset doesn't change, right? I would think len -= last_logical - fiemap->fm_start. Or just calculate end_offset = start_offset + length and set fiemap->fm_length = end_offset - last_logical... ...though now that I look at that file even closer I wonder what is going on with that outer while loop counting extents? Does -n exist to let users cap the number of records to print, even if that means we don't make it to the end of the range requested? --D > -- > 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 -- 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/10/17 2:12 PM, Darrick J. Wong wrote: > On Thu, Aug 10, 2017 at 07:46:35PM +0300, Nikolay Borisov wrote: >> >> >> On 10.08.2017 19:13, Darrick J. Wong wrote: >>> We don't decrement len each time through the ioctl loop? Doesn't that >>> cause us to request and retrieve rows for ranges we don't want? >> >> Hm, you are right, I believe on every iteration we should do: >> >> len -= last_logical - start_offset > > start_offset doesn't change, right? > > I would think len -= last_logical - fiemap->fm_start. > > Or just calculate end_offset = start_offset + length and set > fiemap->fm_length = end_offset - last_logical... > > ...though now that I look at that file even closer I wonder what is > going on with that outer while loop counting extents? Does -n exist to > let users cap the number of records to print, even if that means we > don't make it to the end of the range requested? -n should simply be the size of each fiemap request, i.e. how many to request and fill on each iteration. It should not affect how many get printed in the end. At least that's how xfs_bmap is supposed to work, and therefore in theory how xfs_io's fiemap should work. -Eric -- 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/10/17 2:22 PM, Eric Sandeen wrote: > On 8/10/17 2:12 PM, Darrick J. Wong wrote: >> On Thu, Aug 10, 2017 at 07:46:35PM +0300, Nikolay Borisov wrote: >>> >>> >>> On 10.08.2017 19:13, Darrick J. Wong wrote: >>>> We don't decrement len each time through the ioctl loop? Doesn't that >>>> cause us to request and retrieve rows for ranges we don't want? >>> >>> Hm, you are right, I believe on every iteration we should do: >>> >>> len -= last_logical - start_offset >> >> start_offset doesn't change, right? >> >> I would think len -= last_logical - fiemap->fm_start. >> >> Or just calculate end_offset = start_offset + length and set >> fiemap->fm_length = end_offset - last_logical... >> >> ...though now that I look at that file even closer I wonder what is >> going on with that outer while loop counting extents? Does -n exist to >> let users cap the number of records to print, even if that means we >> don't make it to the end of the range requested? > > -n should simply be the size of each fiemap request, i.e. how many to > request and fill on each iteration. It should not affect how many > get printed in the end. > > At least that's how xfs_bmap is supposed to work, and therefore in theory > how xfs_io's fiemap should work. ... except apparently that is not actually how it works :( -Eric -- 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 11.08.2017 00:57, Eric Sandeen wrote: > On 8/10/17 2:22 PM, Eric Sandeen wrote: >> On 8/10/17 2:12 PM, Darrick J. Wong wrote: >>> On Thu, Aug 10, 2017 at 07:46:35PM +0300, Nikolay Borisov wrote: >>>> >>>> >>>> On 10.08.2017 19:13, Darrick J. Wong wrote: >>>>> We don't decrement len each time through the ioctl loop? Doesn't that >>>>> cause us to request and retrieve rows for ranges we don't want? >>>> >>>> Hm, you are right, I believe on every iteration we should do: >>>> >>>> len -= last_logical - start_offset >>> >>> start_offset doesn't change, right? >>> >>> I would think len -= last_logical - fiemap->fm_start. >>> >>> Or just calculate end_offset = start_offset + length and set >>> fiemap->fm_length = end_offset - last_logical... >>> >>> ...though now that I look at that file even closer I wonder what is >>> going on with that outer while loop counting extents? Does -n exist to >>> let users cap the number of records to print, even if that means we >>> don't make it to the end of the range requested? >> >> -n should simply be the size of each fiemap request, i.e. how many to >> request and fill on each iteration. It should not affect how many >> get printed in the end. >> >> At least that's how xfs_bmap is supposed to work, and therefore in theory >> how xfs_io's fiemap should work. > > ... except apparently that is not actually how it works :( In my tests passing -n 1 doesn't print anything irrespective of whether I use the newly added args, it was strange. It's supposed to define the batch of extents to query at a time. However, that's definitely not what it's doing... Also the man pages states that if -n is not passed fiemap would query the number of extents required and use that, however the code currently just uses the default of 32 extents ... > > -Eric > -- 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..2a0698e9d1e9 100644 --- a/io/fiemap.c +++ b/io/fiemap.c @@ -34,6 +34,7 @@ fiemap_help(void) "\n" " Example:\n" " 'fiemap -v' - tabular format verbose map\n" +" 'fiemap 0 4k' - print fiemap extents for 0-4k range\n" "\n" " fiemap prints the map of disk blocks used by the current file.\n" " The map lists each extent used by the file, as well as regions in the\n" @@ -216,7 +217,11 @@ fiemap_f( int flg_w = 5; __u64 blocksize = 512; __u64 last_logical = 0; + __u64 len = -1LL; struct stat st; + size_t fsblocksize, fssectsize; + + init_cvtnum(&fsblocksize, &fssectsize); while ((c = getopt(argc, argv, "aln:v")) != EOF) { switch (c) { @@ -237,6 +242,25 @@ fiemap_f( } } + if (optind < argc) { + off64_t start_offset = cvtnum(fsblocksize, fssectsize, argv[optind]); + if (start_offset < 0) { + printf("non-numeric offset argument -- %s\n", argv[optind]); + return 0; + } + last_logical = start_offset; + optind++; + } + + if (optind < argc) { + off64_t length = cvtnum(fsblocksize, fssectsize, argv[optind]); + if (length < 0) { + printf("non-numeric len argument -- %s\n", argv[optind]); + return 0; + } + len = length; + } + if (max_extents) num_extents = min(num_extents, max_extents); map_size = sizeof(struct fiemap) + @@ -259,7 +283,7 @@ fiemap_f( memset(fiemap, 0, map_size); fiemap->fm_flags = fiemap_flags; fiemap->fm_start = last_logical; - fiemap->fm_length = -1LL; + fiemap->fm_length = len; fiemap->fm_extent_count = num_extents; ret = ioctl(file->fd, FS_IOC_FIEMAP, (unsigned long)fiemap); @@ -350,7 +374,7 @@ fiemap_init(void) fiemap_cmd.argmin = 0; fiemap_cmd.argmax = -1; fiemap_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK; - fiemap_cmd.args = _("[-alv] [-n nx]"); + fiemap_cmd.args = _("[-alv] [-n nx] [start offset [len]]"); fiemap_cmd.oneline = _("print block mapping for a file"); fiemap_cmd.help = fiemap_help; diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8 index 273b9c54c52d..125db9181851 100644 --- a/man/man8/xfs_io.8 +++ b/man/man8/xfs_io.8 @@ -295,11 +295,12 @@ Prints the block mapping for the current open file. Refer to the .BR xfs_bmap (8) manual page for complete documentation. .TP -.BI "fiemap [ \-alv ] [ \-n " nx " ]" +.BI "fiemap [ \-alv ] [ \-n " nx " ] [ " offset " [ " len " ]]" Prints the block mapping for the current open file using the fiemap ioctl. Options behave as described in the .BR xfs_bmap (8) -manual page. +manual page. Optionally, this command also supports passing the start offset +from where to begin the fiemap and the length of that region. .TP .BI "fsmap [ \-d | \-l | \-r ] [ \-m | \-v ] [ \-n " nx " ] [ " start " ] [ " end " ] Prints the mapping of disk blocks used by the filesystem hosting the current
Add 2 additional, optional, arguments to the embedded fiemap command, that way one can specify exact ranges to be fiemapped. This will be used for a btrfs test. Since the arguments are optional, omitting them just retains the old behavior. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- Changes since v1: * Added help in man describing the new options * Removed whitespace damage * Simplified/dedup the code used to get the parameters in * Updated one line help to indicate that len param can't exist on its own io/fiemap.c | 28 ++++++++++++++++++++++++++-- man/man8/xfs_io.8 | 5 +++-- 2 files changed, 29 insertions(+), 4 deletions(-)