From 75e8fab57c866626d25e75fc464a78fbaa58d632 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 10 Mar 2021 10:40:02 -0800 Subject: [PATCH 1/3] Add t_counter_diff_changed Tests can use t_counter_diff to put a message in their golden output when a specific change in counters is expected. This adds t_counter_diff_changed to output a message that indicates change or not, for tests that want to see counters change but the amount of change doesn't need to be precisely known. Signed-off-by: Zach Brown --- tests/funcs/fs.sh | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/tests/funcs/fs.sh b/tests/funcs/fs.sh index c0050ba3..4ce068c0 100644 --- a/tests/funcs/fs.sh +++ b/tests/funcs/fs.sh @@ -229,16 +229,44 @@ t_counter() { cat "$(t_sysfs_path $nr)/counters/$which" } +# +# output the difference between the current value of a counter and the +# caller's provided previous value. +# +t_counter_diff_value() { + local which="$1" + local old="$2" + local nr="$3" + local new="$(t_counter $which $nr)" + + echo "$((new - old))" +} + # # output the value of the given counter for the given mount, defaulting -# to mount 0 if a mount isn't specified. +# to mount 0 if a mount isn't specified. For tests which expect a +# specific difference in counters. # t_counter_diff() { local which="$1" local old="$2" local nr="$3" - local new - new="$(t_counter $which $nr)" - echo "counter $which diff $((new - old))" + echo "counter $which diff $(t_counter_diff_value $which $old $nr)" +} + +# +# output a message indicating whether or not the counter value changed. +# For tests that expect a difference, or not, but the amount of +# difference isn't significant. +# +t_counter_diff_changed() { + local which="$1" + local old="$2" + local nr="$3" + local diff="$(t_counter_diff_value $which $old $nr)" + + test "$diff" -eq 0 && \ + echo "counter $which didn't change" || + echo "counter $which changed" } From 12fa289399b72f1a66dbb9d25f0be247b63ab2e3 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 10 Mar 2021 12:14:00 -0800 Subject: [PATCH 2/3] Add t_trigger_arm_silent t_trigger_arm always output the value of the trigger after arming on the premise that tests required the trigger being armed. In the process of showing the trigger it calls a bunch of t_ helpers that build the path to the trigger file using statfs_more to get the rid of mounts. If the trigger being armed is in the server's mount and the specific trigger test is fired by the server's statfs_more request processing then the trigger can be fired before read its value. Tests can inconsistently fail as the golden output shows the trigger being armed or not depending on if it was in the server's mount or not. t_trigger_arm_silent doesn't output the value of the armed trigger. It can be used for low level triggers that don't rely on reading the trigger's value to discover that their effect has happened. Signed-off-by: Zach Brown --- tests/funcs/fs.sh | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/funcs/fs.sh b/tests/funcs/fs.sh index 4ce068c0..b4b17cf3 100644 --- a/tests/funcs/fs.sh +++ b/tests/funcs/fs.sh @@ -209,12 +209,19 @@ t_trigger_show() { echo "trigger $which $string: $(t_trigger_get $which $nr)" } -t_trigger_arm() { +t_trigger_arm_silent() { local which="$1" local nr="$2" local path=$(t_trigger_path "$nr") echo 1 > "$path/$which" +} + +t_trigger_arm() { + local which="$1" + local nr="$2" + + t_trigger_arm_silent $which $nr t_trigger_show $which armed $nr } From 5661a1fb028b15be65f3e34f26301f89eaf6311d Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 10 Mar 2021 10:23:48 -0800 Subject: [PATCH 3/3] Fix block-stale-reads test The block-stale-reads test was built from the ashes of a test that used counters and triggers to work with the btree when it was only used on the server. The initial quick translation to try and trigger block cache retries while the forest called the btree got so much wrong. It was still trying to use some 'cl' variable that didn't refer to the client any more, the trigger helpers now call statfs to find paths and can end up triggering themselves. and many more counters stale reads can happen throughout the system while we're working -- not just one from our trigger. This fixes it up to consistently use fs numbers instead of the silly stale cl variable and be less sensitive to triggers firing and counter differences. Signed-off-by: Zach Brown --- tests/golden/block-stale-reads | 57 ++++++++++++++++++++--------- tests/tests/block-stale-reads.sh | 63 +++++++++++++++++++++----------- 2 files changed, 82 insertions(+), 38 deletions(-) diff --git a/tests/golden/block-stale-reads b/tests/golden/block-stale-reads index 51f11b28..60099ace 100644 --- a/tests/golden/block-stale-reads +++ b/tests/golden/block-stale-reads @@ -1,29 +1,52 @@ -== create file for xattr ping pong -# file: /mnt/test/test/block-stale-reads/file -user.xat="initial" - -== retry btree forest reads between mounts -trigger block_remove_stale armed: 0 +== create shared test file +== set and get xattrs between mount pairs while retrying # file: /mnt/test/test/block-stale-reads/file user.xat="1" -trigger block_remove_stale after: 0 -counter block_cache_remove_stale diff 1 -trigger block_remove_stale armed: 0 +counter block_cache_remove_stale changed +counter block_cache_remove_stale changed # file: /mnt/test/test/block-stale-reads/file user.xat="2" -trigger block_remove_stale after: 0 -counter block_cache_remove_stale diff 2 -trigger block_remove_stale armed: 0 +counter block_cache_remove_stale changed +counter block_cache_remove_stale changed # file: /mnt/test/test/block-stale-reads/file user.xat="3" -trigger block_remove_stale after: 0 -counter block_cache_remove_stale diff 3 -trigger block_remove_stale armed: 0 +counter block_cache_remove_stale changed +counter block_cache_remove_stale changed # file: /mnt/test/test/block-stale-reads/file user.xat="4" -trigger block_remove_stale after: 0 -counter block_cache_remove_stale diff 4 +counter block_cache_remove_stale changed +counter block_cache_remove_stale changed +# file: /mnt/test/test/block-stale-reads/file +user.xat="5" + +counter block_cache_remove_stale changed +counter block_cache_remove_stale changed +# file: /mnt/test/test/block-stale-reads/file +user.xat="6" + +counter block_cache_remove_stale changed +counter block_cache_remove_stale changed +# file: /mnt/test/test/block-stale-reads/file +user.xat="7" + +counter block_cache_remove_stale changed +counter block_cache_remove_stale changed +# file: /mnt/test/test/block-stale-reads/file +user.xat="8" + +counter block_cache_remove_stale changed +counter block_cache_remove_stale changed +# file: /mnt/test/test/block-stale-reads/file +user.xat="9" + +counter block_cache_remove_stale changed +counter block_cache_remove_stale changed +# file: /mnt/test/test/block-stale-reads/file +user.xat="10" + +counter block_cache_remove_stale changed +counter block_cache_remove_stale changed diff --git a/tests/tests/block-stale-reads.sh b/tests/tests/block-stale-reads.sh index a41d821d..f20a4f75 100644 --- a/tests/tests/block-stale-reads.sh +++ b/tests/tests/block-stale-reads.sh @@ -1,5 +1,5 @@ # -# exercise stale block reading. +# Exercise stale block reading. # # It would be very difficult to manipulate the allocators, cache, and # persistent blocks to create stable block reading scenarios. Instead @@ -7,34 +7,55 @@ # t_require_commands touch setfattr getfattr -t_require_mounts 2 + +inc_wrap_fs_nr() +{ + local nr="$(($1 + 1))" + + if [ "$nr" == "$T_NR_MOUNTS" ]; then + nr=0 + fi + + echo $nr +} GETFATTR="getfattr --absolute-names" SETFATTR="setfattr" -# -# force re-reading forest btree blocks as each mount reads the items -# written by the other. -# -set_file="$T_D0/file" -get_file="$T_D1/file" -echo "== create file for xattr ping pong" -touch "$set_file" -$SETFATTR -n user.xat -v initial "$set_file" -$GETFATTR -n user.xat "$get_file" 2>&1 | t_filter_fs +echo "== create shared test file" +touch "$T_D0/file" +$SETFATTR -n user.xat -v 0 "$T_D0/file" -echo "== retry btree forest reads between mounts" -for i in $(seq 1 4); do - tmp="$set_file" - set_file="$get_file" - get_file="$tmp" +# +# Trigger retries in the block cache as we bounce xattr values around +# between sequential pairs of mounts. This is a little silly because if +# either of the mounts are the server then they'll almost certaily have +# their trigger fired prematurely by message handling btree calls while +# working with the t_ helpers long before we work with the xattrs. But +# the block cache stale retry path is still being exercised. +# +echo "== set and get xattrs between mount pairs while retrying" +set_nr=0 +get_nr=$(inc_wrap_fs_nr $set_nr) + +for i in $(seq 1 10); do + eval set_file="\$T_D${set_nr}/file" + eval get_file="\$T_D${get_nr}/file" + + old_set=$(t_counter block_cache_remove_stale $set_nr) + old_get=$(t_counter block_cache_remove_stale $get_nr) + + t_trigger_arm_silent block_remove_stale $set_nr + t_trigger_arm_silent block_remove_stale $get_nr $SETFATTR -n user.xat -v $i "$set_file" - t_trigger_arm block_remove_stale $cl - old=$(t_counter btree_stale_read $cl) $GETFATTR -n user.xat "$get_file" 2>&1 | t_filter_fs - t_trigger_show block_remove_stale "after" $cl - t_counter_diff block_cache_remove_stale $old $cl + + t_counter_diff_changed block_cache_remove_stale $old_set $set_nr + t_counter_diff_changed block_cache_remove_stale $old_get $get_nr + + set_nr="$get_nr" + get_nr=$(inc_wrap_fs_nr $set_nr) done t_pass