| Message ID | 20231129032123.2658343-4-shahuang@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: runtime scripts improvements on efi | expand |
On Tue, Nov 28, 2023 at 10:21:23PM -0500, Shaoqin Huang wrote: > Currently running tests on EFI in parallel can cause part of tests to > fail, this is because arm/efi/run script use the EFI_CASE to create the > subdir under the efi-tests, and the EFI_CASE is the filename of the > test, when running tests in parallel, the multiple tests exist in the > same filename will execute at the same time, which will use the same > directory and write the test specific things into it, this cause > chaotic and make some tests fail. How do they fail? iiuc, we're switching from having one of each unique binary on the efi partition to multiple redundant binaries, since we copy the binary for each test, even when it's the same as for other tests. It seems like we should be able to keep single unique binaries and resolve the parallel execution failure by just checking for existence of the binaries or only creating test-specific data directories or something. > > To Fix this things, use the testname instead of the filename to create > the subdir under the efi-tests. We use the EFI_TESTNAME to replace the > EFI_CASE in script. Since every testname is specific, now the tests > can be run parallel. It also considers when user directly use the > arm/efi/run to run test, in this case, still use the filename. > > Besides, replace multiple $EFI_TEST/$EFI_CASE to the $EFI_CASE_DIR, this > makes the script looks more clean and we don'e need to replace many > EFI_CASE to EFI_TESTNAME. > > Signed-off-by: Shaoqin Huang <shahuang@redhat.com> > --- > arm/efi/run | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/arm/efi/run b/arm/efi/run > index 6872c337..03bfbef4 100755 > --- a/arm/efi/run > +++ b/arm/efi/run > @@ -24,6 +24,8 @@ fi > : "${EFI_SRC:=$TEST_DIR}" > : "${EFI_UEFI:=$DEFAULT_UEFI}" > : "${EFI_TEST:=efi-tests}" > +: "${EFI_TESTNAME:=$TESTNAME}" > +: "${EFI_TESTNAME:=$(basename $1 .efi)}" > : "${EFI_CASE:=$(basename $1 .efi)}" > : "${EFI_VAR_GUID:=97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823}" > > @@ -56,20 +58,20 @@ if [ "$EFI_CASE" = "_NO_FILE_4Uhere_" ]; then > EFI_CASE=dummy > fi > > -: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_CASE"}" > +: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_TESTNAME"}" > mkdir -p "$EFI_CASE_DIR" > > -cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_TEST/$EFI_CASE/" > -echo "@echo -off" > "$EFI_TEST/$EFI_CASE/startup.nsh" > +cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_DIR/" > +echo "@echo -off" > "$EFI_CASE_DIR/startup.nsh" Unrelated change, should be a separate patch. > if [ "$EFI_USE_DTB" = "y" ]; then > qemu_args+=(-machine acpi=off) > FDT_BASENAME="dtb" > - $(EFI_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_TEST/$EFI_CASE/$FDT_BASENAME" "${qemu_args[@]}") > - echo "setvar fdtfile -guid $EFI_VAR_GUID -rt =L\"$FDT_BASENAME\"" >> "$EFI_TEST/$EFI_CASE/startup.nsh" > + $(EFI_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_CASE_DIR/$FDT_BASENAME" "${qemu_args[@]}") > + echo "setvar fdtfile -guid $EFI_VAR_GUID -rt =L\"$FDT_BASENAME\"" >> "$EFI_CASE_DIR/startup.nsh" > fi > -echo "$EFI_CASE.efi" "${cmd_args[@]}" >> "$EFI_TEST/$EFI_CASE/startup.nsh" > +echo "$EFI_CASE.efi" "${cmd_args[@]}" >> "$EFI_CASE_DIR/startup.nsh" > > EFI_RUN=y $TEST_DIR/run \ > -bios "$EFI_UEFI" \ > - -drive file.dir="$EFI_TEST/$EFI_CASE/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \ > + -drive file.dir="$EFI_CASE_DIR/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \ > "${qemu_args[@]}" > -- > 2.40.1 > Thanks, drew
Hi drew, On 11/29/23 17:27, Andrew Jones wrote: > On Tue, Nov 28, 2023 at 10:21:23PM -0500, Shaoqin Huang wrote: >> Currently running tests on EFI in parallel can cause part of tests to >> fail, this is because arm/efi/run script use the EFI_CASE to create the >> subdir under the efi-tests, and the EFI_CASE is the filename of the >> test, when running tests in parallel, the multiple tests exist in the >> same filename will execute at the same time, which will use the same >> directory and write the test specific things into it, this cause >> chaotic and make some tests fail. > > How do they fail? iiuc, we're switching from having one of each unique > binary on the efi partition to multiple redundant binaries, since we > copy the binary for each test, even when it's the same as for other > tests. It seems like we should be able to keep single unique binaries > and resolve the parallel execution failure by just checking for existence > of the binaries or only creating test-specific data directories or > something. > The problem comes from the arm/efi/run script, as you can see. If we parallel running multiple tests on efi, for example, running the pmu-sw-incr and pmu-chained-counters and other pmu tests at the same time, the EFI_CASE will be pmu. So they will write their $cmd_args to the $EFI/TEST/pmu/startup.nsh at the same time, which will corrupt the startup.nsh file. cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_TEST/$EFI_CASE/" echo "@echo -off" > "$EFI_TEST/$EFI_CASE/startup.nsh" ... echo "$EFI_CASE.efi" "${cmd_args[@]}" >> "$EFI_TEST/$EFI_CASE/startup.nsh" And I can get the log which outputs: * pmu-sw-incr.log: - ABORT: pmu: Unknown sub-test 'pmu-mem-acce' * pmu-chained-counters.log - ABORT: pmu: Unknown sub-test 'pmu-mem-access-reliab' And the efi-tests/pmu/startup.nsh: @echo -off setvar fdtfile -guid 97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823 -rt =L"dtb" pmu.efi pmu-mem-access-reliability setvar fdtfile -guid 97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823 -rt =L"dtb" pmu.efi pmu-chained-sw-incr Thus when running parallel, some of them will fail. So I create different sub-dir in the efi-tests for each small test. Thanks, Shaoqin >> >> To Fix this things, use the testname instead of the filename to create >> the subdir under the efi-tests. We use the EFI_TESTNAME to replace the >> EFI_CASE in script. Since every testname is specific, now the tests >> can be run parallel. It also considers when user directly use the >> arm/efi/run to run test, in this case, still use the filename. >> >> Besides, replace multiple $EFI_TEST/$EFI_CASE to the $EFI_CASE_DIR, this >> makes the script looks more clean and we don'e need to replace many >> EFI_CASE to EFI_TESTNAME. >> >> Signed-off-by: Shaoqin Huang <shahuang@redhat.com> >> --- >> arm/efi/run | 16 +++++++++------- >> 1 file changed, 9 insertions(+), 7 deletions(-) >> >> diff --git a/arm/efi/run b/arm/efi/run >> index 6872c337..03bfbef4 100755 >> --- a/arm/efi/run >> +++ b/arm/efi/run >> @@ -24,6 +24,8 @@ fi >> : "${EFI_SRC:=$TEST_DIR}" >> : "${EFI_UEFI:=$DEFAULT_UEFI}" >> : "${EFI_TEST:=efi-tests}" >> +: "${EFI_TESTNAME:=$TESTNAME}" >> +: "${EFI_TESTNAME:=$(basename $1 .efi)}" >> : "${EFI_CASE:=$(basename $1 .efi)}" >> : "${EFI_VAR_GUID:=97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823}" >> >> @@ -56,20 +58,20 @@ if [ "$EFI_CASE" = "_NO_FILE_4Uhere_" ]; then >> EFI_CASE=dummy >> fi >> >> -: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_CASE"}" >> +: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_TESTNAME"}" >> mkdir -p "$EFI_CASE_DIR" >> >> -cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_TEST/$EFI_CASE/" >> -echo "@echo -off" > "$EFI_TEST/$EFI_CASE/startup.nsh" >> +cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_DIR/" >> +echo "@echo -off" > "$EFI_CASE_DIR/startup.nsh" > > Unrelated change, should be a separate patch. > Ok, Will separate it. >> if [ "$EFI_USE_DTB" = "y" ]; then >> qemu_args+=(-machine acpi=off) >> FDT_BASENAME="dtb" >> - $(EFI_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_TEST/$EFI_CASE/$FDT_BASENAME" "${qemu_args[@]}") >> - echo "setvar fdtfile -guid $EFI_VAR_GUID -rt =L\"$FDT_BASENAME\"" >> "$EFI_TEST/$EFI_CASE/startup.nsh" >> + $(EFI_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_CASE_DIR/$FDT_BASENAME" "${qemu_args[@]}") >> + echo "setvar fdtfile -guid $EFI_VAR_GUID -rt =L\"$FDT_BASENAME\"" >> "$EFI_CASE_DIR/startup.nsh" >> fi >> -echo "$EFI_CASE.efi" "${cmd_args[@]}" >> "$EFI_TEST/$EFI_CASE/startup.nsh" >> +echo "$EFI_CASE.efi" "${cmd_args[@]}" >> "$EFI_CASE_DIR/startup.nsh" >> >> EFI_RUN=y $TEST_DIR/run \ >> -bios "$EFI_UEFI" \ >> - -drive file.dir="$EFI_TEST/$EFI_CASE/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \ >> + -drive file.dir="$EFI_CASE_DIR/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \ >> "${qemu_args[@]}" >> -- >> 2.40.1 >> > > Thanks, > drew >
On Wed, Nov 29, 2023 at 06:14:20PM +0800, Shaoqin Huang wrote: > Hi drew, > > On 11/29/23 17:27, Andrew Jones wrote: > > On Tue, Nov 28, 2023 at 10:21:23PM -0500, Shaoqin Huang wrote: > > > Currently running tests on EFI in parallel can cause part of tests to > > > fail, this is because arm/efi/run script use the EFI_CASE to create the > > > subdir under the efi-tests, and the EFI_CASE is the filename of the > > > test, when running tests in parallel, the multiple tests exist in the > > > same filename will execute at the same time, which will use the same > > > directory and write the test specific things into it, this cause > > > chaotic and make some tests fail. > > > > How do they fail? iiuc, we're switching from having one of each unique > > binary on the efi partition to multiple redundant binaries, since we > > copy the binary for each test, even when it's the same as for other > > tests. It seems like we should be able to keep single unique binaries > > and resolve the parallel execution failure by just checking for existence > > of the binaries or only creating test-specific data directories or > > something. > > > > The problem comes from the arm/efi/run script, as you can see. If we > parallel running multiple tests on efi, for example, running the pmu-sw-incr > and pmu-chained-counters and other pmu tests at the same time, the EFI_CASE > will be pmu. So they will write their $cmd_args to the > $EFI/TEST/pmu/startup.nsh at the same time, which will corrupt the > startup.nsh file. > > cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_TEST/$EFI_CASE/" > echo "@echo -off" > "$EFI_TEST/$EFI_CASE/startup.nsh" > ... > echo "$EFI_CASE.efi" "${cmd_args[@]}" >> "$EFI_TEST/$EFI_CASE/startup.nsh" > > And I can get the log which outputs: > > * pmu-sw-incr.log: > - ABORT: pmu: Unknown sub-test 'pmu-mem-acce' > * pmu-chained-counters.log > - ABORT: pmu: Unknown sub-test 'pmu-mem-access-reliab' > > And the efi-tests/pmu/startup.nsh: > > @echo -off > setvar fdtfile -guid 97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823 -rt =L"dtb" > pmu.efi pmu-mem-access-reliability > setvar fdtfile -guid 97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823 -rt =L"dtb" > pmu.efi pmu-chained-sw-incr > > > Thus when running parallel, some of them will fail. So I create different > sub-dir in the efi-tests for each small test. Ok, I was guessing it was something like that. Maybe we should create a "bin" type of named directory for all the binaries and then create a separate subdir for each test and its startup.nsh, rather than copying the binaries multiple times. > > > : "${EFI_CASE:=$(basename $1 .efi)}" > > > : "${EFI_VAR_GUID:=97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823}" > > > @@ -56,20 +58,20 @@ if [ "$EFI_CASE" = "_NO_FILE_4Uhere_" ]; then > > > EFI_CASE=dummy > > > fi > > > -: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_CASE"}" > > > +: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_TESTNAME"}" > > > mkdir -p "$EFI_CASE_DIR" > > > -cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_TEST/$EFI_CASE/" > > > -echo "@echo -off" > "$EFI_TEST/$EFI_CASE/startup.nsh" > > > +cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_DIR/" > > > +echo "@echo -off" > "$EFI_CASE_DIR/startup.nsh" > > > > Unrelated change, should be a separate patch. > > > > Ok, Will separate it. Actually, disregard my comment. I was too hasty here and thought the '@echo -off' was getting added, but now I see only the path was getting updated. It's correct to make this change here in this patch. Thanks, drew
Hi drew, On 11/29/23 19:43, Andrew Jones wrote: > On Wed, Nov 29, 2023 at 06:14:20PM +0800, Shaoqin Huang wrote: >> Hi drew, >> >> On 11/29/23 17:27, Andrew Jones wrote: >>> On Tue, Nov 28, 2023 at 10:21:23PM -0500, Shaoqin Huang wrote: >>>> Currently running tests on EFI in parallel can cause part of tests to >>>> fail, this is because arm/efi/run script use the EFI_CASE to create the >>>> subdir under the efi-tests, and the EFI_CASE is the filename of the >>>> test, when running tests in parallel, the multiple tests exist in the >>>> same filename will execute at the same time, which will use the same >>>> directory and write the test specific things into it, this cause >>>> chaotic and make some tests fail. >>> >>> How do they fail? iiuc, we're switching from having one of each unique >>> binary on the efi partition to multiple redundant binaries, since we >>> copy the binary for each test, even when it's the same as for other >>> tests. It seems like we should be able to keep single unique binaries >>> and resolve the parallel execution failure by just checking for existence >>> of the binaries or only creating test-specific data directories or >>> something. >>> >> >> The problem comes from the arm/efi/run script, as you can see. If we >> parallel running multiple tests on efi, for example, running the pmu-sw-incr >> and pmu-chained-counters and other pmu tests at the same time, the EFI_CASE >> will be pmu. So they will write their $cmd_args to the >> $EFI/TEST/pmu/startup.nsh at the same time, which will corrupt the >> startup.nsh file. >> >> cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_TEST/$EFI_CASE/" >> echo "@echo -off" > "$EFI_TEST/$EFI_CASE/startup.nsh" >> ... >> echo "$EFI_CASE.efi" "${cmd_args[@]}" >> "$EFI_TEST/$EFI_CASE/startup.nsh" >> >> And I can get the log which outputs: >> >> * pmu-sw-incr.log: >> - ABORT: pmu: Unknown sub-test 'pmu-mem-acce' >> * pmu-chained-counters.log >> - ABORT: pmu: Unknown sub-test 'pmu-mem-access-reliab' >> >> And the efi-tests/pmu/startup.nsh: >> >> @echo -off >> setvar fdtfile -guid 97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823 -rt =L"dtb" >> pmu.efi pmu-mem-access-reliability >> setvar fdtfile -guid 97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823 -rt =L"dtb" >> pmu.efi pmu-chained-sw-incr >> >> >> Thus when running parallel, some of them will fail. So I create different >> sub-dir in the efi-tests for each small test. > > Ok, I was guessing it was something like that. Maybe we should create a > "bin" type of named directory for all the binaries and then create a > separate subdir for each test and its startup.nsh, rather than copying > the binaries multiple times. > I can put this kind of improvement into another patch. Currently I want to use the safety way to fix this problem, create different directory and copy the .efi for each of them will not cause any interfere for each of the test. We can tolerate the redundant, it doesn't cost many space. Thanks, Shaoqin >>>> : "${EFI_CASE:=$(basename $1 .efi)}" >>>> : "${EFI_VAR_GUID:=97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823}" >>>> @@ -56,20 +58,20 @@ if [ "$EFI_CASE" = "_NO_FILE_4Uhere_" ]; then >>>> EFI_CASE=dummy >>>> fi >>>> -: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_CASE"}" >>>> +: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_TESTNAME"}" >>>> mkdir -p "$EFI_CASE_DIR" >>>> -cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_TEST/$EFI_CASE/" >>>> -echo "@echo -off" > "$EFI_TEST/$EFI_CASE/startup.nsh" >>>> +cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_DIR/" >>>> +echo "@echo -off" > "$EFI_CASE_DIR/startup.nsh" >>> >>> Unrelated change, should be a separate patch. >>> >> >> Ok, Will separate it. > > Actually, disregard my comment. I was too hasty here and thought the > '@echo -off' was getting added, but now I see only the path was getting > updated. It's correct to make this change here in this patch. > > Thanks, > drew >
diff --git a/arm/efi/run b/arm/efi/run index 6872c337..03bfbef4 100755 --- a/arm/efi/run +++ b/arm/efi/run @@ -24,6 +24,8 @@ fi : "${EFI_SRC:=$TEST_DIR}" : "${EFI_UEFI:=$DEFAULT_UEFI}" : "${EFI_TEST:=efi-tests}" +: "${EFI_TESTNAME:=$TESTNAME}" +: "${EFI_TESTNAME:=$(basename $1 .efi)}" : "${EFI_CASE:=$(basename $1 .efi)}" : "${EFI_VAR_GUID:=97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823}" @@ -56,20 +58,20 @@ if [ "$EFI_CASE" = "_NO_FILE_4Uhere_" ]; then EFI_CASE=dummy fi -: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_CASE"}" +: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_TESTNAME"}" mkdir -p "$EFI_CASE_DIR" -cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_TEST/$EFI_CASE/" -echo "@echo -off" > "$EFI_TEST/$EFI_CASE/startup.nsh" +cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_DIR/" +echo "@echo -off" > "$EFI_CASE_DIR/startup.nsh" if [ "$EFI_USE_DTB" = "y" ]; then qemu_args+=(-machine acpi=off) FDT_BASENAME="dtb" - $(EFI_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_TEST/$EFI_CASE/$FDT_BASENAME" "${qemu_args[@]}") - echo "setvar fdtfile -guid $EFI_VAR_GUID -rt =L\"$FDT_BASENAME\"" >> "$EFI_TEST/$EFI_CASE/startup.nsh" + $(EFI_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_CASE_DIR/$FDT_BASENAME" "${qemu_args[@]}") + echo "setvar fdtfile -guid $EFI_VAR_GUID -rt =L\"$FDT_BASENAME\"" >> "$EFI_CASE_DIR/startup.nsh" fi -echo "$EFI_CASE.efi" "${cmd_args[@]}" >> "$EFI_TEST/$EFI_CASE/startup.nsh" +echo "$EFI_CASE.efi" "${cmd_args[@]}" >> "$EFI_CASE_DIR/startup.nsh" EFI_RUN=y $TEST_DIR/run \ -bios "$EFI_UEFI" \ - -drive file.dir="$EFI_TEST/$EFI_CASE/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \ + -drive file.dir="$EFI_CASE_DIR/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \ "${qemu_args[@]}"
Currently running tests on EFI in parallel can cause part of tests to fail, this is because arm/efi/run script use the EFI_CASE to create the subdir under the efi-tests, and the EFI_CASE is the filename of the test, when running tests in parallel, the multiple tests exist in the same filename will execute at the same time, which will use the same directory and write the test specific things into it, this cause chaotic and make some tests fail. To Fix this things, use the testname instead of the filename to create the subdir under the efi-tests. We use the EFI_TESTNAME to replace the EFI_CASE in script. Since every testname is specific, now the tests can be run parallel. It also considers when user directly use the arm/efi/run to run test, in this case, still use the filename. Besides, replace multiple $EFI_TEST/$EFI_CASE to the $EFI_CASE_DIR, this makes the script looks more clean and we don'e need to replace many EFI_CASE to EFI_TESTNAME. Signed-off-by: Shaoqin Huang <shahuang@redhat.com> --- arm/efi/run | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)