* [PATCH] generic: add a test that stresses metadata eviction @ 2026-05-18 6:24 Christoph Hellwig 2026-05-18 12:27 ` Carlos Maiolino 0 siblings, 1 reply; 8+ messages in thread From: Christoph Hellwig @ 2026-05-18 6:24 UTC (permalink / raw) To: zlang; +Cc: cmaiolino, fstests, linux-xfs The fsverity test generic/759 could be used to reproduce a regression in the xfs buffer hashing. Carlos Maiolino figured out that even removing the fsverity-specific bits would reproduce the issue. This test is thus a copy of generic/759 with the fsverity bits striped out, and run 5 times as that seems to be the threshold at which the issue reproduces reliably. Signed-off-by: Christoph Hellwig <hch@lst.de> --- Not sure how to best credit Eric for the original patch and Carlos for the extraction, any ideas? tests/generic/4201 | 108 +++++++++++++++++++++++++++++++++++++++++ tests/generic/4201.out | 2 + 2 files changed, 110 insertions(+) create mode 100755 tests/generic/4201 create mode 100644 tests/generic/4201.out diff --git a/tests/generic/4201 b/tests/generic/4201 new file mode 100755 index 000000000000..fc367beeadaf --- /dev/null +++ b/tests/generic/4201 @@ -0,0 +1,108 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright 2019 Google LLC +# +# FS QA Test generic/4201 +# +# Race dropping file systems caches vs fsstress an repeated reading of files. +# Based on generic/579 with the fsverity bits stripped out, and an increased +# number of runs. +# +. ./common/preamble +_begin_fstest auto stress + +stop_run() +{ + # Stop all subprocesses. + _kill_fsstress + touch $tmp.done + wait +} + +_cleanup() +{ + stop_run + rm -f $tmp.* +} + +. ./common/filter + +fsv_file_size=10000000 +nproc_enabler=$((4 * LOAD_FACTOR)) +nproc_reader=$((6 * LOAD_FACTOR)) +nproc_stress=$((3 * LOAD_FACTOR)) +runtime=$((20 * TIME_FACTOR)) + +run_test() +{ + rm -rf $tmp.done + + # Create the test files and start the fs-verity enabler processes. + for ((proc = 0; proc < nproc_enabler; proc++)); do + orig_file=$SCRATCH_MNT/orig$proc + fsv_file=$SCRATCH_MNT/fsv$proc + head -c $fsv_file_size /dev/urandom > $orig_file + ( + while [ ! -e $tmp.done ]; do + rm -f $fsv_file + cp $orig_file $fsv_file + # Give the readers some time to read from the file. + sleep 0.$((RANDOM % 100)) + done + ) & + done + + # Start the reader processes. + for ((proc = 0; proc < nproc_reader; proc++)); do + ( + while [ ! -e $tmp.done ]; do + # Choose a random file for each iteration, so that + # sometimes multiple processes read from the same file. + i=$((RANDOM % nproc_enabler)) + orig_file=$SCRATCH_MNT/orig$i + fsv_file=$SCRATCH_MNT/fsv$i + + # After the copy from $orig_file to $fsv_file has + # completed, the contents of these two files should + # match, regardless of whether verity has been enabled + # or not yet (or is currently being enabled). + cmp $orig_file $fsv_file |& _filter_scratch | \ + sed -e "s/'//g" | \ + grep -v "SCRATCH_MNT/fsv$i: No such file or directory" | \ + grep -v "EOF on SCRATCH_MNT/fsv$i" + done + ) & + done + + # Start a process that occasionally runs 'sync && drop_caches'. This makes more + # reads go through fs-verity for real, rather than just returning pagecache. + ( + while [ ! -e $tmp.done ]; do + sleep 2.$((RANDOM % 100)) + _scratch_sync && echo 3 > /proc/sys/vm/drop_caches + done + ) & + + # Start the fsstress processes. + _run_fsstress_bg -p $nproc_stress -l 0 -d $SCRATCH_MNT/stressdir + + # Run for a while. + sleep $runtime + + stop_run +} + +for i in $(seq 1 5); do + echo "Starting run $i" >> $seqres.full + + _scratch_mkfs &>> $seqres.full + _scratch_mount + run_test + _scratch_unmount +done + +echo "Silence is golden" + +# success, all done +status=0 +exit diff --git a/tests/generic/4201.out b/tests/generic/4201.out new file mode 100644 index 000000000000..2c3f3d42ff3f --- /dev/null +++ b/tests/generic/4201.out @@ -0,0 +1,2 @@ +QA output created by 4201 +Silence is golden -- 2.53.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] generic: add a test that stresses metadata eviction 2026-05-18 6:24 [PATCH] generic: add a test that stresses metadata eviction Christoph Hellwig @ 2026-05-18 12:27 ` Carlos Maiolino 2026-05-18 19:03 ` Zorro Lang 0 siblings, 1 reply; 8+ messages in thread From: Carlos Maiolino @ 2026-05-18 12:27 UTC (permalink / raw) To: Christoph Hellwig; +Cc: zlang, cmaiolino, fstests, linux-xfs On Mon, May 18, 2026 at 08:24:53AM +0200, Christoph Hellwig wrote: > The fsverity test generic/759 could be used to reproduce a regression in > the xfs buffer hashing. Carlos Maiolino figured out that even removing > the fsverity-specific bits would reproduce the issue. This test is > thus a copy of generic/759 with the fsverity bits striped out, and > run 5 times as that seems to be the threshold at which the issue > reproduces reliably. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > > Not sure how to best credit Eric for the original patch and Carlos > for the extraction, any ideas? To be fair, Andrey initially suggested stripping off the fs-verity parts, IIRC because he wanted to isolate his own series from this crash. My role was mostly reproduce it and hunt the bug down. Risking to come with yet another tag, perhaps something like: Based-on-generic-579-by: <Eric...> It doesn't look ugly to me, but who knows... > > tests/generic/4201 | 108 +++++++++++++++++++++++++++++++++++++++++ > tests/generic/4201.out | 2 + > 2 files changed, 110 insertions(+) > create mode 100755 tests/generic/4201 > create mode 100644 tests/generic/4201.out > > diff --git a/tests/generic/4201 b/tests/generic/4201 > new file mode 100755 > index 000000000000..fc367beeadaf > --- /dev/null > +++ b/tests/generic/4201 > @@ -0,0 +1,108 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright 2019 Google LLC > +# > +# FS QA Test generic/4201 > +# > +# Race dropping file systems caches vs fsstress an repeated reading of files. > +# Based on generic/579 with the fsverity bits stripped out, and an increased > +# number of runs. > +# > +. ./common/preamble > +_begin_fstest auto stress > + > +stop_run() > +{ > + # Stop all subprocesses. > + _kill_fsstress > + touch $tmp.done > + wait > +} > + > +_cleanup() > +{ > + stop_run > + rm -f $tmp.* > +} > + > +. ./common/filter > + > +fsv_file_size=10000000 > +nproc_enabler=$((4 * LOAD_FACTOR)) > +nproc_reader=$((6 * LOAD_FACTOR)) > +nproc_stress=$((3 * LOAD_FACTOR)) > +runtime=$((20 * TIME_FACTOR)) > + > +run_test() > +{ > + rm -rf $tmp.done > + > + # Create the test files and start the fs-verity enabler processes. > + for ((proc = 0; proc < nproc_enabler; proc++)); do > + orig_file=$SCRATCH_MNT/orig$proc > + fsv_file=$SCRATCH_MNT/fsv$proc > + head -c $fsv_file_size /dev/urandom > $orig_file > + ( > + while [ ! -e $tmp.done ]; do > + rm -f $fsv_file > + cp $orig_file $fsv_file > + # Give the readers some time to read from the file. > + sleep 0.$((RANDOM % 100)) > + done > + ) & > + done > + > + # Start the reader processes. > + for ((proc = 0; proc < nproc_reader; proc++)); do > + ( > + while [ ! -e $tmp.done ]; do > + # Choose a random file for each iteration, so that > + # sometimes multiple processes read from the same file. > + i=$((RANDOM % nproc_enabler)) > + orig_file=$SCRATCH_MNT/orig$i > + fsv_file=$SCRATCH_MNT/fsv$i > + > + # After the copy from $orig_file to $fsv_file has > + # completed, the contents of these two files should > + # match, regardless of whether verity has been enabled > + # or not yet (or is currently being enabled). > + cmp $orig_file $fsv_file |& _filter_scratch | \ > + sed -e "s/'//g" | \ > + grep -v "SCRATCH_MNT/fsv$i: No such file or directory" | \ > + grep -v "EOF on SCRATCH_MNT/fsv$i" > + done > + ) & > + done > + > + # Start a process that occasionally runs 'sync && drop_caches'. This makes more > + # reads go through fs-verity for real, rather than just returning pagecache. > + ( > + while [ ! -e $tmp.done ]; do > + sleep 2.$((RANDOM % 100)) > + _scratch_sync && echo 3 > /proc/sys/vm/drop_caches > + done > + ) & > + > + # Start the fsstress processes. > + _run_fsstress_bg -p $nproc_stress -l 0 -d $SCRATCH_MNT/stressdir > + > + # Run for a while. > + sleep $runtime > + > + stop_run > +} > + > +for i in $(seq 1 5); do > + echo "Starting run $i" >> $seqres.full > + > + _scratch_mkfs &>> $seqres.full > + _scratch_mount > + run_test > + _scratch_unmount > +done > + > +echo "Silence is golden" > + > +# success, all done > +status=0 > +exit > diff --git a/tests/generic/4201.out b/tests/generic/4201.out > new file mode 100644 > index 000000000000..2c3f3d42ff3f > --- /dev/null > +++ b/tests/generic/4201.out > @@ -0,0 +1,2 @@ > +QA output created by 4201 > +Silence is golden > -- > 2.53.0 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] generic: add a test that stresses metadata eviction 2026-05-18 12:27 ` Carlos Maiolino @ 2026-05-18 19:03 ` Zorro Lang 2026-05-19 8:37 ` Christoph Hellwig 2026-05-19 12:57 ` Theodore Tso 0 siblings, 2 replies; 8+ messages in thread From: Zorro Lang @ 2026-05-18 19:03 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Carlos Maiolino, fstests, linux-xfs, Eric Biggers On Mon, May 18, 2026 at 02:27:42PM +0200, Carlos Maiolino wrote: > On Mon, May 18, 2026 at 08:24:53AM +0200, Christoph Hellwig wrote: > > The fsverity test generic/759 could be used to reproduce a regression in ^^^ 579 > > the xfs buffer hashing. Carlos Maiolino figured out that even removing > > the fsverity-specific bits would reproduce the issue. This test is > > thus a copy of generic/759 with the fsverity bits striped out, and > > run 5 times as that seems to be the threshold at which the issue > > reproduces reliably. > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > --- > > > > Not sure how to best credit Eric for the original patch and Carlos > > for the extraction, any ideas? > > To be fair, Andrey initially suggested stripping off the fs-verity > parts, IIRC because he wanted to isolate his own series from this crash. > My role was mostly reproduce it and hunt the bug down. > Risking to come with yet another tag, perhaps something like: > > Based-on-generic-579-by: <Eric...> > > It doesn't look ugly to me, but who knows... Hmm... Generally, we state which case was copied/forked from in the introductory comments of the case and in the commit log. If you want to specifically mention the original author, I think we can add a dedicated line in the commit log, for example: 'This case is copied from generic/579, which was written by Eric Biggers.' Currently, we don’t have a formal commit trailer to define this specific scenario, but perhaps this is a good opportunity for us to discuss and agree on one. After all, copying and modifying existing cases happens quite often in xfstests. For example, we could specify the source case in the commit body, and then add a commit trailer likes "Based-on-code-by", the whole things like: This case is copied by generic/579, I removed the fsverity-specific part to reproduce .... ... Based-on-code-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Christoph Hellwig <hch@lst.de> What do you and others think? > > > > > tests/generic/4201 | 108 +++++++++++++++++++++++++++++++++++++++++ > > tests/generic/4201.out | 2 + > > 2 files changed, 110 insertions(+) > > create mode 100755 tests/generic/4201 > > create mode 100644 tests/generic/4201.out > > > > diff --git a/tests/generic/4201 b/tests/generic/4201 > > new file mode 100755 > > index 000000000000..fc367beeadaf > > --- /dev/null > > +++ b/tests/generic/4201 > > @@ -0,0 +1,108 @@ > > +#! /bin/bash > > +# SPDX-License-Identifier: GPL-2.0 > > +# Copyright 2019 Google LLC I saw you tried to keep the original copyright of generic/579. I'm not sure if we can have two Copyright lines at here, one is yours, the other is the original one? Thanks, Zorro > > +# > > +# FS QA Test generic/4201 > > +# > > +# Race dropping file systems caches vs fsstress an repeated reading of files. > > +# Based on generic/579 with the fsverity bits stripped out, and an increased > > +# number of runs. > > +# > > +. ./common/preamble > > +_begin_fstest auto stress > > + > > +stop_run() > > +{ > > + # Stop all subprocesses. > > + _kill_fsstress > > + touch $tmp.done > > + wait > > +} > > + > > +_cleanup() > > +{ > > + stop_run > > + rm -f $tmp.* > > +} > > + > > +. ./common/filter > > + > > +fsv_file_size=10000000 > > +nproc_enabler=$((4 * LOAD_FACTOR)) > > +nproc_reader=$((6 * LOAD_FACTOR)) > > +nproc_stress=$((3 * LOAD_FACTOR)) > > +runtime=$((20 * TIME_FACTOR)) > > + > > +run_test() > > +{ > > + rm -rf $tmp.done > > + > > + # Create the test files and start the fs-verity enabler processes. > > + for ((proc = 0; proc < nproc_enabler; proc++)); do > > + orig_file=$SCRATCH_MNT/orig$proc > > + fsv_file=$SCRATCH_MNT/fsv$proc > > + head -c $fsv_file_size /dev/urandom > $orig_file > > + ( > > + while [ ! -e $tmp.done ]; do > > + rm -f $fsv_file > > + cp $orig_file $fsv_file > > + # Give the readers some time to read from the file. > > + sleep 0.$((RANDOM % 100)) > > + done > > + ) & > > + done > > + > > + # Start the reader processes. > > + for ((proc = 0; proc < nproc_reader; proc++)); do > > + ( > > + while [ ! -e $tmp.done ]; do > > + # Choose a random file for each iteration, so that > > + # sometimes multiple processes read from the same file. > > + i=$((RANDOM % nproc_enabler)) > > + orig_file=$SCRATCH_MNT/orig$i > > + fsv_file=$SCRATCH_MNT/fsv$i > > + > > + # After the copy from $orig_file to $fsv_file has > > + # completed, the contents of these two files should > > + # match, regardless of whether verity has been enabled > > + # or not yet (or is currently being enabled). > > + cmp $orig_file $fsv_file |& _filter_scratch | \ > > + sed -e "s/'//g" | \ > > + grep -v "SCRATCH_MNT/fsv$i: No such file or directory" | \ > > + grep -v "EOF on SCRATCH_MNT/fsv$i" > > + done > > + ) & > > + done > > + > > + # Start a process that occasionally runs 'sync && drop_caches'. This makes more > > + # reads go through fs-verity for real, rather than just returning pagecache. > > + ( > > + while [ ! -e $tmp.done ]; do > > + sleep 2.$((RANDOM % 100)) > > + _scratch_sync && echo 3 > /proc/sys/vm/drop_caches > > + done > > + ) & > > + > > + # Start the fsstress processes. > > + _run_fsstress_bg -p $nproc_stress -l 0 -d $SCRATCH_MNT/stressdir > > + > > + # Run for a while. > > + sleep $runtime > > + > > + stop_run > > +} > > + > > +for i in $(seq 1 5); do > > + echo "Starting run $i" >> $seqres.full > > + > > + _scratch_mkfs &>> $seqres.full > > + _scratch_mount > > + run_test > > + _scratch_unmount > > +done > > + > > +echo "Silence is golden" > > + > > +# success, all done > > +status=0 > > +exit > > diff --git a/tests/generic/4201.out b/tests/generic/4201.out > > new file mode 100644 > > index 000000000000..2c3f3d42ff3f > > --- /dev/null > > +++ b/tests/generic/4201.out > > @@ -0,0 +1,2 @@ > > +QA output created by 4201 > > +Silence is golden > > -- > > 2.53.0 > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] generic: add a test that stresses metadata eviction 2026-05-18 19:03 ` Zorro Lang @ 2026-05-19 8:37 ` Christoph Hellwig 2026-05-19 17:58 ` Eric Biggers 2026-05-19 20:47 ` Zorro Lang 2026-05-19 12:57 ` Theodore Tso 1 sibling, 2 replies; 8+ messages in thread From: Christoph Hellwig @ 2026-05-19 8:37 UTC (permalink / raw) To: Christoph Hellwig, Carlos Maiolino, fstests, linux-xfs, Eric Biggers On Tue, May 19, 2026 at 03:03:37AM +0800, Zorro Lang wrote: > On Mon, May 18, 2026 at 02:27:42PM +0200, Carlos Maiolino wrote: > > On Mon, May 18, 2026 at 08:24:53AM +0200, Christoph Hellwig wrote: > > > The fsverity test generic/759 could be used to reproduce a regression in > ^^^ > 579 Yeah. > Hmm... Generally, we state which case was copied/forked from in the introductory > comments of the case and in the commit log. If you want to specifically mention > the original author, I think we can add a dedicated line in the commit log, for > example: If Eric is fine with that I can stick to the normal rule. > What do you and others think? I don't really care at all. I just want to make sure I don't smub everyone involved with this the wrong way :) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] generic: add a test that stresses metadata eviction 2026-05-19 8:37 ` Christoph Hellwig @ 2026-05-19 17:58 ` Eric Biggers 2026-05-19 20:47 ` Zorro Lang 1 sibling, 0 replies; 8+ messages in thread From: Eric Biggers @ 2026-05-19 17:58 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Carlos Maiolino, fstests, linux-xfs On Tue, May 19, 2026 at 10:37:58AM +0200, Christoph Hellwig wrote: > On Tue, May 19, 2026 at 03:03:37AM +0800, Zorro Lang wrote: > > On Mon, May 18, 2026 at 02:27:42PM +0200, Carlos Maiolino wrote: > > > On Mon, May 18, 2026 at 08:24:53AM +0200, Christoph Hellwig wrote: > > > > The fsverity test generic/759 could be used to reproduce a regression in > > ^^^ > > 579 > > Yeah. > > > Hmm... Generally, we state which case was copied/forked from in the introductory > > comments of the case and in the commit log. If you want to specifically mention > > the original author, I think we can add a dedicated line in the commit log, for > > example: > > If Eric is fine with that I can stick to the normal rule. > > > What do you and others think? > > I don't really care at all. I just want to make sure I don't smub > everyone involved with this the wrong way :) The patch looks fine as-is (other than the typo already pointed out). It already preserves the original copyright line and has a comment "# Based on generic/579 with the fsverity bits stripped out". I don't need any additional credit. Thanks, - Eric ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] generic: add a test that stresses metadata eviction 2026-05-19 8:37 ` Christoph Hellwig 2026-05-19 17:58 ` Eric Biggers @ 2026-05-19 20:47 ` Zorro Lang 1 sibling, 0 replies; 8+ messages in thread From: Zorro Lang @ 2026-05-19 20:47 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Carlos Maiolino, fstests, linux-xfs, Eric Biggers On Tue, May 19, 2026 at 10:37:58AM +0200, Christoph Hellwig wrote: > On Tue, May 19, 2026 at 03:03:37AM +0800, Zorro Lang wrote: > > On Mon, May 18, 2026 at 02:27:42PM +0200, Carlos Maiolino wrote: > > > On Mon, May 18, 2026 at 08:24:53AM +0200, Christoph Hellwig wrote: > > > > The fsverity test generic/759 could be used to reproduce a regression in > > ^^^ > > 579 > > Yeah. > > > Hmm... Generally, we state which case was copied/forked from in the introductory > > comments of the case and in the commit log. If you want to specifically mention > > the original author, I think we can add a dedicated line in the commit log, for > > example: > > If Eric is fine with that I can stick to the normal rule. > > > What do you and others think? > > I don't really care at all. I just want to make sure I don't smub > everyone involved with this the wrong way :) I've merged this patch with above typo fixes. Please feel free to check patches-in-queue branch. Thanks, Zorro > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] generic: add a test that stresses metadata eviction 2026-05-18 19:03 ` Zorro Lang 2026-05-19 8:37 ` Christoph Hellwig @ 2026-05-19 12:57 ` Theodore Tso 2026-05-19 20:39 ` Zorro Lang 1 sibling, 1 reply; 8+ messages in thread From: Theodore Tso @ 2026-05-19 12:57 UTC (permalink / raw) To: Christoph Hellwig, Carlos Maiolino, fstests, linux-xfs, Eric Biggers On Tue, May 19, 2026 at 03:03:37AM +0800, Zorro Lang wrote: > Currently, we don’t have a formal commit trailer to define this > specific scenario, but perhaps this is a good opportunity for us to > discuss and agree on one. My suggestion is that this should go in a comment of the forked test, e.g. # Based on generic/579 with fsverity-specific bits removed That way, if someone fixes a problem in generic/579 in the future, a quick grep might turn up other tests that need a similar adjustment. The commit trailer doesn't seem as useful to me since it's harder to find. - Ted ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] generic: add a test that stresses metadata eviction 2026-05-19 12:57 ` Theodore Tso @ 2026-05-19 20:39 ` Zorro Lang 0 siblings, 0 replies; 8+ messages in thread From: Zorro Lang @ 2026-05-19 20:39 UTC (permalink / raw) To: Theodore Tso Cc: Christoph Hellwig, Carlos Maiolino, fstests, linux-xfs, Eric Biggers On Tue, May 19, 2026 at 08:57:04AM -0400, Theodore Tso wrote: > On Tue, May 19, 2026 at 03:03:37AM +0800, Zorro Lang wrote: > > Currently, we don’t have a formal commit trailer to define this > > specific scenario, but perhaps this is a good opportunity for us to > > discuss and agree on one. > > My suggestion is that this should go in a comment of the forked test, e.g. > > # Based on generic/579 with fsverity-specific bits removed > > That way, if someone fixes a problem in generic/579 in the future, a > quick grep might turn up other tests that need a similar adjustment. > The commit trailer doesn't seem as useful to me since it's harder to > find. Thanks Ted, since Eric agreed with this as well and there are no other objections, I think we can hold off on creating a new git trailer. Let's just proceed as usual: mention the original case number in the commit log, and also add it to the case's initial comments would make it easier to track. Thanks, Zorro > > - Ted > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-05-19 20:47 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-18 6:24 [PATCH] generic: add a test that stresses metadata eviction Christoph Hellwig 2026-05-18 12:27 ` Carlos Maiolino 2026-05-18 19:03 ` Zorro Lang 2026-05-19 8:37 ` Christoph Hellwig 2026-05-19 17:58 ` Eric Biggers 2026-05-19 20:47 ` Zorro Lang 2026-05-19 12:57 ` Theodore Tso 2026-05-19 20:39 ` Zorro Lang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox