Hi,
On Wed, 2015-05-20 at 02:52 -0400, Jan Stancek wrote:
>
>
>
> ----- Original Message -----
> > From: "Zeng Linggang" <zenglg...@cn.fujitsu.com>
> > To: "Jan Stancek" <jstan...@redhat.com>
> > Cc: "Alexey Kodanev" <alexey.koda...@oracle.com>, "vasily isaenko"
> > <vasily.isae...@oracle.com>,
> > ltp-list@lists.sourceforge.net
> > Sent: Wednesday, 20 May, 2015 3:47:22 AM
> > Subject: Re: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is
> > not supported
> >
> > Hi!
> >
> > On Tue, 2015-05-19 at 07:18 -0400, Jan Stancek wrote:
> > >
> > >
> > >
> > > ----- Original Message -----
> > > > From: "Zeng Linggang" <zenglg...@cn.fujitsu.com>
> > > > To: "Alexey Kodanev" <alexey.koda...@oracle.com>
> > > > Cc: "vasily isaenko" <vasily.isae...@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_zhang...@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.
>
> Can you explain why?
>
> >
> > >
> > > 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.
>
> That code is already there, your patch is not removing 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) ?
>
> I think we should do the check on all kernels. lseek() can only check that the
> hole exists, but current check also verifies that hole has correct size and
> content.
>
Right. It is the strict way we need to do. But it would make the test complex.
Now, "turn that warning into TINFO" looks simple but effective.
If nobody reject, I will send new patch like:
- tst_resm(TWARN | TERRNO, "lseek() doesn't support SEEK_HOLE");
+ if (tst_kvercmp(3, 1, 0) < 0)
+ 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);
Thank you.
Best regards,
Zeng
> Regards,
> Jan
[...]
------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
