[PATCH] generic/297: fix the delta time based on stat

[PATCH] generic/297: fix the delta time based on stat

[Wang Yugui]

stat -c '%Y' report seconds as int, so the delta 2.01s may result as 3s.

Signed-off-by: Wang Yugui <wangyugui@e16-tech.com>
---
 tests/generic/297 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/generic/297 b/tests/generic/297
index 6bdc3e1c..e3082202 100755
--- a/tests/generic/297
+++ b/tests/generic/297
@@ -51,7 +51,7 @@ for i in $(seq 0 $fnr); do
 	touch $TEST_DIR/after
 	before=$(stat -c '%Y' $TEST_DIR/before)
 	after=$(stat -c '%Y' $TEST_DIR/after)
-	delta=$((after - before))
+	delta=$((after - before -1)) # 2.01s may result as 3s; so -1
 	test $delta -gt $timeout && break
 done
 
@@ -64,7 +64,7 @@ $TIMEOUT_PROG -s INT ${kill_after}s $XFS_IO_PROG -f -c "reflink $testdir/file1 0
 touch $TEST_DIR/after
 before=$(stat -c '%Y' $TEST_DIR/before)
 after=$(stat -c '%Y' $TEST_DIR/after)
-delta=$((after - before))
+delta=$((after - before - 1)) # 2.01s may result as 3s; so -1
 echo "reflink of $n bytes took $delta seconds" >> $seqres.full
 test $delta -gt $timeout && _fail "reflink didn't stop in time, n=$n t=$delta"
 
-- 
2.36.1

Re: [PATCH] generic/297: fix the delta time based on stat

[Zorro Lang]

On Fri, Jun 10, 2022 at 10:35:53AM +0800, Wang Yugui wrote:
> stat -c '%Y' report seconds as int, so the delta 2.01s may result as 3s.
> 
> Signed-off-by: Wang Yugui <wangyugui@e16-tech.com>
> ---
>  tests/generic/297 | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/generic/297 b/tests/generic/297
> index 6bdc3e1c..e3082202 100755
> --- a/tests/generic/297
> +++ b/tests/generic/297
> @@ -51,7 +51,7 @@ for i in $(seq 0 $fnr); do
>  	touch $TEST_DIR/after
>  	before=$(stat -c '%Y' $TEST_DIR/before)
>  	after=$(stat -c '%Y' $TEST_DIR/after)
> -	delta=$((after - before))
> +	delta=$((after - before -1)) # 2.01s may result as 3s; so -1

What issue is this change trying to fix? "timeout=8"s is not long enough?

Thanks,
Zorro

>  	test $delta -gt $timeout && break
>  done
>  
> @@ -64,7 +64,7 @@ $TIMEOUT_PROG -s INT ${kill_after}s $XFS_IO_PROG -f -c "reflink $testdir/file1 0
>  touch $TEST_DIR/after
>  before=$(stat -c '%Y' $TEST_DIR/before)
>  after=$(stat -c '%Y' $TEST_DIR/after)
> -delta=$((after - before))
> +delta=$((after - before - 1)) # 2.01s may result as 3s; so -1
>  echo "reflink of $n bytes took $delta seconds" >> $seqres.full
>  test $delta -gt $timeout && _fail "reflink didn't stop in time, n=$n t=$delta"
>  
> -- 
> 2.36.1
> 

Re: [PATCH] generic/297: fix the delta time based on stat

[Wang Yugui]

Hi,

> On Fri, Jun 10, 2022 at 10:35:53AM +0800, Wang Yugui wrote:
> > stat -c '%Y' report seconds as int, so the delta 2.01s may result as 3s.
> > 
> > Signed-off-by: Wang Yugui <wangyugui@e16-tech.com>
> > ---
> >  tests/generic/297 | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/generic/297 b/tests/generic/297
> > index 6bdc3e1c..e3082202 100755
> > --- a/tests/generic/297
> > +++ b/tests/generic/297
> > @@ -51,7 +51,7 @@ for i in $(seq 0 $fnr); do
> >  	touch $TEST_DIR/after
> >  	before=$(stat -c '%Y' $TEST_DIR/before)
> >  	after=$(stat -c '%Y' $TEST_DIR/after)
> > -	delta=$((after - before))
> > +	delta=$((after - before -1)) # 2.01s may result as 3s; so -1
> 
> What issue is this change trying to fix? "timeout=8"s is not long enough?

for the command
$TIMEOUT_PROG -s INT ${kill_after}s

delta=$((after - before )) may report 'kill_after+1' in some case.

so no relationship to "timeout=8" or "timeout=2".

'$XFS_IO_PROG -f -c reflink' without '$TIMEOUT_PROG -s INT ${kill_after}s'
may take 20s because this is a very complex reflink.

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/06/10

Re: [PATCH] generic/297: fix the delta time based on stat

[Dave Chinner]

On Fri, Jun 10, 2022 at 12:36:24PM +0800, Wang Yugui wrote:
> Hi,
> 
> > On Fri, Jun 10, 2022 at 10:35:53AM +0800, Wang Yugui wrote:
> > > stat -c '%Y' report seconds as int, so the delta 2.01s may result as 3s.
> > > 
> > > Signed-off-by: Wang Yugui <wangyugui@e16-tech.com>
> > > ---
> > >  tests/generic/297 | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tests/generic/297 b/tests/generic/297
> > > index 6bdc3e1c..e3082202 100755
> > > --- a/tests/generic/297
> > > +++ b/tests/generic/297
> > > @@ -51,7 +51,7 @@ for i in $(seq 0 $fnr); do
> > >  	touch $TEST_DIR/after
> > >  	before=$(stat -c '%Y' $TEST_DIR/before)
> > >  	after=$(stat -c '%Y' $TEST_DIR/after)
> > > -	delta=$((after - before))
> > > +	delta=$((after - before -1)) # 2.01s may result as 3s; so -1
> > 
> > What issue is this change trying to fix? "timeout=8"s is not long enough?
> 
> for the command
> $TIMEOUT_PROG -s INT ${kill_after}s
> 
> delta=$((after - before )) may report 'kill_after+1' in some case.

Yes, that's what it is supposed to report. The process is
killed 2s after the test starts, so delta is going to be kill_after
+ the delay for the task to exit and the 'touch $TEST_DIR/after'
command to run. This can take a second or two, depending on how fast
the reflink operation is proceeding (e.g. waiting on IO).

> so no relationship to "timeout=8" or "timeout=2".

If the reflink doesn't get killed within 6s then delta will be
greater than the timeout (8) and the test fails. So if delta is 2 or
3, then it makes no difference to the test result, right?

> '$XFS_IO_PROG -f -c reflink' without '$TIMEOUT_PROG -s INT ${kill_after}s'
> may take 20s because this is a very complex reflink.

Yes, and if it takes 20s (i.e. delta=20s) then it means that the
kill signal was not delivered and acted upon correctly and so the
test should fail.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] generic/297: fix the delta time based on stat

[Wang Yugui]

Hi,

> On Fri, Jun 10, 2022 at 12:36:24PM +0800, Wang Yugui wrote:
> > Hi,
> > 
> > > On Fri, Jun 10, 2022 at 10:35:53AM +0800, Wang Yugui wrote:
> > > > stat -c '%Y' report seconds as int, so the delta 2.01s may result as 3s.
> > > > 
> > > > Signed-off-by: Wang Yugui <wangyugui@e16-tech.com>
> > > > ---
> > > >  tests/generic/297 | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/tests/generic/297 b/tests/generic/297
> > > > index 6bdc3e1c..e3082202 100755
> > > > --- a/tests/generic/297
> > > > +++ b/tests/generic/297
> > > > @@ -51,7 +51,7 @@ for i in $(seq 0 $fnr); do
> > > >  	touch $TEST_DIR/after
> > > >  	before=$(stat -c '%Y' $TEST_DIR/before)
> > > >  	after=$(stat -c '%Y' $TEST_DIR/after)
> > > > -	delta=$((after - before))
> > > > +	delta=$((after - before -1)) # 2.01s may result as 3s; so -1
> > > 
> > > What issue is this change trying to fix? "timeout=8"s is not long enough?
> > 
> > for the command
> > $TIMEOUT_PROG -s INT ${kill_after}s
> > 
> > delta=$((after - before )) may report 'kill_after+1' in some case.
> 
> Yes, that's what it is supposed to report. The process is
> killed 2s after the test starts, so delta is going to be kill_after
> + the delay for the task to exit and the 'touch $TEST_DIR/after'
> command to run. This can take a second or two, depending on how fast
> the reflink operation is proceeding (e.g. waiting on IO).
> 
> > so no relationship to "timeout=8" or "timeout=2".
> 
> If the reflink doesn't get killed within 6s then delta will be
> greater than the timeout (8) and the test fails. So if delta is 2 or
> 3, then it makes no difference to the test result, right?
> 
> > '$XFS_IO_PROG -f -c reflink' without '$TIMEOUT_PROG -s INT ${kill_after}s'
> > may take 20s because this is a very complex reflink.
> 
> Yes, and if it takes 20s (i.e. delta=20s) then it means that the
> kill signal was not delivered and acted upon correctly and so the
> test should fail.

If we want to detect the 'timeout', a simple way it to check
the return value of '$TIMEOUT_PROG '

