review: xfstests/016 change

review: xfstests/016 change

[Timothy Shimmin]

Hi there,

xfstests/016 has been failing for a while in many cases.
It was relying on the log traffic for create/rm to be unaltered
in XFS over the life of XFS.
Its initial purpose was to test out the writing of the log around
wrapping and checking that as we wrapped we didn't corrupt any data
after the end of the log.

The patch is to modify xfstest, 016, to take a sampling of the log traffic
to guestimate how many ops are needed to get to the end of the log,
instead of hard-coding in how many ops are expected to get there.

--Tim


===========================================================================
Index: xfstests/016
===========================================================================

--- a/xfstests/016	2008-05-05 11:26:50.000000000 +1000
+++ b/xfstests/016	2008-05-05 13:07:01.040000000 +1000
@@ -50,7 +50,7 @@ _init()
      echo "*** reset partition"
      $here/src/devzero -b 2048 -n 50 -v 198 $SCRATCH_DEV
      echo "*** mkfs"
-    force_opts="-dsize=50m -lsize=2097152"
+    force_opts="-dsize=50m -lsize=$log_size"
      echo mkfs_xfs $force_opts $SCRATCH_DEV >>$seq.full
      _scratch_mkfs_xfs $force_opts >$tmp.mkfs0 2>&1
      [ $? -ne 0 ] && \
@@ -156,6 +156,10 @@ _supported_os Linux

  rm -f $seq.full

+# mkfs sizes
+log_size=2097152
+log_size_bb=`expr $log_size / 512`
+
  _require_scratch
  _init

@@ -163,27 +167,47 @@ block=`_after_log $SCRATCH_DEV`
  echo "fsblock after log = $block"               >>$seq.full
  _check_corrupt $SCRATCH_DEV $block

-size=`_log_size`
-echo "log size = $size BB"                      >>$seq.full
+actual_log_size=`_log_size`
+echo "log size = $actual_log_size BB"                      >>$seq.full
  head=`_log_head`
  echo "log position = $head"                     >>$seq.full
  lsunit=`_log_sunit`
  echo "log sunit = $lsunit"			>>$seq.full

-[ $size -eq 4096 ] || \
+# sanity checks
+[ $actual_log_size -eq $log_size_bb ] || \
      _fail "!!! unexpected log size $size"
  [ $head -eq 2 -o $head -eq $((lsunit/512)) ] || \
      _fail "!!! unexpected initial log position $head vs. $((lsunit/512))"

-echo "    lots of traffic"                      >>$seq.full
-_log_traffic 850
+# find how how many blocks per op for 100 ops
+# ignore the fact that it will also include an unmount record etc...
+# this should be small overall
+echo "    lots of traffic for sampling" >>$seq.full
+sample_size_ops=100
+_log_traffic $sample_size_ops
+head1=`_log_head`
+num_blocks=`expr $head1 - $head`
+blocks_per_op=`echo "scale=3; $num_blocks / $sample_size_ops" | bc`
+echo "blocks_per_op = $blocks_per_op" >>$seq.full
+num_expected_ops=`echo "$log_size_bb / $blocks_per_op" | bc`
+echo "num_expected_ops = $num_expected_ops" >>$seq.full
+num_expected_to_go=`echo "$num_expected_ops - $sample_size_ops" | bc`
+echo "num_expected_to_go = $num_expected_to_go" >>$seq.full
+
+echo "    lots more traffic" >>$seq.full
+_log_traffic $num_expected_to_go
  head=`_log_head`
  echo "log position = $head"                     >>$seq.full

-[ $head -gt 3850 -a $head -lt 4050 ] || \
-    _fail "!!! unexpected log position $head"
+# e.g. 3891
+near_end_min=`echo "0.95 * $log_size_bb" | bc | sed 's/\..*//'`
+echo "near_end_min = $near_end_min" >>$seq.full
+
+[ $head -gt $near_end_min -a $head -lt $log_size_bb ] || \
+    _fail "!!! unexpected near end log position $head"

-for c in 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20
+for c in `seq 0 20`
  do
      echo "   little traffic"            >>$seq.full
      _log_traffic 2
@@ -193,7 +217,7 @@ do
  done

  [ $head -lt 1000 ] || \
-    _fail "!!! unexpected log position $head"
+    _fail "!!! unexpected wrapped log position $head"

  # success, all done
  status=0

Re: review: xfstests/016 change

[Christoph Hellwig]

On Mon, May 05, 2008 at 01:29:27PM +1000, Timothy Shimmin wrote:
> Hi there,
>
> xfstests/016 has been failing for a while in many cases.
> It was relying on the log traffic for create/rm to be unaltered
> in XFS over the life of XFS.
> Its initial purpose was to test out the writing of the log around
> wrapping and checking that as we wrapped we didn't corrupt any data
> after the end of the log.
>
> The patch is to modify xfstest, 016, to take a sampling of the log traffic
> to guestimate how many ops are needed to get to the end of the log,
> instead of hard-coding in how many ops are expected to get there.

I get four rejects out of four hunk when trying to apply this patch,
looks like whitespace-damage.

Re: review: xfstests/016 change

[Timothy Shimmin]

