| Message ID | d8ad4bab26557fdf70e7ebc3f771bbb37ae889d7.1574093242.git.jstancek@redhat.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | fallocate05: increase FALLOCATE_SIZE | expand |
On 11/18/19 5:08 PM, Jan Stancek wrote: > write() returning ENOSPC doesn't guarantee that filesystem after > some internal book-keeping, flushing, finishing transactions, etc. > won't still find some extra space. > > Increase FALLOCATE_SIZE to minimize chance of hitting sporadic > failures when that happens. We're planning to rewrite fallocate05 this week and FALLOCATE_SIZE will be removed entirely. The test must use a multiple of the real file system block size, otherwise it'll test different things on different platforms.
Hi! > write() returning ENOSPC doesn't guarantee that filesystem after > some internal book-keeping, flushing, finishing transactions, etc. > won't still find some extra space. > > Increase FALLOCATE_SIZE to minimize chance of hitting sporadic > failures when that happens. > > Thanks to Carlos Maiolino and Eric Sandeen for their comments > and suggestions. Acked.
Hi Jan, On Tue, Nov 19, 2019 at 12:08 AM Jan Stancek <jstancek@redhat.com> wrote: > write() returning ENOSPC doesn't guarantee that filesystem after > some internal book-keeping, flushing, finishing transactions, etc. > won't still find some extra space. > Thanks for the patch, I have drafted a similar one but you sent out first :). Another patch I was thinking is to enhance the tst_fill_fs routine, which as Eric suggested, makes more reliably to get to a full filesystem. Something like what xfstest does to cut the trial write size in half and try again until the size is less than the filesystem block size. Comments? --- a/lib/tst_fill_fs.c +++ b/lib/tst_fill_fs.c @@ -6,6 +6,7 @@ #include <errno.h> #include <stdio.h> #include <stdlib.h> +#include <sys/statvfs.h> #define TST_NO_DEFAULT_MAIN #include "tst_test.h" @@ -19,6 +20,8 @@ void tst_fill_fs(const char *path, int verbose) size_t len; ssize_t ret; int fd; + struct statvfs fi; + statvfs(path, &fi); for (;;) { len = random() % (1024 * 102400); @@ -41,6 +44,12 @@ void tst_fill_fs(const char *path, int verbose) ret = write(fd, buf, MIN(len, sizeof(buf))); if (ret < 0) { + if (errno == ENOSPC) { + len /= 2; + if (len >= fi.f_bsize) + continue; + } + SAFE_CLOSE(fd); if (errno != ENOSPC) > > Increase FALLOCATE_SIZE to minimize chance of hitting sporadic > failures when that happens. > > Thanks to Carlos Maiolino and Eric Sandeen for their comments > and suggestions. > > Fixes #610 > Signed-off-by: Jan Stancek <jstancek@redhat.com> > Reviewed-by: Li Wang <liwang@redhat.com> > --- > testcases/kernel/syscalls/fallocate/fallocate05.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/testcases/kernel/syscalls/fallocate/fallocate05.c > b/testcases/kernel/syscalls/fallocate/fallocate05.c > index 50c610c448ba..17034e5b11e7 100644 > --- a/testcases/kernel/syscalls/fallocate/fallocate05.c > +++ b/testcases/kernel/syscalls/fallocate/fallocate05.c > @@ -17,7 +17,7 @@ > #include "lapi/fallocate.h" > > #define MNTPOINT "mntpoint" > -#define FALLOCATE_SIZE 8192 > +#define FALLOCATE_SIZE (1024*1024) > #define TESTED_FLAGS "fallocate(FALLOC_FL_PUNCH_HOLE | > FALLOC_FL_KEEP_SIZE)" > > static int fd; > -- > 1.8.3.1 > >
On Tue, Nov 19, 2019 at 12:19 AM Martin Doucha <mdoucha@suse.cz> wrote: > On 11/18/19 5:08 PM, Jan Stancek wrote: > > write() returning ENOSPC doesn't guarantee that filesystem after > > some internal book-keeping, flushing, finishing transactions, etc. > > won't still find some extra space. > > > > Increase FALLOCATE_SIZE to minimize chance of hitting sporadic > > failures when that happens. > > We're planning to rewrite fallocate05 this week and FALLOCATE_SIZE will > be removed entirely. The test must use a multiple of the real file > system block size, otherwise it'll test different things on different > platforms. > Sounds good. Thanks! But it'd be better to merge Jan's patch first because there will still need time for new patch reviewing.
----- Original Message ----- > Another patch I was thinking is to enhance the tst_fill_fs routine, which > as Eric suggested, makes more reliably to get to a full filesystem. > Something like what xfstest does to cut the trial write size in half and > try again until the size is less than the filesystem block size. > > Comments? fallocate05 seems to be the only test using it, but in general I think we can do that too. Assuming this alone would be reliable, is there any advantage of running test with small FALLOCATE_SIZE? > > --- a/lib/tst_fill_fs.c > +++ b/lib/tst_fill_fs.c > @@ -6,6 +6,7 @@ > #include <errno.h> > #include <stdio.h> > #include <stdlib.h> > +#include <sys/statvfs.h> > > #define TST_NO_DEFAULT_MAIN > #include "tst_test.h" > @@ -19,6 +20,8 @@ void tst_fill_fs(const char *path, int verbose) > size_t len; > ssize_t ret; > int fd; > + struct statvfs fi; > + statvfs(path, &fi); > > for (;;) { > len = random() % (1024 * 102400); > @@ -41,6 +44,12 @@ void tst_fill_fs(const char *path, int verbose) > ret = write(fd, buf, MIN(len, sizeof(buf))); > > if (ret < 0) { > + if (errno == ENOSPC) { > + len /= 2; > + if (len >= fi.f_bsize) > + continue; > + } > + > SAFE_CLOSE(fd); > > if (errno != ENOSPC) > > > > > > > Increase FALLOCATE_SIZE to minimize chance of hitting sporadic > > failures when that happens. > > > > Thanks to Carlos Maiolino and Eric Sandeen for their comments > > and suggestions. > > > > Fixes #610 > > Signed-off-by: Jan Stancek <jstancek@redhat.com> > > > Reviewed-by: Li Wang <liwang@redhat.com> Thanks, I pushed this patch for now.
Hi, On Tue, Nov 19, 2019 at 4:13 PM Jan Stancek <jstancek@redhat.com> wrote: > > > ----- Original Message ----- > > Another patch I was thinking is to enhance the tst_fill_fs routine, which > > as Eric suggested, makes more reliably to get to a full filesystem. > > Something like what xfstest does to cut the trial write size in half and > > try again until the size is less than the filesystem block size. > > > > Comments? > > fallocate05 seems to be the only test using it, but in general I think we > Thanks, I will format a patch. > can do that too. Assuming this alone would be reliable, is there any > advantage of running test with small FALLOCATE_SIZE? > Hmm, maybe no, my purpose to shrink the FALLOCATE_SIZE is just to reproduce the problem. I thought that if the filesystem is not 'really' full, then the fallocate() should succeed with a smaller size, only for degging. > > > > > --- a/lib/tst_fill_fs.c > > +++ b/lib/tst_fill_fs.c > > @@ -6,6 +6,7 @@ > > #include <errno.h> > > #include <stdio.h> > > #include <stdlib.h> > > +#include <sys/statvfs.h> > > > > #define TST_NO_DEFAULT_MAIN > > #include "tst_test.h" > > @@ -19,6 +20,8 @@ void tst_fill_fs(const char *path, int verbose) > > size_t len; > > ssize_t ret; > > int fd; > > + struct statvfs fi; > > + statvfs(path, &fi); > > > > for (;;) { > > len = random() % (1024 * 102400); > > @@ -41,6 +44,12 @@ void tst_fill_fs(const char *path, int verbose) > > ret = write(fd, buf, MIN(len, sizeof(buf))); > > > > if (ret < 0) { > > + if (errno == ENOSPC) { > > + len /= 2; > > + if (len >= fi.f_bsize) > > + continue; > > + } > > + > > SAFE_CLOSE(fd); > > > > if (errno != ENOSPC) > > > > > > > > > > > > Increase FALLOCATE_SIZE to minimize chance of hitting sporadic > > > failures when that happens. > > > > > > Thanks to Carlos Maiolino and Eric Sandeen for their comments > > > and suggestions. > > > > > > Fixes #610 > > > Signed-off-by: Jan Stancek <jstancek@redhat.com> > > > > > Reviewed-by: Li Wang <liwang@redhat.com> > > Thanks, I pushed this patch for now. > >
On 11/19/19 9:13 AM, Jan Stancek wrote: > > > ----- Original Message ----- >> Another patch I was thinking is to enhance the tst_fill_fs routine, which >> as Eric suggested, makes more reliably to get to a full filesystem. >> Something like what xfstest does to cut the trial write size in half and >> try again until the size is less than the filesystem block size. >> >> Comments? > > fallocate05 seems to be the only test using it, but in general I think we > can do that too. Assuming this alone would be reliable, is there any > advantage of running test with small FALLOCATE_SIZE? Note that simply increasing FALLOCATE_SIZE will not fix an incorrect pass when the file system wasn't completely full. Here's the code that checks whether some blocks were properly fallocate()d: tst_fill_fs(MNTPOINT, 1); ret = write(fd, buf, sizeof(buf)); if (ret < 0) tst_res(TFAIL | TERRNO, "write() failed unexpectedly"); else tst_res(TPASS, "write() wrote %zu bytes", ret); If the file system somehow finds a few free blocks after tst_fill_fs() returns, short write() will still count as a pass.
----- Original Message ----- > > ----- Original Message ----- > > fallocate05 seems to be the only test using it, but in general I think we > > can do that too. Assuming this alone would be reliable, is there any > > advantage of running test with small FALLOCATE_SIZE? > > Note that simply increasing FALLOCATE_SIZE will not fix an incorrect > pass when the file system wasn't completely full. Here's the code that > checks whether some blocks were properly fallocate()d: > > tst_fill_fs(MNTPOINT, 1); > ret = write(fd, buf, sizeof(buf)); > if (ret < 0) > tst_res(TFAIL | TERRNO, "write() failed unexpectedly"); > else > tst_res(TPASS, "write() wrote %zu bytes", ret); > > If the file system somehow finds a few free blocks after tst_fill_fs() > returns, short write() will still count as a pass. That is good point, but that seems like issue that existed even with 8k FALLOCATE_SIZE, right?
diff --git a/testcases/kernel/syscalls/fallocate/fallocate05.c b/testcases/kernel/syscalls/fallocate/fallocate05.c index 50c610c448ba..17034e5b11e7 100644 --- a/testcases/kernel/syscalls/fallocate/fallocate05.c +++ b/testcases/kernel/syscalls/fallocate/fallocate05.c @@ -17,7 +17,7 @@ #include "lapi/fallocate.h" #define MNTPOINT "mntpoint" -#define FALLOCATE_SIZE 8192 +#define FALLOCATE_SIZE (1024*1024) #define TESTED_FLAGS "fallocate(FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE)" static int fd;
write() returning ENOSPC doesn't guarantee that filesystem after some internal book-keeping, flushing, finishing transactions, etc. won't still find some extra space. Increase FALLOCATE_SIZE to minimize chance of hitting sporadic failures when that happens. Thanks to Carlos Maiolino and Eric Sandeen for their comments and suggestions. Fixes #610 Signed-off-by: Jan Stancek <jstancek@redhat.com> --- testcases/kernel/syscalls/fallocate/fallocate05.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)