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

[Zeng Linggang <zenglg.jy@cn.fujitsu.com>]

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



------------------------------------------------------------------------------
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