| Message ID | 1494502507-27742-1-git-send-email-agruenba@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, May 11, 2017 at 01:35:07PM +0200, Andreas Gruenbacher wrote: > src/seek_sanity_test (test generic/285) assumes that after preallocating > space in a file with fallocate, fseek SEEK_HOLE / SEEK_DATA will still > report the allocated space as a hole. On filesystems without unwritten > extent support, that space will be reported as data, though. > > Tested on ext4, xfs, and gfs2 + patches for fseek SEEK_HOLE / SEEK_DATA > support. The idea seems fine to me, but I'm not that familiar with SEEK_DATA/HOLE support, it'd be great if someone else could help review this patch. Some of my thoughts inline. > > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> > --- > src/seek_sanity_test.c | 35 +++++++++++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c > index a6dd48c..0d7fa0a 100644 > --- a/src/seek_sanity_test.c > +++ b/src/seek_sanity_test.c > @@ -37,6 +37,7 @@ > > static blksize_t alloc_size; > int default_behavior = 0; > +int unwritten_extents = 0; > char *base_file_path; > > static void get_file_system(int fd) > @@ -282,6 +283,11 @@ static int test09(int fd, int testnum) > int bufsz = alloc_size; > int filsz = 8 << 20; > > + if (!unwritten_extents) { > + fprintf(stdout, "Test skipped\n"); > + goto out; > + } > + > /* > * HOLE - unwritten DATA in dirty page - HOLE - > * unwritten DATA in writeback page > @@ -338,6 +344,11 @@ static int test08(int fd, int testnum) > int bufsz = alloc_size; > int filsz = 4 << 20; > > + if (!unwritten_extents) { > + fprintf(stdout, "Test skipped\n"); > + goto out; > + } > + > /* HOLE - unwritten DATA in writeback page */ > /* Each unit is bufsz */ > buf = do_malloc(bufsz); > @@ -387,6 +398,11 @@ static int test07(int fd, int testnum) > int bufsz = alloc_size; > int filsz = 4 << 20; > > + if (!unwritten_extents) { > + fprintf(stdout, "Test skipped\n"); > + goto out; > + } > + > /* HOLE - unwritten DATA in dirty page */ > /* Each unit is bufsz */ > buf = do_malloc(bufsz); > @@ -776,6 +792,25 @@ static int test_basic_support(void) > fprintf(stderr, "File system supports the default behavior.\n"); > } > > + ftruncate(fd, 0); > + if (fallocate(fd, 0, 0, alloc_size) == -1) { > + if (errno == EOPNOTSUPP) { > + fprintf(stderr, "File system does not support fallocate."); > + } else { > + fprintf(stderr, "ERROR %d: Failed to preallocate " > + "space to %ld bytes.", errno, (long) alloc_size); > + } Use do_fallocate here? It already did the EOPNOTSUPP check. And introduce another flag, e.g. prealloc, to save the fallocate support status? So that test0[7-9] don't have to do the EOPNOTSUPP check again. > + fprintf(stderr, " Skipping unwritten extent tests.\n"); > + goto out; > + } else { > + pos = lseek(fd, 0, SEEK_DATA); Hmm, it's hard to tell if it's a bug in lseek or the fs doesn't support unwritten extents, because we're going to test lseek SEEK_DATA/HOLE interface. How about using fiemap and check the FIEMAP_EXTENT_UNWRITTEN flag? Only falling back to lseek if fiemap is not supported? Thanks, Eryu > + if (pos == 0) { > + fprintf(stderr, "File system does not support unwritten extents.\n"); > + goto out; > + } > + } > + unwritten_extents = 1; > + > printf("\n"); > > out: > -- > 2.7.4 > > -- > To unsubscribe from this list: send the line "unsubscribe fstests" 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 fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 12, 2017 at 7:09 AM, Eryu Guan <eguan@redhat.com> wrote: > On Thu, May 11, 2017 at 01:35:07PM +0200, Andreas Gruenbacher wrote: >> src/seek_sanity_test (test generic/285) assumes that after preallocating >> space in a file with fallocate, fseek SEEK_HOLE / SEEK_DATA will still >> report the allocated space as a hole. On filesystems without unwritten >> extent support, that space will be reported as data, though. >> >> Tested on ext4, xfs, and gfs2 + patches for fseek SEEK_HOLE / SEEK_DATA >> support. > > The idea seems fine to me, but I'm not that familiar with SEEK_DATA/HOLE > support, it'd be great if someone else could help review this patch. > > Some of my thoughts inline. > >> >> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> >> --- >> src/seek_sanity_test.c | 35 +++++++++++++++++++++++++++++++++++ >> 1 file changed, 35 insertions(+) >> >> diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c >> index a6dd48c..0d7fa0a 100644 >> --- a/src/seek_sanity_test.c >> +++ b/src/seek_sanity_test.c >> @@ -37,6 +37,7 @@ >> >> static blksize_t alloc_size; >> int default_behavior = 0; >> +int unwritten_extents = 0; >> char *base_file_path; >> >> static void get_file_system(int fd) >> @@ -282,6 +283,11 @@ static int test09(int fd, int testnum) >> int bufsz = alloc_size; >> int filsz = 8 << 20; >> >> + if (!unwritten_extents) { >> + fprintf(stdout, "Test skipped\n"); >> + goto out; >> + } >> + >> /* >> * HOLE - unwritten DATA in dirty page - HOLE - >> * unwritten DATA in writeback page >> @@ -338,6 +344,11 @@ static int test08(int fd, int testnum) >> int bufsz = alloc_size; >> int filsz = 4 << 20; >> >> + if (!unwritten_extents) { >> + fprintf(stdout, "Test skipped\n"); >> + goto out; >> + } >> + >> /* HOLE - unwritten DATA in writeback page */ >> /* Each unit is bufsz */ >> buf = do_malloc(bufsz); >> @@ -387,6 +398,11 @@ static int test07(int fd, int testnum) >> int bufsz = alloc_size; >> int filsz = 4 << 20; >> >> + if (!unwritten_extents) { >> + fprintf(stdout, "Test skipped\n"); >> + goto out; >> + } >> + >> /* HOLE - unwritten DATA in dirty page */ >> /* Each unit is bufsz */ >> buf = do_malloc(bufsz); >> @@ -776,6 +792,25 @@ static int test_basic_support(void) >> fprintf(stderr, "File system supports the default behavior.\n"); >> } >> >> + ftruncate(fd, 0); >> + if (fallocate(fd, 0, 0, alloc_size) == -1) { >> + if (errno == EOPNOTSUPP) { >> + fprintf(stderr, "File system does not support fallocate."); >> + } else { >> + fprintf(stderr, "ERROR %d: Failed to preallocate " >> + "space to %ld bytes.", errno, (long) alloc_size); >> + } > > Use do_fallocate here? It already did the EOPNOTSUPP check. And > introduce another flag, e.g. prealloc, to save the fallocate support > status? So that test0[7-9] don't have to do the EOPNOTSUPP check again. In fact unwritten extent support implies fallocate support, so we can get rid of the redundant checks for missing fallocate support. >> + fprintf(stderr, " Skipping unwritten extent tests.\n"); >> + goto out; >> + } else { >> + pos = lseek(fd, 0, SEEK_DATA); > > Hmm, it's hard to tell if it's a bug in lseek or the fs doesn't support > unwritten extents, because we're going to test lseek SEEK_DATA/HOLE > interface. > > How about using fiemap and check the FIEMAP_EXTENT_UNWRITTEN flag? Only > falling back to lseek if fiemap is not supported? Wouldn't this only complicate and open things up to fiemap bugs in addition? I'll send an updated patch. Thanks, Andreas -- To unsubscribe from this list: send the line "unsubscribe fstests" 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/src/seek_sanity_test.c b/src/seek_sanity_test.c index a6dd48c..0d7fa0a 100644 --- a/src/seek_sanity_test.c +++ b/src/seek_sanity_test.c @@ -37,6 +37,7 @@ static blksize_t alloc_size; int default_behavior = 0; +int unwritten_extents = 0; char *base_file_path; static void get_file_system(int fd) @@ -282,6 +283,11 @@ static int test09(int fd, int testnum) int bufsz = alloc_size; int filsz = 8 << 20; + if (!unwritten_extents) { + fprintf(stdout, "Test skipped\n"); + goto out; + } + /* * HOLE - unwritten DATA in dirty page - HOLE - * unwritten DATA in writeback page @@ -338,6 +344,11 @@ static int test08(int fd, int testnum) int bufsz = alloc_size; int filsz = 4 << 20; + if (!unwritten_extents) { + fprintf(stdout, "Test skipped\n"); + goto out; + } + /* HOLE - unwritten DATA in writeback page */ /* Each unit is bufsz */ buf = do_malloc(bufsz); @@ -387,6 +398,11 @@ static int test07(int fd, int testnum) int bufsz = alloc_size; int filsz = 4 << 20; + if (!unwritten_extents) { + fprintf(stdout, "Test skipped\n"); + goto out; + } + /* HOLE - unwritten DATA in dirty page */ /* Each unit is bufsz */ buf = do_malloc(bufsz); @@ -776,6 +792,25 @@ static int test_basic_support(void) fprintf(stderr, "File system supports the default behavior.\n"); } + ftruncate(fd, 0); + if (fallocate(fd, 0, 0, alloc_size) == -1) { + if (errno == EOPNOTSUPP) { + fprintf(stderr, "File system does not support fallocate."); + } else { + fprintf(stderr, "ERROR %d: Failed to preallocate " + "space to %ld bytes.", errno, (long) alloc_size); + } + fprintf(stderr, " Skipping unwritten extent tests.\n"); + goto out; + } else { + pos = lseek(fd, 0, SEEK_DATA); + if (pos == 0) { + fprintf(stderr, "File system does not support unwritten extents.\n"); + goto out; + } + } + unwritten_extents = 1; + printf("\n"); out:
src/seek_sanity_test (test generic/285) assumes that after preallocating space in a file with fallocate, fseek SEEK_HOLE / SEEK_DATA will still report the allocated space as a hole. On filesystems without unwritten extent support, that space will be reported as data, though. Tested on ext4, xfs, and gfs2 + patches for fseek SEEK_HOLE / SEEK_DATA support. Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> --- src/seek_sanity_test.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)