#timeout exit with status 124.

now we just fix this delta value error because of float/int.

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/06/10

Re: [PATCH] generic/297: fix the delta time based on stat

[Zorro Lang]

On Fri, Jun 10, 2022 at 12:36:24PM +0800, Wang Yugui wrote:
> Hi,
> 
> > On Fri, Jun 10, 2022 at 10:35:53AM +0800, Wang Yugui wrote:
> > > stat -c '%Y' report seconds as int, so the delta 2.01s may result as 3s.
> > > 
> > > Signed-off-by: Wang Yugui <wangyugui@e16-tech.com>
> > > ---
> > >  tests/generic/297 | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tests/generic/297 b/tests/generic/297
> > > index 6bdc3e1c..e3082202 100755
> > > --- a/tests/generic/297
> > > +++ b/tests/generic/297
> > > @@ -51,7 +51,7 @@ for i in $(seq 0 $fnr); do
> > >  	touch $TEST_DIR/after
> > >  	before=$(stat -c '%Y' $TEST_DIR/before)
> > >  	after=$(stat -c '%Y' $TEST_DIR/after)
> > > -	delta=$((after - before))
> > > +	delta=$((after - before -1)) # 2.01s may result as 3s; so -1
> > 
> > What issue is this change trying to fix? "timeout=8"s is not long enough?
> 
> for the command
> $TIMEOUT_PROG -s INT ${kill_after}s
> 
> delta=$((after - before )) may report 'kill_after+1' in some case.
> 
> so no relationship to "timeout=8" or "timeout=2".
> 
> '$XFS_IO_PROG -f -c reflink' without '$TIMEOUT_PROG -s INT ${kill_after}s'
> may take 20s because this is a very complex reflink.

Hmm... still don't understand what are you trying to fix.

That reflink operation need long time, that's expected. The first time it's
running in a while loop for getting a proper testing size, no matter 8.01s
or 9s, I think it's all good.

The second time the reflink run with `TIMEOUT_PROG 2s`, we expect it can be
killed in 'timeout=8s', that's long enough. I think it's not necessary to
care about it's keep running in 8.01s or 9s.

Thanks,
Zorro

> 
> Best Regards
> Wang Yugui (wangyugui@e16-tech.com)
> 2022/06/10
> 
> 
> 

Re: [PATCH] generic/297: fix the delta time based on stat

[Wang Yugui]

Hi,

> On Fri, Jun 10, 2022 at 12:36:24PM +0800, Wang Yugui wrote:
> > Hi,
> > 
> > > On Fri, Jun 10, 2022 at 10:35:53AM +0800, Wang Yugui wrote:
> > > > stat -c '%Y' report seconds as int, so the delta 2.01s may result as 3s.
> > > > 
> > > > Signed-off-by: Wang Yugui <wangyugui@e16-tech.com>
> > > > ---
> > > >  tests/generic/297 | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/tests/generic/297 b/tests/generic/297
> > > > index 6bdc3e1c..e3082202 100755
> > > > --- a/tests/generic/297
> > > > +++ b/tests/generic/297
> > > > @@ -51,7 +51,7 @@ for i in $(seq 0 $fnr); do
> > > >  	touch $TEST_DIR/after
> > > >  	before=$(stat -c '%Y' $TEST_DIR/before)
> > > >  	after=$(stat -c '%Y' $TEST_DIR/after)
> > > > -	delta=$((after - before))
> > > > +	delta=$((after - before -1)) # 2.01s may result as 3s; so -1
> > > 
> > > What issue is this change trying to fix? "timeout=8"s is not long enough?
> > 
> > for the command
> > $TIMEOUT_PROG -s INT ${kill_after}s
> > 
> > delta=$((after - before )) may report 'kill_after+1' in some case.
> > 
> > so no relationship to "timeout=8" or "timeout=2".
> > 
> > '$XFS_IO_PROG -f -c reflink' without '$TIMEOUT_PROG -s INT ${kill_after}s'
> > may take 20s because this is a very complex reflink.
> 
> Hmm... still don't understand what are you trying to fix.
> 
> That reflink operation need long time, that's expected. The first time it's
> running in a while loop for getting a proper testing size, no matter 8.01s
> or 9s, I think it's all good.
> 
> The second time the reflink run with `TIMEOUT_PROG 2s`, we expect it can be
> killed in 'timeout=8s', that's long enough. I think it's not necessary to
> care about it's keep running in 8.01s or 9s.

drop this patch please.

btrfs of generic/297 has a few freqency to fail.

maybe because  $XFS_IO_PROG is interrupt by timeout,
but the reflink inside kernel continue to finish.

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/06/10