[-- Attachment #1: Type: text/plain, Size: 882 bytes --]

Christoph Hellwig wrote:
> On Mon, May 05, 2008 at 01:29:27PM +1000, Timothy Shimmin wrote:
>> Hi there,
>>
>> xfstests/016 has been failing for a while in many cases.
>> It was relying on the log traffic for create/rm to be unaltered
>> in XFS over the life of XFS.
>> Its initial purpose was to test out the writing of the log around
>> wrapping and checking that as we wrapped we didn't corrupt any data
>> after the end of the log.
>>
>> The patch is to modify xfstest, 016, to take a sampling of the log traffic
>> to guestimate how many ops are needed to get to the end of the log,
>> instead of hard-coding in how many ops are expected to get there.
> 
> I get four rejects out of four hunk when trying to apply this patch,
> looks like whitespace-damage.

Groan. Sorry.
I'll attach 016.patch for the moment.

Need to see where things went wrong with the whitespace.

--Tim


[-- Attachment #2: 016.patch --]
[-- Type: text/x-patch, Size: 3139 bytes --]


===========================================================================
Index: xfstests/016
===========================================================================

--- a/xfstests/016	2008-05-05 11:26:50.000000000 +1000
+++ b/xfstests/016	2008-05-05 13:07:01.040000000 +1000
@@ -50,7 +50,7 @@ _init()
     echo "*** reset partition"
     $here/src/devzero -b 2048 -n 50 -v 198 $SCRATCH_DEV
     echo "*** mkfs"
-    force_opts="-dsize=50m -lsize=2097152"
+    force_opts="-dsize=50m -lsize=$log_size"
     echo mkfs_xfs $force_opts $SCRATCH_DEV >>$seq.full
     _scratch_mkfs_xfs $force_opts >$tmp.mkfs0 2>&1
     [ $? -ne 0 ] && \
@@ -156,6 +156,10 @@ _supported_os Linux
 
 rm -f $seq.full
 
+# mkfs sizes
+log_size=2097152
+log_size_bb=`expr $log_size / 512`
+
 _require_scratch
 _init
 
@@ -163,27 +167,47 @@ block=`_after_log $SCRATCH_DEV`
 echo "fsblock after log = $block"               >>$seq.full
 _check_corrupt $SCRATCH_DEV $block
 
-size=`_log_size`
-echo "log size = $size BB"                      >>$seq.full
+actual_log_size=`_log_size`
+echo "log size = $actual_log_size BB"                      >>$seq.full
 head=`_log_head`
 echo "log position = $head"                     >>$seq.full
 lsunit=`_log_sunit`
 echo "log sunit = $lsunit"			>>$seq.full
 
-[ $size -eq 4096 ] || \
+# sanity checks
+[ $actual_log_size -eq $log_size_bb ] || \
     _fail "!!! unexpected log size $size"
 [ $head -eq 2 -o $head -eq $((lsunit/512)) ] || \
     _fail "!!! unexpected initial log position $head vs. $((lsunit/512))"
 
-echo "    lots of traffic"                      >>$seq.full
-_log_traffic 850
+# find how how many blocks per op for 100 ops
+# ignore the fact that it will also include an unmount record etc...
+# this should be small overall
+echo "    lots of traffic for sampling" >>$seq.full
+sample_size_ops=100
+_log_traffic $sample_size_ops
+head1=`_log_head`
+num_blocks=`expr $head1 - $head`
+blocks_per_op=`echo "scale=3; $num_blocks / $sample_size_ops" | bc`
+echo "blocks_per_op = $blocks_per_op" >>$seq.full
+num_expected_ops=`echo "$log_size_bb / $blocks_per_op" | bc`
+echo "num_expected_ops = $num_expected_ops" >>$seq.full
+num_expected_to_go=`echo "$num_expected_ops - $sample_size_ops" | bc`
+echo "num_expected_to_go = $num_expected_to_go" >>$seq.full
+
+echo "    lots more traffic" >>$seq.full
+_log_traffic $num_expected_to_go
 head=`_log_head`
 echo "log position = $head"                     >>$seq.full
 
-[ $head -gt 3850 -a $head -lt 4050 ] || \
-    _fail "!!! unexpected log position $head"
+# e.g. 3891
+near_end_min=`echo "0.95 * $log_size_bb" | bc | sed 's/\..*//'`
+echo "near_end_min = $near_end_min" >>$seq.full
+
+[ $head -gt $near_end_min -a $head -lt $log_size_bb ] || \
+    _fail "!!! unexpected near end log position $head"
 
-for c in 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20
+for c in `seq 0 20`
 do
     echo "   little traffic"            >>$seq.full
     _log_traffic 2
@@ -193,7 +217,7 @@ do
 done
 
 [ $head -lt 1000 ] || \
-    _fail "!!! unexpected log position $head"
+    _fail "!!! unexpected wrapped log position $head"
 
 # success, all done
 status=0

Re: review: xfstests/016 change

[Christoph Hellwig]

On Mon, May 05, 2008 at 04:19:54PM +1000, Timothy Shimmin wrote:
> Groan. Sorry.
> I'll attach 016.patch for the moment.
>
> Need to see where things went wrong with the whitespace.

Still fails for me with that patch applied:

016 43s ...
 - output mismatch (see 016.out.bad)
13a14,17
> *** generate log traffic
>    *** mount
>    *** fiddle
>    *** unmount
Failures: 016
Failed 1 of 1 tests

Re: review: xfstests/016 change

[Timothy Shimmin]

Christoph Hellwig wrote:
> On Mon, May 05, 2008 at 04:19:54PM +1000, Timothy Shimmin wrote:
>> Groan. Sorry.
>> I'll attach 016.patch for the moment.
>>
>> Need to see where things went wrong with the whitespace.
> 
> Still fails for me with that patch applied:
> 
> 016 43s ...
>  - output mismatch (see 016.out.bad)
> 13a14,17
>> *** generate log traffic
>>    *** mount
>>    *** fiddle
>>    *** unmount
> Failures: 016
> Failed 1 of 1 tests

Forgot adding 016.out change to patch.
The failure diff above means it worked ;-)

--Tim