xfs_io: add the lseek() SEEK_DATA/SEEK_HOLE support
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/1 xfs_io: add the lseek() SEEK_DATA/SEEK_HOLE support
Add the lseek() SEEK_DATA/SEEK_HOLE support into xfs_io. The result from the lseek() call is printed to the output:
xfs_io> lseek -h 609k
lseek for hole starting at offset 623616 result offset 630784
Configure this feature only on Linux distributions that support SEEK_DATA/SEEK_HOLE.
--- configure.in | 1 include/builddefs.in | 1 io/Makefile | 5 ++ io/init.c | 1 io/io.h | 6 ++ io/lseek.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++ m4/package_libcdev.m4 | 15 +++++++ man/man8/xfs_io.8 | 7 +++ 8 files changed, 137 insertions(+)
configure.in +1 -0
@@ -109,6 +109,7 @@ AC_HAVE_GETMNTINFO AC_HAVE_FALLOCATE AC_HAVE_FIEMAP AC_HAVE_PREADV +AC_HAVE_LSEEK_DATA AC_HAVE_SYNC_FILE_RANGE AC_HAVE_BLKID_TOPO($enable_blkid)
include/builddefs.in +1 -0
@@ -102,6 +102,7 @@ HAVE_GETMNTINFO = @have_getmntinfo@ HAVE_FALLOCATE = @have_fallocate@ HAVE_FIEMAP = @have_fiemap@ HAVE_PREADV = @have_preadv@ +HAVE_LSEEK_DATA = @have_lseek_data@ HAVE_SYNC_FILE_RANGE = @have_sync_file_range@ GCCFLAGS = -funsigned-char -fno-strict-aliasing -Wall
io/Makefile +5 -0
@@ -80,6 +80,11 @@ ifeq ($(HAVE_PREADV),yes) LCFLAGS += -DHAVE_PREADV -DHAVE_PWRITEV endif +ifeq ($(HAVE_LSEEK_DATA),yes) +LCFLAGS += -DHAVE_LSEEK_DATA +CFILES += lseek.c +endif + default: depend $(LTCOMMAND) include $(BUILDRULES)
io/init.c +1 -0
@@ -64,6 +64,7 @@ init_commands(void)help_init ();imap_init ();inject_init (); +lseek_init ();madvise_init ();mincore_init ();mmap_init ();
io/io.h +6 -0
@@ -108,6 +108,12 @@ extern void quit_init(void);extern void shutdown_init (void );extern void truncate_init (void ); +#ifdef HAVE_LSEEK_DATA +extern void lseek_init (void ); +#else +#define lseek_init () do { } while (0) +#endif +#ifdef HAVE_FADVISE extern void fadvise_init (void );#else
io/lseek.c new file +101 -0
101 lines not on this page · raw message
m4/package_libcdev.m4 +15 -0
@@ -153,6 +153,21 @@ AC_DEFUN([AC_HAVE_PREADV], AC_SUBST(have_preadv) ]) +# +# Check if we have a working fadvise system call +# +AC_DEFUN([AC_HAVE_LSEEK_DATA], + [ AC_MSG_CHECKING([for lseek SEEK_DATA]) + AC_TRY_COMPILE([ +#include <linux/fs.h> + ], [ + lseek(0, 0, SEEK_DATA); + ], have_lseek_data=yes + AC_MSG_RESULT(yes), + AC_MSG_RESULT(no)) + AC_SUBST(have_lseek_data) + ]) + # # Check if we have a sync_file_range libc call (Linux) #
man/man8/xfs_io.8 +7 -0
@@ -377,6 +377,13 @@ must be specified as another open file .RB ( \-f ) or by path .RB ( \-i ). +.TP +.BI "lseek [ \-b " offset " | \-h " offset " ] +Repositions and prints the file pointer to the next offset containing data +.RB ( \-d ) +or next offset containing a hole +.RB ( \-h ) +.TP .SH MEMORY MAPPED I/O COMMANDS .TP
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 4 lines · Mark Tinguely
Add the
lseek()SEEK_DATA/SEEK_HOLE support intoxfs_io. The result from thelseek()call is printed to the output:
xfs_io> lseek -h 609k
lseek for hole starting at offset 623616 result offset 630784
Nice, though I think the output is too verbose. We really want to make it easy to parse, and we don't need to know what offset the seek started at as that is in the command. i.e. something like:
Type Offset
HOLE 630784
or:
DATA found at 524288
If probably all that is necessary. Indeed, one thing that might be useful is adding a "-r" option to dump out the result of repeated seeks of the same type, so if you were to do something like:
xfs_io> lseek -r -d 0
Type Offset
DATA 0
DATA 65536
DATA 524288xfs_io> lseek -r -h 0
Type Offset
HOLE 16384
HOLE 131072
HOLE 1049576
If we then add a "-a" option to dump "all", it could alternate data/hole after determining if the first offset is a hole or data:
xfs_io> lseek -r -a 0
DATA 0
HOLE 16384
DATA 65536
HOLE 131072
DATA 524288
HOLE 1049576
That gives us a method of verifying the basic layout of the file and that the basic data/hole search operation (i.e. what cp will do) works correctly via a single command in a test.
Quoted 1 line
+#define
_LARGEFILE64_SOURCE/* Seefeature_test_macros(7)*/
That's defined by _GNU_SOURCE, which is set in the makefiles, so not necessary here.
Quoted 4 lines
io/lseek.c new file +4
-0+static void +lseek_help (void ) +{ + printf(_(
io/lseek.c line 47
Quoted 10 lines
io/lseek.c new file +10
-0+"\n" +" returns the next hole or data offset at or after the specified offset\n" +"\n" +" Example:\n" +" 'lseek -d 512' - offset of data at or following offset 512\n" +"\n" +" Repositions and returns the offset of either the next data or hole.\n" +" There is an implied hole at the end of file. If the specified offset is\n" +" past end of file, or there is no data past the specied offset, the offset\n" +" -1 is returned.\n"
I'd prefer that "EOF" rather than "-1" is printed in this case.
Quoted 26 lines
+" -d -- search for data starting at the specified offset.\n"
+" -h -- search for a hole starting at the specified offset.\n"
+"\n"));io/lseek.c new file +23
-0+} + +static int +lseek_f ( +int argc, +char **argv) +{ +off64_t offset, res_off; +size_t fsblocksize, fssectsize; +int flag; +int c; + + flag =0 ; +init_cvtnum (&fsblocksize, &fssectsize); + +while ((c =getopt (argc, argv,"dh" )) !=EOF ) { +switch (c) { +case 'd' : + flag =SEEK_DATA ; +break ; +case 'h' : + flag =SEEK_HOLE ; +break ;
io/lseek.c line 83
Quoted 10 lines
io/lseek.c new file +10
-0+default : +return command_usage (&lseek_cmd); + } + } +if (!flag || optind >2 ) +return command_usage (&lseek_cmd); + offset =cvtnum (fsblocksize, fssectsize, argv[optind]); + res_off =lseek64 (file->fd, offset, flag); +printf ("lseek for %s starting at offset %lld result offset %lld\n" , + (flag ==SEEK_DATA ) ?"data" :"hole" , offset, res_off);
I think that needs the _("....") around the format string.
Also, you need to check for ENXIO for a seek beyond EOF rather than some other execution error encountered during the lseek. i.e. ENXIO is a valid result, while something like EINVAL or EBADF indicates an actual error. I think that needs to be differentiated in the output.
Quoted 4 lines
or by path
.RB ( \-i ).
+.TP
+.BI "lseek [ \-b " offset " | \-h " offset " ]
^^ -d
The parameters aren't optional - that's what the "[ ... ]" brackets indicate. i.e. this will render like:
lseek [ -b _offset_ | -h _offset_ ]
indicating lseek can be executed without parameters successfully. If should probably render like:
lseek -d | -h _offset_
Indicating that you must select either -d or -h, and you must supply an offset parameter....
man/man8/xfs_io.8 must be specified as another open file · line 385
Quoted 4 lines
man/man8/xfs_io.8 +4
-0+Repositions and prints the file pointer to the next offset containing data +.RB ( \-d ) +or next offset containing a hole +.RB ( \-h )
Given that we only support pread and pwrite operations, the repositioning of the file pointer is irrelevant so probably should not be mentioned. If it was relevant, then we'd also need to support the other seek modes to reposition the file pointer. So jsut mentioning that it returns the offset of the next ... is probably sufficient here.
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 18 lines · Dave Chinner
On Mon, Oct 22, 2012 at 04:38:00PM -0500, Mark Tinguely wrote:
Type Offset
HOLE 630784
xfs_io> lseek -r -d 0
Type Offset
DATA 0
DATA 65536
DATA 524288xfs_io> lseek -r -h 0
Type Offset
HOLE 16384
HOLE 131072
HOLE 1049576
Quoted 7 lines
xfs_io> lseek -r -a 0
DATA 0
HOLE 16384
DATA 65536
HOLE 131072
DATA 524288
HOLE 1049576
Good idea.
Quoted 4 lines
Quoted 1 line
+#define
_LARGEFILE64_SOURCE/* Seefeature_test_macros(7)*/That's defined by
_GNU_SOURCE, which is set in the makefiles, so not necessary here.
Okay, I think a couple of the header files are redundantly redundant too.
Quoted 17 lines
Quoted 5 lines
io/lseek.c new file +4
-0+static void +lseek_help (void ) +{ + printf(_(io/lseek.c line 47
Quoted 10 lines
io/lseek.c new file +10
-0+"\n" +" returns the next hole or data offset at or after the specified offset\n" +"\n" +" Example:\n" +" 'lseek -d 512' - offset of data at or following offset 512\n" +"\n" +" Repositions and returns the offset of either the next data or hole.\n" +" There is an implied hole at the end of file. If the specified offset is\n" +" past end of file, or there is no data past the specied offset, the offset\n" +" -1 is returned.\n" I'd prefer that "EOF" rather than "-1" is printed in this case.
sounds good.
<deleted mess of things to clean up>
Quoted 7 lines
Given that we only support pread and pwrite operations, the repositioning of the file pointer is irrelevant so probably should not be mentioned. If it was relevant, then we'd also need to support the other seek modes to reposition the file pointer. So jsut mentioning that it returns the offset of the next ... is probably sufficient here.
agreed. I did not the other lseek() whence options for that very reason.
Quoted 4 lines
Cheers,
Dave.
Thanks for the feedback.
PS. To Christoph: Yes, a test will be added.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH v2 1/1 xfs_io: add the lseek() SEEK_DATA/SEEK_HOLE support
In reply to Dave ChinnerAdd the lseek SEEK_DATA/SEEK_HOLE support into xfs_io. The result from the lseek() call will be printed to the output. For example:
xfs_io> lseek -h 609k
Type Offset
hole 630784
--- configure.in | 1 include/builddefs.in | 1 io/Makefile | 5 + io/init.c | 1 io/io.h | 6 + io/lseek.c | 165 ++++++++++++++++++++++++++++++++++++++++++++++++++ m4/package_libcdev.m4 | 15 ++++ man/man8/xfs_io.8 | 35 ++++++++++ 8 files changed, 229 insertions(+)
configure.in +1 -0
@@ -109,6 +109,7 @@ AC_HAVE_GETMNTINFO AC_HAVE_FALLOCATE AC_HAVE_FIEMAP AC_HAVE_PREADV +AC_HAVE_LSEEK_DATA AC_HAVE_SYNC_FILE_RANGE AC_HAVE_BLKID_TOPO($enable_blkid)
include/builddefs.in +1 -0
@@ -102,6 +102,7 @@ HAVE_GETMNTINFO = @have_getmntinfo@ HAVE_FALLOCATE = @have_fallocate@ HAVE_FIEMAP = @have_fiemap@ HAVE_PREADV = @have_preadv@ +HAVE_LSEEK_DATA = @have_lseek_data@ HAVE_SYNC_FILE_RANGE = @have_sync_file_range@ GCCFLAGS = -funsigned-char -fno-strict-aliasing -Wall
io/Makefile +5 -0
@@ -80,6 +80,11 @@ ifeq ($(HAVE_PREADV),yes) LCFLAGS += -DHAVE_PREADV -DHAVE_PWRITEV endif +ifeq ($(HAVE_LSEEK_DATA),yes) +LCFLAGS += -DHAVE_LSEEK_DATA +CFILES += lseek.c +endif + default: depend $(LTCOMMAND) include $(BUILDRULES)
io/init.c +1 -0
@@ -64,6 +64,7 @@ init_commands(void)help_init ();imap_init ();inject_init (); +lseek_init ();madvise_init ();mincore_init ();mmap_init ();
io/io.h +6 -0
@@ -108,6 +108,12 @@ extern void quit_init(void);extern void shutdown_init (void );extern void truncate_init (void ); +#ifdef HAVE_LSEEK_DATA +extern void lseek_init (void ); +#else +#define lseek_init () do { } while (0) +#endif +#ifdef HAVE_FADVISE extern void fadvise_init (void );#else
io/lseek.c new file +165 -0
165 lines not on this page · raw message
m4/package_libcdev.m4 +15 -0
@@ -153,6 +153,21 @@ AC_DEFUN([AC_HAVE_PREADV], AC_SUBST(have_preadv) ]) +# +# Check if we have a working fadvise system call +# +AC_DEFUN([AC_HAVE_LSEEK_DATA], + [ AC_MSG_CHECKING([for lseek SEEK_DATA]) + AC_TRY_COMPILE([ +#include <linux/fs.h> + ], [ + lseek(0, 0, SEEK_DATA); + ], have_lseek_data=yes + AC_MSG_RESULT(yes), + AC_MSG_RESULT(no)) + AC_SUBST(have_lseek_data) + ]) + # # Check if we have a sync_file_range libc call (Linux) #
man/man8/xfs_io.8 +35 -0
@@ -377,6 +377,41 @@ must be specified as another open file .RB ( \-f ) or by path .RB ( \-i ). +.TP +.BI "lseek \-a | \-d | \-h [ \-r ] offset" +On platforms that support the +.BR lseek (2) +.B SEEK_DATA +and +.B SEEK_HOLE +options, display the offsets of the specified segments. +.RS 1.0i +.PD 0 +.TP 0.4i +.B \-a +Display both +.B data +and +.B hole +segments starting at the specified +.B offset. +.TP +.B \-d +Display the +.B data +segment starting at the specified +.B offset. +.TP +.B \-h +Display the +.B hole +segment starting at the specified +.B offset. +.TP +.B \-r +Recursively display all the specified segments starting at the specified +.B offset. +.TP .SH MEMORY MAPPED I/O COMMANDS .TP
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH v3 1/1 xfs_io: [v3] add the lseek() SEEK_DATA/SEEK_HOLE support
Add the lseek SEEK_DATA/SEEK_HOLE support into xfs_io. The result from the lseek() call will be printed to the output. For example:
xfs_io> lseek -h 609k
Type Offset
hole 630784
v1 -> v2 Add "-a" and "-r" options.
Simplify the output.
v2 -> v3 Refactor for configure.in -> configure.ac change.
SEEK_DATA with -1 offset behaves badly on older Linux. Display error message as "ERR <errno>".
--- configure.ac | 1 include/builddefs.in | 1 io/Makefile | 5 + io/init.c | 1 io/io.h | 6 + io/lseek.c | 169 ++++++++++++++++++++++++++++++++++++++++++++++++++ m4/package_libcdev.m4 | 15 ++++ man/man8/xfs_io.8 | 35 ++++++++++ 8 files changed, 233 insertions(+)
configure.ac +1 -0
@@ -109,6 +109,7 @@ AC_HAVE_GETMNTINFO AC_HAVE_FALLOCATE AC_HAVE_FIEMAP AC_HAVE_PREADV +AC_HAVE_LSEEK_DATA AC_HAVE_SYNC_FILE_RANGE AC_HAVE_BLKID_TOPO($enable_blkid)
include/builddefs.in +1 -0
@@ -102,6 +102,7 @@ HAVE_GETMNTINFO = @have_getmntinfo@ HAVE_FALLOCATE = @have_fallocate@ HAVE_FIEMAP = @have_fiemap@ HAVE_PREADV = @have_preadv@ +HAVE_LSEEK_DATA = @have_lseek_data@ HAVE_SYNC_FILE_RANGE = @have_sync_file_range@ GCCFLAGS = -funsigned-char -fno-strict-aliasing -Wall
io/Makefile +5 -0
@@ -80,6 +80,11 @@ ifeq ($(HAVE_PREADV),yes) LCFLAGS += -DHAVE_PREADV -DHAVE_PWRITEV endif +ifeq ($(HAVE_LSEEK_DATA),yes) +LCFLAGS += -DHAVE_LSEEK_DATA +CFILES += lseek.c +endif + default: depend $(LTCOMMAND) include $(BUILDRULES)
io/init.c +1 -0
@@ -64,6 +64,7 @@ init_commands(void)help_init ();imap_init ();inject_init (); +lseek_init ();madvise_init ();mincore_init ();mmap_init ();
io/io.h +6 -0
@@ -108,6 +108,12 @@ extern void quit_init(void);extern void shutdown_init (void );extern void truncate_init (void ); +#ifdef HAVE_LSEEK_DATA +extern void lseek_init (void ); +#else +#define lseek_init () do { } while (0) +#endif +#ifdef HAVE_FADVISE extern void fadvise_init (void );#else
io/lseek.c new file +169 -0
169 lines not on this page · raw message
m4/package_libcdev.m4 +15 -0
@@ -153,6 +153,21 @@ AC_DEFUN([AC_HAVE_PREADV], AC_SUBST(have_preadv) ]) +# +# Check if we have a working fadvise system call +# +AC_DEFUN([AC_HAVE_LSEEK_DATA], + [ AC_MSG_CHECKING([for lseek SEEK_DATA]) + AC_TRY_COMPILE([ +#include <linux/fs.h> + ], [ + lseek(0, 0, SEEK_DATA); + ], have_lseek_data=yes + AC_MSG_RESULT(yes), + AC_MSG_RESULT(no)) + AC_SUBST(have_lseek_data) + ]) + # # Check if we have a sync_file_range libc call (Linux) #
man/man8/xfs_io.8 +35 -0
@@ -377,6 +377,41 @@ must be specified as another open file .RB ( \-f ) or by path .RB ( \-i ). +.TP +.BI "lseek \-a | \-d | \-h [ \-r ] offset" +On platforms that support the +.BR lseek (2) +.B SEEK_DATA +and +.B SEEK_HOLE +options, display the offsets of the specified segments. +.RS 1.0i +.PD 0 +.TP 0.4i +.B \-a +Display both +.B data +and +.B hole +segments starting at the specified +.B offset. +.TP +.B \-d +Display the +.B data +segment starting at the specified +.B offset. +.TP +.B \-h +Display the +.B hole +segment starting at the specified +.B offset. +.TP +.B \-r +Recursively display all the specified segments starting at the specified +.B offset. +.TP .SH MEMORY MAPPED I/O COMMANDS .TP
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH v3] xfs_io: [v3] add the lseek() SEEK_DATA/SEEK_HOLE support
Quoted 13 lines · Mark Tinguely
Add the lseek SEEK_DATA/SEEK_HOLE support into
xfs_io. The result from thelseek()call will be printed to the output. For example:
xfs_io> lseek -h 609k
Type Offset
hole 630784v1 -> v2 Add "-a" and "-r" options.
Simplify the output.
v2 -> v3 Refactor for configure.in -> configure.ac change.
SEEK_DATAwith -1 offset behaves badly on older Linux. Display error message as "ERR <errno>".
....
io/lseek.c line 19
Quoted 2 lines
io/lseek.c new file +2
-0+ +#include <linux/fs.h>
I missed this first time around - why is this include necessary?
io/lseek.c line 54
Quoted 3 lines
io/lseek.c new file +3
-0+#define LSEEK_DFLAG 1 << 0 +#define LSEEK_HFLAG 1 << 1 +#define LSEEK_RFLAG 1 << 2
Need parenthesis there, i.e. "(1 << 0)", so we don't get weird bugs due to operator precendence causing unintended behaviour, but....
Quoted 9 lines
io/lseek.c new file +9
-0+static int +lseek_f ( +int argc, +char **argv) +{ +off64_t offset, roff; +size_t fsblocksize, fssectsize; +int cseg; +int flag;
io/lseek.c line 74
Quoted 10 lines
io/lseek.c new file +10
-0+int i, c; + + flag =0 ; +init_cvtnum (&fsblocksize, &fssectsize); + +while ((c =getopt (argc, argv,"adhr" )) !=EOF ) { +switch (c) { +case 'a' : + flag |=LSEEK_HFLAG ; +/* fall through to pick up the DFLAG */
I'd just set the flags directly - much more obvious, less likely to go wrong in the future...
io/lseek.c line 100
Quoted 8 lines
io/lseek.c new file +8
-0+ offset =cvtnum (fsblocksize, fssectsize, argv[optind]); + +/* recursively display all information, decide where to start. */ + roff =lseek64 (file->fd, offset,SEEK_DATA ); +if (roff == offset) + cseg =LSEEK_DFLAG ; +else + cseg =LSEEK_HFLAG ;
I'm not really sure what these variables mean. "roff" is returned offset, "cseg" is current segment? Perhaps a comment or a better names is in order because right now I'm guessing at what a segment is as it is not referenced in any of the comments...
io/lseek.c line 108
Quoted 8 lines
io/lseek.c new file +8
-0+ +printf ("Type offset\n" ); + +/* loop to handle the data / hole entries in assending order. +* Only display the EOF when the initial offset was greater +* the end of file. +*/ +for (i =0 ; flag &LSEEK_RFLAG || i <2 ; i++) {
I'm very surprised gcc doesn't warn here about lack of parenthesis.
As it is, I think the code would be much clearer if you separated the non-recursive case from the recursive case given it took me 20 minutes to work out whether this loop worked correctly for both cases (e.g. why does it need 2 loops instead of 1 for the non recursive case, and does the double loop behaviour give the right results?). Something like:
if (!(flags & RECURSIVE)) return seek_single(offset, flags); return seek_recursive(offset, flags);
makes the simple case is easy to verify correct (it's just a single seek), and the recursive case doesn't get complicated by the requirements of the single seek case. That results in code that is much easier to maintain and modify in future....
io/lseek.c line 119
Quoted 11 lines
io/lseek.c new file +11
-0+if (cseg ==LSEEK_DFLAG ) { +if (flag &LSEEK_DFLAG ) { + roff =lseek64 (file->fd, offset,SEEK_DATA ); +if (roff ==-1 ) { +if (i <2 ) { +if (errno ==ENXIO ) +printf ("data EOF\n" ); +else +printf ("data ERR %d\n" , + errno); + }
Why would you only print a seek error for the first seek? I would have thought that the recursive case also needs differentiate between an error and EOF detection.
io/lseek.c line 129
Quoted 9 lines
io/lseek.c new file +9
-0+ }else +printf ("data %lld\n" , roff); + } +/* set the offset and type for the next iteration */ + cseg =LSEEK_HFLAG ; + roff =lseek64 (file->fd, offset,SEEK_HOLE ); +if (roff !=-1 ) + offset = roff; +continue ;
This extra seek only needs to be done if LSEEK_HFLAG is not set to move the offset to the point where the next SEEK_DATA will find the next data extent....
io/lseek.c line 134
Quoted 5 lines
io/lseek.c new file +5
-0+ } + +/* cseg == LSEEK_HFLAG */ +if (flag &LSEEK_HFLAG ) { + roff =lseek64 (file->fd, offset,SEEK_HOLE );
But if LSEEK_HFLAG is set, we do the same seek anyway on the next loop iteration. The logic currently is:
loop {
if (in data) {
if (seek data) {
move to next data
print
}
move to next hole
update offset
continue
}
/* in hole */
if (seek hole) {
move to next hole
print
}
move to next data
update offset
}Effectively, the loop is only dealing with a data extent or a hole extent in each loop. But given that holes and data always alternate, the same canbe acheived in a single loop:
loop {
move to next data
if (data)
print
update offset
move to next hole
if (hole)
print
update offset
}And the initial loop start (i.e. offset starts in a hole or data) can be handled with a simple "skip data" variable that is cleared on the first pass through the loop. This gets rid of needing to track the current segment type and gets rid of the redundant seeks to the same offset when dumping both data and holes....
And the printing can become a simple functions that takes a "data" or "hole" string and can be common for all callers (single and recursive)...
io/lseek.c line 159
Quoted 4 lines
io/lseek.c new file +4
-0+lseek_init (void) +{ + lseek_cmd.name ="lseek" ; + lseek_cmd.altname ="seek" ;
I'd just call them both "seek" - the "l" part of the lseek() call is an implementation detail.
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH v3] xfs_io: [v3] add the lseek() SEEK_DATA/SEEK_HOLE support
Quoted 14 lines · Dave Chinner
Quoted 8 lines · Mark Tinguely
Add the lseek SEEK_DATA/SEEK_HOLE support into
xfs_io. The result from thelseek()call will be printed to the output. For example:
xfs_io> lseek -h 609k
Type Offset
hole 630784Quoted 5 lines
v1 -> v2 Add "-a" and "-r" options.
Simplify the output.
v2 -> v3 Refactor for configure.in -> configure.ac change.
SEEK_DATAwith -1 offset behaves badly on older Linux. Display error message as "ERR<errno>".
Quoted 5 lines
....
Review
Quoted 2 lines
(quoted diff) +2
-0+ +#include<linux/fs.h>I missed this first time around - why is this include necessary?
Take it out and you will find that it contains the defines for SEEK_DATA/SEEK_HOLE.
--Mark.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH v3] xfs_io: [v3] add the lseek() SEEK_DATA/SEEK_HOLE support
Quoted 15 lines · Mark Tinguely
Quoted 14 lines · Dave Chinner
Quoted 13 lines · Mark Tinguely
Add the lseek SEEK_DATA/SEEK_HOLE support into
xfs_io. The result from thelseek()call will be printed to the output. For example:
xfs_io> lseek -h 609k
Type Offset
hole 630784v1 -> v2 Add "-a" and "-r" options.
Simplify the output.
v2 -> v3 Refactor for configure.in -> configure.ac change.
SEEK_DATAwith -1 offset behaves badly on older Linux. Display error message as "ERR<errno>".
Quoted 8 lines
Quoted 5 lines
....
Review
Quoted 2 lines
(quoted diff) +2
-0+ +#include<linux/fs.h>I missed this first time around - why is this include necessary?
Take it out and you will find that it contains the defines for SEEK_DATA/SEEK_HOLE.
It was added in glibc 2.14, IIRC. All this means is that your userspace libraries are not current, while your kernel headers are.
So, you shouldn't be including linux/fs.h directly, I think, especially as SEEK_DATA/SEEK_HOLE is functionality that is not linux specific. I suspect that you should do something more like:
#ifndef SEEK_DATA
#define SEEK_DATA 3
#define SEEK_HOLE 4
#endif
Because the autoconf test passed, but the parameters are not defined correctly by userspace. That way it will still work on other platforms if they support this functionality....
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
In addition to Dave's comments: can we get a testcase for this support into xfstests?
Quoted 241 lines · Mark Tinguely
Add the
lseek()SEEK_DATA/SEEK_HOLE support intoxfs_io. The result from thelseek()call is printed to the output:
xfs_io> lseek -h 609k
lseek for hole starting at offset 623616 result offset 630784Configure this feature only on Linux distributions that support SEEK_DATA/SEEK_HOLE.
Signed-off-by Mark Tinguely <[email protected]>--- configure.in | 1 include/builddefs.in | 1 io/Makefile | 5 ++ io/init.c | 1 io/io.h | 6 ++ io/lseek.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++ m4/package_libcdev.m4 | 15 +++++++ man/man8/xfs_io.8 | 7 +++ 8 files changed, 137 insertions(+)Index: b/configure.in
===================================================================configure.in +1
-0@@ -109,6 +109,7 @@ AC_HAVE_GETMNTINFO AC_HAVE_FALLOCATE AC_HAVE_FIEMAP AC_HAVE_PREADV +AC_HAVE_LSEEK_DATA AC_HAVE_SYNC_FILE_RANGE AC_HAVE_BLKID_TOPO($enable_blkid)Index: b/include/builddefs.in
===================================================================include/builddefs.in +1
-0@@ -102,6 +102,7 @@ HAVE_GETMNTINFO = @have_getmntinfo@ HAVE_FALLOCATE = @have_fallocate@ HAVE_FIEMAP = @have_fiemap@ HAVE_PREADV = @have_preadv@ +HAVE_LSEEK_DATA = @have_lseek_data@ HAVE_SYNC_FILE_RANGE = @have_sync_file_range@ GCCFLAGS = -funsigned-char -fno-strict-aliasing -WallIndex: b/io/Makefile
===================================================================io/Makefile +5
-0@@ -80,6 +80,11 @@ ifeq ($(HAVE_PREADV),yes) LCFLAGS += -DHAVE_PREADV -DHAVE_PWRITEV endif +ifeq ($(HAVE_LSEEK_DATA),yes) +LCFLAGS += -DHAVE_LSEEK_DATA +CFILES += lseek.c +endif + default: depend $(LTCOMMAND) include $(BUILDRULES)Index:
b/io/init.c
===================================================================io/init.c +1
-0@@ -64,6 +64,7 @@ init_commands(void)help_init ();imap_init ();inject_init (); +lseek_init ();madvise_init ();mincore_init ();mmap_init ();Index:
b/io/io.h
===================================================================io/io.h +6
-0@@ -108,6 +108,12 @@ extern void quit_init(void);extern void shutdown_init (void );extern void truncate_init (void ); +#ifdef HAVE_LSEEK_DATA +extern void lseek_init (void ); +#else +#define lseek_init () do { } while (0) +#endif +#ifdef HAVE_FADVISE extern void fadvise_init (void );#else Index:
b/io/lseek.c
===================================================================io/lseek.c new file +101
-0@@ -0,0 +1,101 @@ +/* +* Copyright (c) 2012 SGI +* All Rights Reserved. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License as +* published by the Free Software Foundation. +* +* This program is distributed in the hope that it would be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write the Free Software Foundation, +* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +*/ + +#define _LARGEFILE64_SOURCE/* See feature_test_macros(7) */ +#include <sys/types.h> +#include <unistd.h> +#include <linux/fs.h> + +#include <sys/uio.h> +#include <xfs/xfs.h> +#include <xfs/command.h> +#include <xfs/input.h> +#include <ctype.h> +#include "init.h" +#include "io.h" + +static cmdinfo_t lseek_cmd; + +static void +lseek_help (void ) +{ +printf (_ ( +"\n" +" returns the next hole or data offset at or after the specified offset\n" +"\n" +" Example:\n" +" 'lseek -d 512' - offset of data at or following offset 512\n" +"\n" +" Repositions and returns the offset of either the next data or hole.\n" +" There is an implied hole at the end of file. If the specified offset is\n" +" past end of file, or there is no data past the specied offset, the offset\n" +" -1 is returned.\n" +" -d -- search for data starting at the specified offset.\n" +" -h -- search for a hole starting at the specified offset.\n" +"\n" )); +} + +static int +lseek_f ( +int argc, +char **argv) +{ +off64_t offset, res_off; +size_t fsblocksize, fssectsize; +int flag; +int c; + + flag =0 ; +init_cvtnum (&fsblocksize, &fssectsize); + +while ((c =getopt (argc, argv,"dh" )) !=EOF ) { +switch (c) { +case 'd' : + flag =SEEK_DATA ; +break ; +case 'h' : + flag =SEEK_HOLE ; +break ; +default : +return command_usage (&lseek_cmd); + } + } +if (!flag || optind >2 ) +return command_usage (&lseek_cmd); + offset =cvtnum (fsblocksize, fssectsize, argv[optind]); + res_off =lseek64 (file->fd, offset, flag); +printf ("lseek for %s starting at offset %lld result offset %lld\n" , + (flag ==SEEK_DATA ) ?"data" :"hole" , offset, res_off); +return 0 ; +} + +void +lseek_init (void ) +{ + lseek_cmd.name ="lseek" ; + lseek_cmd.altname ="seek" ; + lseek_cmd.cfunc = lseek_f; + lseek_cmd.argmin =2 ; + lseek_cmd.argmax =2 ; + lseek_cmd.flags =CMD_NOMAP_OK |CMD_FOREIGN_OK ; + lseek_cmd.args =_ ("[-d | -h] off" ); + lseek_cmd.oneline =_ ("locate and postition to next data or hole" ); + lseek_cmd.help = lseek_help; + +add_command (&lseek_cmd); +}Index: b/m4/package_libcdev.m4
===================================================================m4/package_libcdev.m4 +15
-0@@ -153,6 +153,21 @@ AC_DEFUN([AC_HAVE_PREADV], AC_SUBST(have_preadv) ]) +# +# Check if we have a working fadvise system call +# +AC_DEFUN([AC_HAVE_LSEEK_DATA], + [ AC_MSG_CHECKING([for lseek SEEK_DATA]) + AC_TRY_COMPILE([ +#include <linux/fs.h> + ], [ + lseek(0, 0, SEEK_DATA); + ], have_lseek_data=yes + AC_MSG_RESULT(yes), + AC_MSG_RESULT(no)) + AC_SUBST(have_lseek_data) + ]) + # # Check if we have a sync_file_range libc call (Linux) #Index: b/man/man8/xfs_io.8
===================================================================man/man8/xfs_io.8 +7
-0@@ -377,6 +377,13 @@ must be specified as another open file .RB ( \-f ) or by path .RB ( \-i ). +.TP +.BI "lseek [ \-b " offset " | \-h " offset " ] +Repositions and prints the file pointer to the next offset containing data +.RB ( \-d ) +or next offset containing a hole +.RB ( \-h ) +.TP .SH MEMORY MAPPED I/O COMMANDS .TP
Quoted 4 lines
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
---end quoted text---
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs