* [PATCH] generic/{175,297,298}: fix use of uninitialized var
@ 2023-08-12 15:15 Amir Goldstein
2023-08-13 14:52 ` Zorro Lang
2023-08-13 15:35 ` Darrick J. Wong
0 siblings, 2 replies; 5+ messages in thread
From: Amir Goldstein @ 2023-08-12 15:15 UTC (permalink / raw)
To: Zorro Lang; +Cc: Darrick J . Wong, Chandan Babu R, Luis Chamberlain, fstests
Not sure how those tests pass in regression tests.
Probably truncate silently fails and is not critical to the test.
in kdevops I get errors like:
/data/fstests-install/xfstests/tests/generic/298: line 45: /dev/loop12):
syntax error: operand expected (error token is "/dev/loop12)")
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
tests/generic/175 | 2 +-
tests/generic/297 | 2 +-
tests/generic/298 | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/generic/175 b/tests/generic/175
index 07af2199..14825a39 100755
--- a/tests/generic/175
+++ b/tests/generic/175
@@ -33,7 +33,7 @@ _pwrite_byte 0x61 0 $blksz "$testdir/file1" >> "$seqres.full"
fnr=19
echo "Create extents"
-truncate -s $(( (2 ** i) * blksz)) "$testdir/file1"
+truncate -s $(( (2 ** (fnr + 1)) * blksz)) "$testdir/file1"
for i in $(seq 0 $fnr); do
echo " ++ Reflink size $i, $((2 ** i)) blocks" >> "$seqres.full"
n=$(( (2 ** i) * blksz))
diff --git a/tests/generic/297 b/tests/generic/297
index 6bdc3e1c..1fc48671 100755
--- a/tests/generic/297
+++ b/tests/generic/297
@@ -42,7 +42,7 @@ _pwrite_byte 0x61 0 $blksz $testdir/file1 >> $seqres.full
fnr=26 # 2^26 reflink extents should be enough to find a slow op?
timeout=8 # guarantee a good long run...
echo "Find a reflink size that takes a long time"
-truncate -s $(( (2 ** i) * blksz)) $testdir/file1
+truncate -s $(( (2 ** (fnr + 1)) * blksz)) $testdir/file1
for i in $(seq 0 $fnr); do
echo " ++ Reflink size $i, $((2 ** i)) blocks" >> $seqres.full
n=$(( (2 ** i) * blksz))
diff --git a/tests/generic/298 b/tests/generic/298
index 95d4c02b..2e917a87 100755
--- a/tests/generic/298
+++ b/tests/generic/298
@@ -42,7 +42,7 @@ _pwrite_byte 0x61 0 $blksz $testdir/file1 >> $seqres.full
fnr=26 # 2^26 reflink extents should be enough to find a slow op?
timeout=8 # guarantee a good long run...
echo "Find a reflink size that takes a long time"
-truncate -s $(( (2 ** i) * blksz)) $testdir/file1
+truncate -s $(( (2 ** (fnr + 1)) * blksz)) $testdir/file1
for i in $(seq 0 $fnr); do
echo " ++ Reflink size $i, $((2 ** i)) blocks" >> $seqres.full
n=$(( (2 ** i) * blksz))
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] generic/{175,297,298}: fix use of uninitialized var
2023-08-12 15:15 [PATCH] generic/{175,297,298}: fix use of uninitialized var Amir Goldstein
@ 2023-08-13 14:52 ` Zorro Lang
2023-08-13 15:35 ` Darrick J. Wong
1 sibling, 0 replies; 5+ messages in thread
From: Zorro Lang @ 2023-08-13 14:52 UTC (permalink / raw)
To: Amir Goldstein
Cc: Darrick J . Wong, Chandan Babu R, Luis Chamberlain, fstests
On Sat, Aug 12, 2023 at 06:15:00PM +0300, Amir Goldstein wrote:
> Not sure how those tests pass in regression tests.
> Probably truncate silently fails and is not critical to the test.
>
> in kdevops I get errors like:
> /data/fstests-install/xfstests/tests/generic/298: line 45: /dev/loop12):
> syntax error: operand expected (error token is "/dev/loop12)")
>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
Hmm, weird, I never hit this failure, all test passed [1]. But I think the
reason is [2], $(()) looks like treat empty variable as 0.
Anyway, this patch makes sense, so
Reviewed-by: Zorro Lang <zlang@redhat.com>
[1]
# ./check -s simpledev generic/175 generic/297 generic/298
SECTION -- simpledev
FSTYP -- xfs (non-debug)
PLATFORM -- Linux/x86_64 hp-dl380pg8-01 6.5.0-0.rc3.23.fc39.x86_64 #1 SMP PREEMPT_DYNAMIC Mon Jul 24 13:32:24 UTC 2023
MKFS_OPTIONS -- -f /dev/sda3
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/sda3 /mnt/scratch
generic/175 91s ... 40s
generic/297 44s
generic/298 125s ... 44s
Ran: generic/175 generic/297 generic/298
Passed all 3 tests
SECTION -- simpledev
=========================
Ran: generic/175 generic/297 generic/298
Passed all 3 tests
[2]
# blksz=4096
# echo $blksz
4096
# echo $i
# echo $(( (2 ** i) * blksz))
4096
# echo $i
# echo $((i))
0
> tests/generic/175 | 2 +-
> tests/generic/297 | 2 +-
> tests/generic/298 | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tests/generic/175 b/tests/generic/175
> index 07af2199..14825a39 100755
> --- a/tests/generic/175
> +++ b/tests/generic/175
> @@ -33,7 +33,7 @@ _pwrite_byte 0x61 0 $blksz "$testdir/file1" >> "$seqres.full"
>
> fnr=19
> echo "Create extents"
> -truncate -s $(( (2 ** i) * blksz)) "$testdir/file1"
> +truncate -s $(( (2 ** (fnr + 1)) * blksz)) "$testdir/file1"
> for i in $(seq 0 $fnr); do
> echo " ++ Reflink size $i, $((2 ** i)) blocks" >> "$seqres.full"
> n=$(( (2 ** i) * blksz))
> diff --git a/tests/generic/297 b/tests/generic/297
> index 6bdc3e1c..1fc48671 100755
> --- a/tests/generic/297
> +++ b/tests/generic/297
> @@ -42,7 +42,7 @@ _pwrite_byte 0x61 0 $blksz $testdir/file1 >> $seqres.full
> fnr=26 # 2^26 reflink extents should be enough to find a slow op?
> timeout=8 # guarantee a good long run...
> echo "Find a reflink size that takes a long time"
> -truncate -s $(( (2 ** i) * blksz)) $testdir/file1
> +truncate -s $(( (2 ** (fnr + 1)) * blksz)) $testdir/file1
> for i in $(seq 0 $fnr); do
> echo " ++ Reflink size $i, $((2 ** i)) blocks" >> $seqres.full
> n=$(( (2 ** i) * blksz))
> diff --git a/tests/generic/298 b/tests/generic/298
> index 95d4c02b..2e917a87 100755
> --- a/tests/generic/298
> +++ b/tests/generic/298
> @@ -42,7 +42,7 @@ _pwrite_byte 0x61 0 $blksz $testdir/file1 >> $seqres.full
> fnr=26 # 2^26 reflink extents should be enough to find a slow op?
> timeout=8 # guarantee a good long run...
> echo "Find a reflink size that takes a long time"
> -truncate -s $(( (2 ** i) * blksz)) $testdir/file1
> +truncate -s $(( (2 ** (fnr + 1)) * blksz)) $testdir/file1
> for i in $(seq 0 $fnr); do
> echo " ++ Reflink size $i, $((2 ** i)) blocks" >> $seqres.full
> n=$(( (2 ** i) * blksz))
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] generic/{175,297,298}: fix use of uninitialized var
2023-08-12 15:15 [PATCH] generic/{175,297,298}: fix use of uninitialized var Amir Goldstein
2023-08-13 14:52 ` Zorro Lang
@ 2023-08-13 15:35 ` Darrick J. Wong
2023-08-13 18:53 ` Amir Goldstein
1 sibling, 1 reply; 5+ messages in thread
From: Darrick J. Wong @ 2023-08-13 15:35 UTC (permalink / raw)
To: Amir Goldstein; +Cc: Zorro Lang, Chandan Babu R, Luis Chamberlain, fstests
On Sat, Aug 12, 2023 at 06:15:00PM +0300, Amir Goldstein wrote:
> Not sure how those tests pass in regression tests.
> Probably truncate silently fails and is not critical to the test.
>
> in kdevops I get errors like:
> /data/fstests-install/xfstests/tests/generic/298: line 45: /dev/loop12):
> syntax error: operand expected (error token is "/dev/loop12)")
>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
> tests/generic/175 | 2 +-
> tests/generic/297 | 2 +-
> tests/generic/298 | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tests/generic/175 b/tests/generic/175
> index 07af2199..14825a39 100755
> --- a/tests/generic/175
> +++ b/tests/generic/175
> @@ -33,7 +33,7 @@ _pwrite_byte 0x61 0 $blksz "$testdir/file1" >> "$seqres.full"
>
> fnr=19
> echo "Create extents"
> -truncate -s $(( (2 ** i) * blksz)) "$testdir/file1"
> +truncate -s $(( (2 ** (fnr + 1)) * blksz)) "$testdir/file1"
> for i in $(seq 0 $fnr); do
Hmm. Frankly I don't remember why I put those truncate calls in there.
Does the test still work if you remove them entirely? AFAICT the test
writes a single block's worth of data to the file, then truncates the
file size to one block. Which I think is pointless.
> echo " ++ Reflink size $i, $((2 ** i)) blocks" >> "$seqres.full"
> n=$(( (2 ** i) * blksz))
> diff --git a/tests/generic/297 b/tests/generic/297
> index 6bdc3e1c..1fc48671 100755
> --- a/tests/generic/297
> +++ b/tests/generic/297
> @@ -42,7 +42,7 @@ _pwrite_byte 0x61 0 $blksz $testdir/file1 >> $seqres.full
> fnr=26 # 2^26 reflink extents should be enough to find a slow op?
> timeout=8 # guarantee a good long run...
> echo "Find a reflink size that takes a long time"
> -truncate -s $(( (2 ** i) * blksz)) $testdir/file1
> +truncate -s $(( (2 ** (fnr + 1)) * blksz)) $testdir/file1
> for i in $(seq 0 $fnr); do
> echo " ++ Reflink size $i, $((2 ** i)) blocks" >> $seqres.full
> n=$(( (2 ** i) * blksz))
The loop control logic could be converted to:
echo "Find a reflink size that takes a long time"
deadline="$(( $(date +%s) + timeout ))"
for ((i = 0, now = $(date +%s); i < fnr && now < deadline; i++, now = $(date +%s))); do
echo " ++ Reflink size $i, $((2 ** i)) blocks" >> $seqres.full
n=$(( (2 ** i) * blksz))
$XFS_IO_PROG -f -c "reflink $testdir/file1 0 $n $n" $testdir/file1 >> $seqres.full 2>&1
done
(also in 298)
--D
> diff --git a/tests/generic/298 b/tests/generic/298
> index 95d4c02b..2e917a87 100755
> --- a/tests/generic/298
> +++ b/tests/generic/298
> @@ -42,7 +42,7 @@ _pwrite_byte 0x61 0 $blksz $testdir/file1 >> $seqres.full
> fnr=26 # 2^26 reflink extents should be enough to find a slow op?
> timeout=8 # guarantee a good long run...
> echo "Find a reflink size that takes a long time"
> -truncate -s $(( (2 ** i) * blksz)) $testdir/file1
> +truncate -s $(( (2 ** (fnr + 1)) * blksz)) $testdir/file1
> for i in $(seq 0 $fnr); do
> echo " ++ Reflink size $i, $((2 ** i)) blocks" >> $seqres.full
> n=$(( (2 ** i) * blksz))
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] generic/{175,297,298}: fix use of uninitialized var
2023-08-13 15:35 ` Darrick J. Wong
@ 2023-08-13 18:53 ` Amir Goldstein
2023-08-15 2:16 ` Darrick J. Wong
0 siblings, 1 reply; 5+ messages in thread
From: Amir Goldstein @ 2023-08-13 18:53 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Zorro Lang, Chandan Babu R, Luis Chamberlain, fstests
On Sun, Aug 13, 2023 at 6:35 PM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Sat, Aug 12, 2023 at 06:15:00PM +0300, Amir Goldstein wrote:
> > Not sure how those tests pass in regression tests.
> > Probably truncate silently fails and is not critical to the test.
> >
> > in kdevops I get errors like:
> > /data/fstests-install/xfstests/tests/generic/298: line 45: /dev/loop12):
> > syntax error: operand expected (error token is "/dev/loop12)")
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> > tests/generic/175 | 2 +-
> > tests/generic/297 | 2 +-
> > tests/generic/298 | 2 +-
> > 3 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/tests/generic/175 b/tests/generic/175
> > index 07af2199..14825a39 100755
> > --- a/tests/generic/175
> > +++ b/tests/generic/175
> > @@ -33,7 +33,7 @@ _pwrite_byte 0x61 0 $blksz "$testdir/file1" >> "$seqres.full"
> >
> > fnr=19
> > echo "Create extents"
> > -truncate -s $(( (2 ** i) * blksz)) "$testdir/file1"
> > +truncate -s $(( (2 ** (fnr + 1)) * blksz)) "$testdir/file1"
> > for i in $(seq 0 $fnr); do
>
> Hmm. Frankly I don't remember why I put those truncate calls in there.
You specifically added truncate in commit
ddf6ff2f reflink: ensure that we can handle reflinking a lot of extents
and then it was copied to the two new tests.
maybe this will give you a hint?
> Does the test still work if you remove them entirely? AFAICT the test
> writes a single block's worth of data to the file, then truncates the
> file size to one block. Which I think is pointless.
>
> > echo " ++ Reflink size $i, $((2 ** i)) blocks" >> "$seqres.full"
> > n=$(( (2 ** i) * blksz))
> > diff --git a/tests/generic/297 b/tests/generic/297
> > index 6bdc3e1c..1fc48671 100755
> > --- a/tests/generic/297
> > +++ b/tests/generic/297
> > @@ -42,7 +42,7 @@ _pwrite_byte 0x61 0 $blksz $testdir/file1 >> $seqres.full
> > fnr=26 # 2^26 reflink extents should be enough to find a slow op?
> > timeout=8 # guarantee a good long run...
> > echo "Find a reflink size that takes a long time"
> > -truncate -s $(( (2 ** i) * blksz)) $testdir/file1
> > +truncate -s $(( (2 ** (fnr + 1)) * blksz)) $testdir/file1
> > for i in $(seq 0 $fnr); do
> > echo " ++ Reflink size $i, $((2 ** i)) blocks" >> $seqres.full
> > n=$(( (2 ** i) * blksz))
>
> The loop control logic could be converted to:
>
> echo "Find a reflink size that takes a long time"
> deadline="$(( $(date +%s) + timeout ))"
> for ((i = 0, now = $(date +%s); i < fnr && now < deadline; i++, now = $(date +%s))); do
> echo " ++ Reflink size $i, $((2 ** i)) blocks" >> $seqres.full
> n=$(( (2 ** i) * blksz))
> $XFS_IO_PROG -f -c "reflink $testdir/file1 0 $n $n" $testdir/file1 >> $seqres.full 2>&1
> done
>
> (also in 298)
>
Whatever works is fine by me.
Feel free to keep my commit message or drop it.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] generic/{175,297,298}: fix use of uninitialized var
2023-08-13 18:53 ` Amir Goldstein
@ 2023-08-15 2:16 ` Darrick J. Wong
0 siblings, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2023-08-15 2:16 UTC (permalink / raw)
To: Amir Goldstein; +Cc: Zorro Lang, Chandan Babu R, Luis Chamberlain, fstests
On Sun, Aug 13, 2023 at 09:53:30PM +0300, Amir Goldstein wrote:
> On Sun, Aug 13, 2023 at 6:35 PM Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > On Sat, Aug 12, 2023 at 06:15:00PM +0300, Amir Goldstein wrote:
> > > Not sure how those tests pass in regression tests.
> > > Probably truncate silently fails and is not critical to the test.
> > >
> > > in kdevops I get errors like:
> > > /data/fstests-install/xfstests/tests/generic/298: line 45: /dev/loop12):
> > > syntax error: operand expected (error token is "/dev/loop12)")
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > ---
> > > tests/generic/175 | 2 +-
> > > tests/generic/297 | 2 +-
> > > tests/generic/298 | 2 +-
> > > 3 files changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tests/generic/175 b/tests/generic/175
> > > index 07af2199..14825a39 100755
> > > --- a/tests/generic/175
> > > +++ b/tests/generic/175
> > > @@ -33,7 +33,7 @@ _pwrite_byte 0x61 0 $blksz "$testdir/file1" >> "$seqres.full"
> > >
> > > fnr=19
> > > echo "Create extents"
> > > -truncate -s $(( (2 ** i) * blksz)) "$testdir/file1"
> > > +truncate -s $(( (2 ** (fnr + 1)) * blksz)) "$testdir/file1"
> > > for i in $(seq 0 $fnr); do
> >
> > Hmm. Frankly I don't remember why I put those truncate calls in there.
>
> You specifically added truncate in commit
> ddf6ff2f reflink: ensure that we can handle reflinking a lot of extents
> and then it was copied to the two new tests.
> maybe this will give you a hint?
Alas, no. Total etch-a-sketch over here. :(
AFAICT the tests work just fine without the truncate calls, so let's
remove them.
--D
> > Does the test still work if you remove them entirely? AFAICT the test
> > writes a single block's worth of data to the file, then truncates the
> > file size to one block. Which I think is pointless.
> >
> > > echo " ++ Reflink size $i, $((2 ** i)) blocks" >> "$seqres.full"
> > > n=$(( (2 ** i) * blksz))
> > > diff --git a/tests/generic/297 b/tests/generic/297
> > > index 6bdc3e1c..1fc48671 100755
> > > --- a/tests/generic/297
> > > +++ b/tests/generic/297
> > > @@ -42,7 +42,7 @@ _pwrite_byte 0x61 0 $blksz $testdir/file1 >> $seqres.full
> > > fnr=26 # 2^26 reflink extents should be enough to find a slow op?
> > > timeout=8 # guarantee a good long run...
> > > echo "Find a reflink size that takes a long time"
> > > -truncate -s $(( (2 ** i) * blksz)) $testdir/file1
> > > +truncate -s $(( (2 ** (fnr + 1)) * blksz)) $testdir/file1
> > > for i in $(seq 0 $fnr); do
> > > echo " ++ Reflink size $i, $((2 ** i)) blocks" >> $seqres.full
> > > n=$(( (2 ** i) * blksz))
> >
> > The loop control logic could be converted to:
> >
> > echo "Find a reflink size that takes a long time"
> > deadline="$(( $(date +%s) + timeout ))"
> > for ((i = 0, now = $(date +%s); i < fnr && now < deadline; i++, now = $(date +%s))); do
> > echo " ++ Reflink size $i, $((2 ** i)) blocks" >> $seqres.full
> > n=$(( (2 ** i) * blksz))
> > $XFS_IO_PROG -f -c "reflink $testdir/file1 0 $n $n" $testdir/file1 >> $seqres.full 2>&1
> > done
> >
> > (also in 298)
> >
>
> Whatever works is fine by me.
> Feel free to keep my commit message or drop it.
>
> Thanks,
> Amir.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-08-15 2:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-12 15:15 [PATCH] generic/{175,297,298}: fix use of uninitialized var Amir Goldstein
2023-08-13 14:52 ` Zorro Lang
2023-08-13 15:35 ` Darrick J. Wong
2023-08-13 18:53 ` Amir Goldstein
2023-08-15 2:16 ` Darrick J. Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox