Re: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported

Zeng Linggang
2015-05-20T01:46:46Z

Hi!

On Tue, 2015-05-19 at 07:18 -0400, Jan Stancek wrote:
>
>
>
> ----- Original Message -----
> > From: "Zeng Linggang" <zenglg.jy@cn.fujitsu.com>
> > To: "Alexey Kodanev" <alexey.kodanev@oracle.com>
> > Cc: "vasily isaenko" <vasily.isaenko@oracle.com>, ltp-list@lists.sourceforge.net
> > Sent: Tuesday, 19 May, 2015 10:50:22 AM
> > Subject: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported
> >
> > SEEK_HOLE is only supported since version 3.1. Check the specified
> > range blocks are zeroed while the kernel does not supported SEEK_HOLE.
> >
> > Signed-off-by: Zhang Jin <jy_zhangjin@cn.fujitsu.com>
> > ---
> > testcases/kernel/syscalls/fallocate/fallocate04.c | 13 ++++++++++---
> > 1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c
> > b/testcases/kernel/syscalls/fallocate/fallocate04.c
> > index 911bbe8..e94c572 100644
> > --- a/testcases/kernel/syscalls/fallocate/fallocate04.c
> > +++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
> > @@ -98,13 +98,13 @@ static void setup(void)
> > get_blocksize();
> > }
> >
> > -static void check_file_data(const char exp_buf[], size_t size)
> > +static void check_file_data_(const char exp_buf[], size_t size, off_t
> > offset)
> > {
> > char rbuf[size];
> >
> > tst_resm(TINFO, "reading the file, compare with expected buffer");
> >
> > - SAFE_LSEEK(cleanup, fd, 0, SEEK_SET);
> > + SAFE_LSEEK(cleanup, fd, offset, SEEK_SET);
> > SAFE_READ(cleanup, 1, fd, rbuf, size);
> >
> > if (memcmp(exp_buf, rbuf, size)) {
> > @@ -116,6 +116,11 @@ static void check_file_data(const char exp_buf[], size_t
> > size)
> > }
> > }
> >
> > +static inline void check_file_data(const char exp_buf[], size_t size)
> > +{
> > + check_file_data_(exp_buf, size, 0);
> > +}
> > +
> > static void test01(void)
> > {
> > tst_resm(TINFO, "allocate '%zu' bytes", buf_size);
> > @@ -158,7 +163,9 @@ static void test02(void)
> > tst_brkm(TFAIL | TERRNO, cleanup,
> > "fallocate() or lseek() failed");
> > }
> > - tst_resm(TWARN | TERRNO, "lseek() doesn't support SEEK_HOLE");
> > + char zeros[block_size];
> > + memset(zeros, 0, block_size);
> > + check_file_data_(zeros, block_size, block_size);
>
> Hi,
>
> Isn't this redundant?

To be honest, I do not think it is redundant.

>
> Couple lines below is this check, which also checks that range
> <block_size, block_size*2> is zeroed.

Yep, this looks simple.
This maybe cost more runtime because of 'fill_tst_buf'.
But I do not reject it.

>
> char exp_buf[buf_size];
>
> fill_tst_buf(exp_buf);
> memset(exp_buf + block_size, 0, block_size);
> check_file_data(exp_buf, buf_size);
>
> Wouldn't it be enough to turn that warning into TINFO, for kernels < 3.1?
>
> diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c b/testcases/kernel/syscalls/fallocate/fallocate04.c
> index 911bbe8..45d9827 100644
> --- a/testcases/kernel/syscalls/fallocate/fallocate04.c
> +++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
> @@ -158,9 +158,12 @@ static void test02(void)
> tst_brkm(TFAIL | TERRNO, cleanup,
> "fallocate() or lseek() failed");
> }
> - tst_resm(TWARN | TERRNO, "lseek() doesn't support SEEK_HOLE");
> + if (tst_kvercmp(3, 1, 0) < 0)

How about put them before lseek(SEEK_HOLE) ?

if (tst_kvercmp(3, 1, 0) < 0) {
fill_tst_buf();
memset();
check_file_data();
} else {
lseek(SEEK_HOLE);
...
}

Thank you very much.

Best regards,
Zeng

> + tst_resm(TINFO, "lseek() doesn't support SEEK_HOLE, "
> + "this is expected for < 3.1 kernels");
> + } else {
> + tst_resm(TINFO, "found a hole at '%ld' offset", ret);
> }
> - tst_resm(TINFO, "found a hole at '%ld' offset", ret);
>
> size_t alloc_size1 = get_allocsize();
>
> Regards,
> Jan