On Mon, 2025-01-20 at 13:57 -0500, James Bottomley wrote:
> On Mon, 2025-01-20 at 17:31 +0100, Ard Biesheuvel wrote:
> > On Sun, 19 Jan 2025 at 17:59, Ard Biesheuvel <ardb@xxxxxxxxxx>
> > wrote:
> > >
> > > On Sun, 19 Jan 2025 at 16:12, James Bottomley
> > > <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:
> > > >
> > ...
> > >
> > > Thanks James. I've queued up this version now, so we'll get some
> > > coverage from the robots. I'll redo my own testing tomorrow, but
> > > I'll omit these changes from my initial PR to Linus. If we're
> > > confident that things are sound, I'll send another PR during the
> > > second half of the merge window.
> >
> > I'm hitting the failure cases below. The first one appears to hit
> > the same 'Operation not permitted' condition on the write, the
> > error message is just hidden by the /dev/null redirect.
> >
> > I'm running the make command from a root shell. Using printf from
> > the command line works happily so I suspect there is some issue
> > with the concurrency and the subshells?
>
> It could be that the file isn't opened until the subshell is spawned.
> I can probably use the pipe to wait for the subshell to start; I'll
> try to code that up.
OK, this is what I came up with. It works for me (but then so did the
other script ... I think my testing VM is probably a little slow).
Regards,
James
--->8>8>8<8<8<8---
From: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
Subject: [PATCH] selftests/efivarfs: add concurrent update tests
The delete on last close functionality can now only be tested properly
by using multiple threads to hold open the variable files and testing
what happens as they complete.
Signed-off-by: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
---
diff --git a/tools/testing/selftests/efivarfs/efivarfs.sh b/tools/testing/selftests/efivarfs/efivarfs.sh
index 4a84a810dc2c..57e2e151b8d9 100755
--- a/tools/testing/selftests/efivarfs/efivarfs.sh
+++ b/tools/testing/selftests/efivarfs/efivarfs.sh
@@ -227,6 +227,136 @@ test_no_set_size()
exit $ret
}
+setup_test_multiple()
+{
+ ##
+ # we're going to do multi-threaded tests, so create a set of
+ # pipes for synchronization. We use pipes 1..3 to start the
+ # stalled shell job and pipes 4..6 as indicators that the job
+ # has started. If you need more than 3 jobs the two +3's below
+ # need increasing
+ ##
+
+ declare -ag p
+
+ # empty is because arrays number from 0 but jobs number from 1
+ p[0]=""
+
+ for f in 1 2 3 4 5 6; do
+ p[$f]=/tmp/efivarfs_pipe${f}
+ mknod ${p[$f]} p
+ done
+
+ declare -g var=$efivarfs_mount/test_multiple-$test_guid
+
+ cleanup() {
+ for f in ${p[@]}; do
+ rm -f ${f}
+ done
+ if [ -e $var ]; then
+ file_cleanup $var
+ fi
+ }
+ trap cleanup exit
+
+ waitstart() {
+ cat ${p[$[$1+3]]} > /dev/null
+ }
+
+ waitpipe() {
+ echo 1 > ${p[$[$1+3]]}
+ cat ${p[$1]} > /dev/null
+ }
+
+ endjob() {
+ echo 1 > ${p[$1]}
+ wait -n %$1
+ }
+}
+
+test_multiple_zero_size()
+{
+ ##
+ # check for remove on last close, set up three threads all
+ # holding the variable (one write and two reads) and then
+ # close them sequentially (waiting for completion) and check
+ # the state of the variable
+ ##
+
+ { waitpipe 1; echo 1; } > $var 2> /dev/null &
+ waitstart 1
+ # zero length file should exist
+ [ -e $var ] || exit 1
+ # second and third delayed close
+ { waitpipe 2; } < $var &
+ waitstart 2
+ { waitpipe 3; } < $var &
+ waitstart 3
+ # close first fd
+ endjob 1
+ # var should only be deleted on last close
+ [ -e $var ] || exit 1
+ # close second fd
+ endjob 2
+ [ -e $var ] || exit 1
+ # file should go on last close
+ endjob 3
+ [ ! -e $var ] || exit 1
+}
+
+test_multiple_create()
+{
+ ##
+ # set multiple threads to access the variable but delay
+ # the final write to check the close of 2 and 3. The
+ # final write should succeed in creating the variable
+ ##
+ { waitpipe 1; printf '\x07\x00\x00\x00\x54'; } > $var &
+ waitstart 1
+ [ -e $var -a ! -s $var ] || exit 1
+ { waitpipe 2; } < $var &
+ waitstart 2
+ { waitpipe 3; } < $var &
+ waitstart 3
+ # close second and third fds
+ endjob 2
+ # var should only be created (have size) on last close
+ [ -e $var -a ! -s $var ] || exit 1
+ endjob 3
+ [ -e $var -a ! -s $var ] || exit 1
+ # close first fd
+ endjob 1
+ # variable should still exist
+ [ -s $var ] || exit 1
+ file_cleanup $var
+}
+
+test_multiple_delete_on_write() {
+ ##
+ # delete the variable on final write; seqencing similar
+ # to test_multiple_create()
+ ##
+ printf '\x07\x00\x00\x00\x54' > $var
+ chattr -i $var
+ { waitpipe 1; printf '\x07\x00\x00\x00'; } > $var &
+ waitstart 1
+ [ -e $var -a -s $var ] || exit 1
+ { waitpipe 2; } < $var &
+ waitstart 2
+ { waitpipe 3; } < $var &
+ waitstart 3
+ # close first fd; write should set variable size to zero
+ endjob 1
+ # var should only be deleted on last close
+ [ -e $var -a ! -s $var ] || exit 1
+ endjob 2
+ [ -e $var ] || exit 1
+ # close last fd
+ endjob 3
+ # variable should now be removed
+ [ ! -e $var ] || exit 1
+}
+
check_prereqs
rc=0
@@ -240,5 +370,9 @@ run_test test_open_unlink
run_test test_valid_filenames
run_test test_invalid_filenames
run_test test_no_set_size
+setup_test_multiple
+run_test test_multiple_zero_size
+run_test test_multiple_create
+run_test test_multiple_delete_on_write
exit $rc
