| Message ID | 20240205210638.157741-16-haitao.huang@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Cgroup support for SGX EPC memory | expand |
On Mon, 2024-02-05 at 13:06 -0800, Haitao Huang wrote: > The scripts rely on cgroup-tools package from libcgroup [1]. > > To run selftests for epc cgroup: > > sudo ./run_epc_cg_selftests.sh > > To watch misc cgroup 'current' changes during testing, run this in a > separate terminal: > > ./watch_misc_for_tests.sh current > > With different cgroups, the script starts one or multiple concurrent > SGX > selftests, each to run one unclobbered_vdso_oversubscribed test. > Each > of such test tries to load an enclave of EPC size equal to the EPC > capacity available on the platform. The script checks results against > the expectation set for each cgroup and reports success or failure. > > The script creates 3 different cgroups at the beginning with > following > expectations: > > 1) SMALL - intentionally small enough to fail the test loading an > enclave of size equal to the capacity. > 2) LARGE - large enough to run up to 4 concurrent tests but fail some > if > more than 4 concurrent tests are run. The script starts 4 expecting > at > least one test to pass, and then starts 5 expecting at least one test > to fail. > 3) LARGER - limit is the same as the capacity, large enough to run > lots of > concurrent tests. The script starts 8 of them and expects all pass. > Then it reruns the same test with one process randomly killed and > usage checked to be zero after all process exit. > > The script also includes a test with low mem_cg limit and LARGE > sgx_epc > limit to verify that the RAM used for per-cgroup reclamation is > charged > to a proper mem_cg. > > [1] https://github.com/libcgroup/libcgroup/blob/main/README > > Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com> > --- > V7: > - Added memcontrol test. > > V5: > - Added script with automatic results checking, remove the > interactive > script. > - The script can run independent from the series below. > --- > .../selftests/sgx/run_epc_cg_selftests.sh | 246 > ++++++++++++++++++ > .../selftests/sgx/watch_misc_for_tests.sh | 13 + > 2 files changed, 259 insertions(+) > create mode 100755 > tools/testing/selftests/sgx/run_epc_cg_selftests.sh > create mode 100755 > tools/testing/selftests/sgx/watch_misc_for_tests.sh > > diff --git a/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > new file mode 100755 > index 000000000000..e027bf39f005 > --- /dev/null > +++ b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > @@ -0,0 +1,246 @@ > +#!/bin/bash This is not portable and neither does hold in the wild. It does not even often hold as it is not uncommon to place bash to the path /usr/bin/bash. If I recall correctly, e.g. NixOS has a path that is neither of those two. Should be #!/usr/bin/env bash That is POSIX compatible form. Just got around trying to test this in NUC7 so looking into this in more detail. That said can you make the script work with just "#!/usr/bin/env sh" and make sure that it is busybox ash compatible? I don't see any necessity to make this bash only and it adds to the compilation time of the image. Otherwise lot of this could be tested just with qemu+bzImage+busybox(inside initramfs). Now you are adding fully glibc shenanigans for the sake of syntax sugar. > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright(c) 2023 Intel Corporation. > + > +TEST_ROOT_CG=selftest > +cgcreate -g misc:$TEST_ROOT_CG How do you know that cgcreate exists? It is used a lot in the script with no check for the existence. Please fix e.g. with "command -v cgreate". > +if [ $? -ne 0 ]; then > + echo "# Please make sure cgroup-tools is installed, and misc > cgroup is mounted." > + exit 1 > +fi And please do not do it this way. Also, please remove the advice for "cgroups-tool". This is not meant to be debian only. Better would be to e.g. point out the URL of the upstream project. And yeah the whole message should be based on "command -v", not like this. > +TEST_CG_SUB1=$TEST_ROOT_CG/test1 > +TEST_CG_SUB2=$TEST_ROOT_CG/test2 > +# We will only set limit in test1 and run tests in test3 > +TEST_CG_SUB3=$TEST_ROOT_CG/test1/test3 > +TEST_CG_SUB4=$TEST_ROOT_CG/test4 > + > +cgcreate -g misc:$TEST_CG_SUB1 > +cgcreate -g misc:$TEST_CG_SUB2 > +cgcreate -g misc:$TEST_CG_SUB3 > +cgcreate -g misc:$TEST_CG_SUB4 > + > +# Default to V2 > +CG_MISC_ROOT=/sys/fs/cgroup > +CG_MEM_ROOT=/sys/fs/cgroup > +CG_V1=0 > +if [ ! -d "/sys/fs/cgroup/misc" ]; then > + echo "# cgroup V2 is in use." > +else > + echo "# cgroup V1 is in use." Is "#" prefix a standard for kselftest? I don't know this, thus asking. > + CG_MISC_ROOT=/sys/fs/cgroup/misc > + CG_MEM_ROOT=/sys/fs/cgroup/memory > + CG_V1=1 Have you checked what is the indentation policy for bash scripts inside kernel tree. I don't know what it is. That's why I'm asking. > +fi > + > +CAPACITY=$(grep "sgx_epc" "$CG_MISC_ROOT/misc.capacity" | awk > '{print $2}') > +# This is below number of VA pages needed for enclave of capacity > size. So > +# should fail oversubscribed cases > +SMALL=$(( CAPACITY / 512 )) > + > +# At least load one enclave of capacity size successfully, maybe up > to 4. > +# But some may fail if we run more than 4 concurrent enclaves of > capacity size. > +LARGE=$(( SMALL * 4 )) > + > +# Load lots of enclaves > +LARGER=$CAPACITY > +echo "# Setting up limits." > +echo "sgx_epc $SMALL" > $CG_MISC_ROOT/$TEST_CG_SUB1/misc.max > +echo "sgx_epc $LARGE" > $CG_MISC_ROOT/$TEST_CG_SUB2/misc.max > +echo "sgx_epc $LARGER" > $CG_MISC_ROOT/$TEST_CG_SUB4/misc.max > + > +timestamp=$(date +%Y%m%d_%H%M%S) > + > +test_cmd="./test_sgx -t unclobbered_vdso_oversubscribed" > + > +wait_check_process_status() { > + local pid=$1 > + local check_for_success=$2 # If 1, check for success; > + # If 0, check for failure > + wait "$pid" > + local status=$? > + > + if [[ $check_for_success -eq 1 && $status -eq 0 ]]; then > + echo "# Process $pid succeeded." > + return 0 > + elif [[ $check_for_success -eq 0 && $status -ne 0 ]]; then > + echo "# Process $pid returned failure." > + return 0 > + fi > + return 1 > +} > + > +wai > wait_and_detect_for_any() { what is "any"? Maybe for some key functions could have short documentation what they are and for what test uses them. I cannot possibly remember all of this just by hints such as "this waits for Any" ;-) I don't think there is actual kernel guideline to engineer the script to work with just ash but at least for me that would inevitably increase my motivation to test this patch set more rather than less. BR, Jarkko
On Wed Mar 27, 2024 at 2:55 PM EET, Jarkko Sakkinen wrote: > On Mon, 2024-02-05 at 13:06 -0800, Haitao Huang wrote: > > The scripts rely on cgroup-tools package from libcgroup [1]. > > > > To run selftests for epc cgroup: > > > > sudo ./run_epc_cg_selftests.sh > > > > To watch misc cgroup 'current' changes during testing, run this in a > > separate terminal: > > > > ./watch_misc_for_tests.sh current > > > > With different cgroups, the script starts one or multiple concurrent > > SGX > > selftests, each to run one unclobbered_vdso_oversubscribed test. > > Each > > of such test tries to load an enclave of EPC size equal to the EPC > > capacity available on the platform. The script checks results against > > the expectation set for each cgroup and reports success or failure. > > > > The script creates 3 different cgroups at the beginning with > > following > > expectations: > > > > 1) SMALL - intentionally small enough to fail the test loading an > > enclave of size equal to the capacity. > > 2) LARGE - large enough to run up to 4 concurrent tests but fail some > > if > > more than 4 concurrent tests are run. The script starts 4 expecting > > at > > least one test to pass, and then starts 5 expecting at least one test > > to fail. > > 3) LARGER - limit is the same as the capacity, large enough to run > > lots of > > concurrent tests. The script starts 8 of them and expects all pass. > > Then it reruns the same test with one process randomly killed and > > usage checked to be zero after all process exit. > > > > The script also includes a test with low mem_cg limit and LARGE > > sgx_epc > > limit to verify that the RAM used for per-cgroup reclamation is > > charged > > to a proper mem_cg. > > > > [1] https://github.com/libcgroup/libcgroup/blob/main/README > > > > Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com> > > --- > > V7: > > - Added memcontrol test. > > > > V5: > > - Added script with automatic results checking, remove the > > interactive > > script. > > - The script can run independent from the series below. > > --- > > .../selftests/sgx/run_epc_cg_selftests.sh | 246 > > ++++++++++++++++++ > > .../selftests/sgx/watch_misc_for_tests.sh | 13 + > > 2 files changed, 259 insertions(+) > > create mode 100755 > > tools/testing/selftests/sgx/run_epc_cg_selftests.sh > > create mode 100755 > > tools/testing/selftests/sgx/watch_misc_for_tests.sh > > > > diff --git a/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > > b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > > new file mode 100755 > > index 000000000000..e027bf39f005 > > --- /dev/null > > +++ b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > > @@ -0,0 +1,246 @@ > > +#!/bin/bash > > This is not portable and neither does hold in the wild. > > It does not even often hold as it is not uncommon to place bash > to the path /usr/bin/bash. If I recall correctly, e.g. NixOS has > a path that is neither of those two. > > Should be #!/usr/bin/env bash > > That is POSIX compatible form. > > Just got around trying to test this in NUC7 so looking into this in > more detail. > > That said can you make the script work with just "#!/usr/bin/env sh" > and make sure that it is busybox ash compatible? > > I don't see any necessity to make this bash only and it adds to the > compilation time of the image. Otherwise lot of this could be tested > just with qemu+bzImage+busybox(inside initramfs). > > Now you are adding fully glibc shenanigans for the sake of syntax > sugar. > > > +# SPDX-License-Identifier: GPL-2.0 > > +# Copyright(c) 2023 Intel Corporation. > > + > > +TEST_ROOT_CG=selftest > > +cgcreate -g misc:$TEST_ROOT_CG > > How do you know that cgcreate exists? It is used a lot in the script > with no check for the existence. Please fix e.g. with "command -v > cgreate". > > > +if [ $? -ne 0 ]; then > > + echo "# Please make sure cgroup-tools is installed, and misc > > cgroup is mounted." > > + exit 1 > > +fi > > And please do not do it this way. Also, please remove the advice for > "cgroups-tool". This is not meant to be debian only. Better would be > to e.g. point out the URL of the upstream project. > > And yeah the whole message should be based on "command -v", not like > this. > > > +TEST_CG_SUB1=$TEST_ROOT_CG/test1 > > +TEST_CG_SUB2=$TEST_ROOT_CG/test2 > > +# We will only set limit in test1 and run tests in test3 > > +TEST_CG_SUB3=$TEST_ROOT_CG/test1/test3 > > +TEST_CG_SUB4=$TEST_ROOT_CG/test4 > > + > > +cgcreate -g misc:$TEST_CG_SUB1 > > > > > +cgcreate -g misc:$TEST_CG_SUB2 > > +cgcreate -g misc:$TEST_CG_SUB3 > > +cgcreate -g misc:$TEST_CG_SUB4 > > + > > +# Default to V2 > > +CG_MISC_ROOT=/sys/fs/cgroup > > +CG_MEM_ROOT=/sys/fs/cgroup > > +CG_V1=0 > > +if [ ! -d "/sys/fs/cgroup/misc" ]; then > > + echo "# cgroup V2 is in use." > > +else > > + echo "# cgroup V1 is in use." > > Is "#" prefix a standard for kselftest? I don't know this, thus asking. > > > + CG_MISC_ROOT=/sys/fs/cgroup/misc > > + CG_MEM_ROOT=/sys/fs/cgroup/memory > > + CG_V1=1 > > Have you checked what is the indentation policy for bash scripts inside > kernel tree. I don't know what it is. That's why I'm asking. > > > +fi > > + > > +CAPACITY=$(grep "sgx_epc" "$CG_MISC_ROOT/misc.capacity" | awk > > '{print $2}') > > +# This is below number of VA pages needed for enclave of capacity > > size. So > > +# should fail oversubscribed cases > > +SMALL=$(( CAPACITY / 512 )) > > + > > +# At least load one enclave of capacity size successfully, maybe up > > to 4. > > +# But some may fail if we run more than 4 concurrent enclaves of > > capacity size. > > +LARGE=$(( SMALL * 4 )) > > + > > +# Load lots of enclaves > > +LARGER=$CAPACITY > > +echo "# Setting up limits." > > +echo "sgx_epc $SMALL" > $CG_MISC_ROOT/$TEST_CG_SUB1/misc.max > > +echo "sgx_epc $LARGE" > $CG_MISC_ROOT/$TEST_CG_SUB2/misc.max > > +echo "sgx_epc $LARGER" > $CG_MISC_ROOT/$TEST_CG_SUB4/misc.max > > + > > +timestamp=$(date +%Y%m%d_%H%M%S) > > + > > +test_cmd="./test_sgx -t unclobbered_vdso_oversubscribed" > > + > > +wait_check_process_status() { > > + local pid=$1 > > + local check_for_success=$2 # If 1, check for success; > > + # If 0, check for failure > > + wait "$pid" > > + local status=$? > > + > > + if [[ $check_for_success -eq 1 && $status -eq 0 ]]; then > > + echo "# Process $pid succeeded." > > + return 0 > > + elif [[ $check_for_success -eq 0 && $status -ne 0 ]]; then > > + echo "# Process $pid returned failure." > > + return 0 > > + fi > > + return 1 > > +} > > + > > +wai > > wait_and_detect_for_any() { > > what is "any"? > > Maybe for some key functions could have short documentation what they > are and for what test uses them. I cannot possibly remember all of this > just by hints such as "this waits for Any" ;-) > > I don't think there is actual kernel guideline to engineer the script > to work with just ash but at least for me that would inevitably > increase my motivation to test this patch set more rather than less. I also wonder is cgroup-tools dependency absolutely required or could you just have a function that would interact with sysfs? BR, Jarkko
On Wed, 27 Mar 2024 11:56:35 -0500, Jarkko Sakkinen <jarkko@kernel.org> wrote: > On Wed Mar 27, 2024 at 2:55 PM EET, Jarkko Sakkinen wrote: >> On Mon, 2024-02-05 at 13:06 -0800, Haitao Huang wrote: >> > The scripts rely on cgroup-tools package from libcgroup [1]. >> > >> > To run selftests for epc cgroup: >> > >> > sudo ./run_epc_cg_selftests.sh >> > >> > To watch misc cgroup 'current' changes during testing, run this in a >> > separate terminal: >> > >> > ./watch_misc_for_tests.sh current >> > >> > With different cgroups, the script starts one or multiple concurrent >> > SGX >> > selftests, each to run one unclobbered_vdso_oversubscribed test.> Each >> > of such test tries to load an enclave of EPC size equal to the EPC >> > capacity available on the platform. The script checks results against >> > the expectation set for each cgroup and reports success or failure. >> > >> > The script creates 3 different cgroups at the beginning with >> > following >> > expectations: >> > >> > 1) SMALL - intentionally small enough to fail the test loading an >> > enclave of size equal to the capacity. >> > 2) LARGE - large enough to run up to 4 concurrent tests but fail some >> > if >> > more than 4 concurrent tests are run. The script starts 4 expecting >> > at >> > least one test to pass, and then starts 5 expecting at least one test >> > to fail. >> > 3) LARGER - limit is the same as the capacity, large enough to run >> > lots of >> > concurrent tests. The script starts 8 of them and expects all pass. >> > Then it reruns the same test with one process randomly killed and >> > usage checked to be zero after all process exit. >> > >> > The script also includes a test with low mem_cg limit and LARGE >> > sgx_epc >> > limit to verify that the RAM used for per-cgroup reclamation is >> > charged >> > to a proper mem_cg. >> > >> > [1] https://github.com/libcgroup/libcgroup/blob/main/README >> > >> > Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com> >> > --- >> > V7: >> > - Added memcontrol test. >> > >> > V5: >> > - Added script with automatic results checking, remove the >> > interactive >> > script. >> > - The script can run independent from the series below. >> > --- >> > .../selftests/sgx/run_epc_cg_selftests.sh | 246 >> > ++++++++++++++++++ >> > .../selftests/sgx/watch_misc_for_tests.sh | 13 + >> > 2 files changed, 259 insertions(+) >> > create mode 100755 >> > tools/testing/selftests/sgx/run_epc_cg_selftests.sh >> > create mode 100755 >> > tools/testing/selftests/sgx/watch_misc_for_tests.sh >> > >> > diff --git a/tools/testing/selftests/sgx/run_epc_cg_selftests.sh >> > b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh >> > new file mode 100755 >> > index 000000000000..e027bf39f005 >> > --- /dev/null >> > +++ b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh >> > @@ -0,0 +1,246 @@ >> > +#!/bin/bash >> >> This is not portable and neither does hold in the wild. >> >> It does not even often hold as it is not uncommon to place bash >> to the path /usr/bin/bash. If I recall correctly, e.g. NixOS has >> a path that is neither of those two. >> >> Should be #!/usr/bin/env bash >> >> That is POSIX compatible form. >> >> Just got around trying to test this in NUC7 so looking into this in >> more detail. >> >> That said can you make the script work with just "#!/usr/bin/env sh" >> and make sure that it is busybox ash compatible? >> >> I don't see any necessity to make this bash only and it adds to the >> compilation time of the image. Otherwise lot of this could be tested >> just with qemu+bzImage+busybox(inside initramfs). >> >> Now you are adding fully glibc shenanigans for the sake of syntax >> sugar. >> >> > +# SPDX-License-Identifier: GPL-2.0 >> > +# Copyright(c) 2023 Intel Corporation. >> > + >> > +TEST_ROOT_CG=selftest >> > +cgcreate -g misc:$TEST_ROOT_CG >> >> How do you know that cgcreate exists? It is used a lot in the script >> with no check for the existence. Please fix e.g. with "command -v >> cgreate". >> >> > +if [ $? -ne 0 ]; then >> > + echo "# Please make sure cgroup-tools is installed, and misc >> > cgroup is mounted." >> > + exit 1 >> > +fi >> >> And please do not do it this way. Also, please remove the advice for >> "cgroups-tool". This is not meant to be debian only. Better would be >> to e.g. point out the URL of the upstream project. >> >> And yeah the whole message should be based on "command -v", not like >> this. >> >> > +TEST_CG_SUB1=$TEST_ROOT_CG/test1 >> > +TEST_CG_SUB2=$TEST_ROOT_CG/test2 >> > +# We will only set limit in test1 and run tests in test3 >> > +TEST_CG_SUB3=$TEST_ROOT_CG/test1/test3 >> > +TEST_CG_SUB4=$TEST_ROOT_CG/test4 >> > + >> > +cgcreate -g misc:$TEST_CG_SUB1 >> >> >> >> > +cgcreate -g misc:$TEST_CG_SUB2 >> > +cgcreate -g misc:$TEST_CG_SUB3 >> > +cgcreate -g misc:$TEST_CG_SUB4 >> > + >> > +# Default to V2 >> > +CG_MISC_ROOT=/sys/fs/cgroup >> > +CG_MEM_ROOT=/sys/fs/cgroup >> > +CG_V1=0 >> > +if [ ! -d "/sys/fs/cgroup/misc" ]; then >> > + echo "# cgroup V2 is in use." >> > +else >> > + echo "# cgroup V1 is in use." >> >> Is "#" prefix a standard for kselftest? I don't know this, thus asking. >> >> > + CG_MISC_ROOT=/sys/fs/cgroup/misc >> > + CG_MEM_ROOT=/sys/fs/cgroup/memory >> > + CG_V1=1 >> >> Have you checked what is the indentation policy for bash scripts inside >> kernel tree. I don't know what it is. That's why I'm asking. >> >> > +fi >> > + >> > +CAPACITY=$(grep "sgx_epc" "$CG_MISC_ROOT/misc.capacity" | awk >> > '{print $2}') >> > +# This is below number of VA pages needed for enclave of capacity >> > size. So >> > +# should fail oversubscribed cases >> > +SMALL=$(( CAPACITY / 512 )) >> > + >> > +# At least load one enclave of capacity size successfully, maybe up >> > to 4. >> > +# But some may fail if we run more than 4 concurrent enclaves of >> > capacity size. >> > +LARGE=$(( SMALL * 4 )) >> > + >> > +# Load lots of enclaves >> > +LARGER=$CAPACITY >> > +echo "# Setting up limits." >> > +echo "sgx_epc $SMALL" > $CG_MISC_ROOT/$TEST_CG_SUB1/misc.max >> > +echo "sgx_epc $LARGE" > $CG_MISC_ROOT/$TEST_CG_SUB2/misc.max >> > +echo "sgx_epc $LARGER" > $CG_MISC_ROOT/$TEST_CG_SUB4/misc.max >> > + >> > +timestamp=$(date +%Y%m%d_%H%M%S) >> > + >> > +test_cmd="./test_sgx -t unclobbered_vdso_oversubscribed" >> > + >> > +wait_check_process_status() { >> > + local pid=$1 >> > + local check_for_success=$2 # If 1, check for success; >> > + # If 0, check for failure >> > + wait "$pid" >> > + local status=$? >> > + >> > + if [[ $check_for_success -eq 1 && $status -eq 0 ]]; then >> > + echo "# Process $pid succeeded." >> > + return 0 >> > + elif [[ $check_for_success -eq 0 && $status -ne 0 ]]; then >> > + echo "# Process $pid returned failure." >> > + return 0 >> > + fi >> > + return 1 >> > +} >> > + >> > +wai >> > wait_and_detect_for_any() { >> >> what is "any"? >> >> Maybe for some key functions could have short documentation what they >> are and for what test uses them. I cannot possibly remember all of this >> just by hints such as "this waits for Any" ;-) >> >> I don't think there is actual kernel guideline to engineer the script >> to work with just ash but at least for me that would inevitably >> increase my motivation to test this patch set more rather than less. > > I also wonder is cgroup-tools dependency absolutely required or could > you just have a function that would interact with sysfs? I should have checked email before hit the send button for v10 :-). It'd be more complicated and less readable to do all the stuff without the cgroup-tools, esp cgexec. I checked dependency, cgroup-tools only depends on libc so I hope this would not cause too much inconvenience. I saw bash was also used in cgroup test scripts so at least that's consistent :-) I can look into ash if that's required. Let me know. Certainly can add more docs as you suggested. Thanks Haitao
On Wed, 27 Mar 2024 19:57:26 -0500, Haitao Huang <haitao.huang@linux.intel.com> wrote: > On Wed, 27 Mar 2024 11:56:35 -0500, Jarkko Sakkinen <jarkko@kernel.org> > wrote: > >> On Wed Mar 27, 2024 at 2:55 PM EET, Jarkko Sakkinen wrote: >>> On Mon, 2024-02-05 at 13:06 -0800, Haitao Huang wrote: >>> > The scripts rely on cgroup-tools package from libcgroup [1]. >>> > >>> > To run selftests for epc cgroup: >>> > >>> > sudo ./run_epc_cg_selftests.sh >>> > >>> > To watch misc cgroup 'current' changes during testing, run this in a >>> > separate terminal: ... >>> > wait_and_detect_for_any() { >>> >>> what is "any"? >>> >>> Maybe for some key functions could have short documentation what they >>> are and for what test uses them. I cannot possibly remember all of this >>> just by hints such as "this waits for Any" ;-) >>> >>> I don't think there is actual kernel guideline to engineer the script >>> to work with just ash but at least for me that would inevitably >>> increase my motivation to test this patch set more rather than less. >> >> I also wonder is cgroup-tools dependency absolutely required or could >> you just have a function that would interact with sysfs? > > I should have checked email before hit the send button for v10 :-). > > It'd be more complicated and less readable to do all the stuff without > the cgroup-tools, esp cgexec. I checked dependency, cgroup-tools only > depends on libc so I hope this would not cause too much inconvenience. > > I saw bash was also used in cgroup test scripts so at least that's > consistent :-) > I can look into ash if that's required. Let me know. > Sorry I missed you earlier comments. I actually tried to make it compatible with busybox on Ubuntu. I'll address your other comments and update. But meanwhile could you tryout this version in your env? https://github.com/haitaohuang/linux/commit/3c424b841cf3cf66b085a424f4b537fbc3bbff6f Thanks Haitao
On Wed, 27 Mar 2024 07:55:34 -0500, Jarkko Sakkinen <jarkko@kernel.org> wrote: > On Mon, 2024-02-05 at 13:06 -0800, Haitao Huang wrote: >> The scripts rely on cgroup-tools package from libcgroup [1]. >> >> To run selftests for epc cgroup: >> >> sudo ./run_epc_cg_selftests.sh >> >> To watch misc cgroup 'current' changes during testing, run this in a >> separate terminal: >> >> ./watch_misc_for_tests.sh current >> >> With different cgroups, the script starts one or multiple concurrent >> SGX >> selftests, each to run one unclobbered_vdso_oversubscribed test.Each >> of such test tries to load an enclave of EPC size equal to the EPC >> capacity available on the platform. The script checks results against >> the expectation set for each cgroup and reports success or failure. >> >> The script creates 3 different cgroups at the beginning with >> following >> expectations: >> >> 1) SMALL - intentionally small enough to fail the test loading an >> enclave of size equal to the capacity. >> 2) LARGE - large enough to run up to 4 concurrent tests but fail some >> if >> more than 4 concurrent tests are run. The script starts 4 expecting >> at >> least one test to pass, and then starts 5 expecting at least one test >> to fail. >> 3) LARGER - limit is the same as the capacity, large enough to run >> lots of >> concurrent tests. The script starts 8 of them and expects all pass. >> Then it reruns the same test with one process randomly killed and >> usage checked to be zero after all process exit. >> >> The script also includes a test with low mem_cg limit and LARGE >> sgx_epc >> limit to verify that the RAM used for per-cgroup reclamation is >> charged >> to a proper mem_cg. >> >> [1] https://github.com/libcgroup/libcgroup/blob/main/README >> >> Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com> >> --- >> V7: >> - Added memcontrol test. >> >> V5: >> - Added script with automatic results checking, remove the >> interactive >> script. >> - The script can run independent from the series below. >> --- >> .../selftests/sgx/run_epc_cg_selftests.sh | 246 >> ++++++++++++++++++ >> .../selftests/sgx/watch_misc_for_tests.sh | 13 + >> 2 files changed, 259 insertions(+) >> create mode 100755 >> tools/testing/selftests/sgx/run_epc_cg_selftests.sh >> create mode 100755 >> tools/testing/selftests/sgx/watch_misc_for_tests.sh >> >> diff --git a/tools/testing/selftests/sgx/run_epc_cg_selftests.sh >> b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh >> new file mode 100755 >> index 000000000000..e027bf39f005 >> --- /dev/null >> +++ b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh >> @@ -0,0 +1,246 @@ >> +#!/bin/bash > > This is not portable and neither does hold in the wild. > > It does not even often hold as it is not uncommon to place bash > to the path /usr/bin/bash. If I recall correctly, e.g. NixOS has > a path that is neither of those two. > > Should be #!/usr/bin/env bash > > That is POSIX compatible form. > Sure > Just got around trying to test this in NUC7 so looking into this in > more detail. Thanks. Could you please check if this version works for you? https://github.com/haitaohuang/linux/commit/3c424b841cf3cf66b085a424f4b537fbc3bbff6f > > That said can you make the script work with just "#!/usr/bin/env sh" > and make sure that it is busybox ash compatible? Yes. > > I don't see any necessity to make this bash only and it adds to the > compilation time of the image. Otherwise lot of this could be tested > just with qemu+bzImage+busybox(inside initramfs). > will still need cgroup-tools as you pointed out later. Compiling from its upstream code OK? > Now you are adding fully glibc shenanigans for the sake of syntax > sugar. > >> +# SPDX-License-Identifier: GPL-2.0 >> +# Copyright(c) 2023 Intel Corporation. >> + >> +TEST_ROOT_CG=selftest >> +cgcreate -g misc:$TEST_ROOT_CG > > How do you know that cgcreate exists? It is used a lot in the script > with no check for the existence. Please fix e.g. with "command -v > cgreate". > >> +if [ $? -ne 0 ]; then >> + echo "# Please make sure cgroup-tools is installed, and misc >> cgroup is mounted." >> + exit 1 >> +fi > > And please do not do it this way. Also, please remove the advice for > "cgroups-tool". This is not meant to be debian only. Better would be > to e.g. point out the URL of the upstream project. > > And yeah the whole message should be based on "command -v", not like > this. > OK >> +TEST_CG_SUB1=$TEST_ROOT_CG/test1 >> +TEST_CG_SUB2=$TEST_ROOT_CG/test2 >> +# We will only set limit in test1 and run tests in test3 >> +TEST_CG_SUB3=$TEST_ROOT_CG/test1/test3 >> +TEST_CG_SUB4=$TEST_ROOT_CG/test4 >> + >> +cgcreate -g misc:$TEST_CG_SUB1 > > > >> +cgcreate -g misc:$TEST_CG_SUB2 >> +cgcreate -g misc:$TEST_CG_SUB3 >> +cgcreate -g misc:$TEST_CG_SUB4 >> + >> +# Default to V2 >> +CG_MISC_ROOT=/sys/fs/cgroup >> +CG_MEM_ROOT=/sys/fs/cgroup >> +CG_V1=0 >> +if [ ! -d "/sys/fs/cgroup/misc" ]; then >> + echo "# cgroup V2 is in use." >> +else >> + echo "# cgroup V1 is in use." > > Is "#" prefix a standard for kselftest? I don't know this, thus asking. > >> + CG_MISC_ROOT=/sys/fs/cgroup/misc >> + CG_MEM_ROOT=/sys/fs/cgroup/memory >> + CG_V1=1 > > Have you checked what is the indentation policy for bash scripts inside > kernel tree. I don't know what it is. That's why I'm asking. > Right. I looked around and found scripts using bash in cgroup selftests (at least it marked with "#!/bin/bash"). And that's why I used it initially. I don't see any specific rule for testing scripts after searching through the documentation. I do see bash is one of minimal requirement for compiling kernel: https://docs.kernel.org/process/changes.html?highlight=bash Anyway, I think we can make it compatible with busybox if needed. >> +fi >> + >> +CAPACITY=$(grep "sgx_epc" "$CG_MISC_ROOT/misc.capacity" | awk >> '{print $2}') >> +# This is below number of VA pages needed for enclave of capacity >> size. So >> +# should fail oversubscribed cases >> +SMALL=$(( CAPACITY / 512 )) >> + >> +# At least load one enclave of capacity size successfully, maybe up >> to 4. >> +# But some may fail if we run more than 4 concurrent enclaves of >> capacity size. >> +LARGE=$(( SMALL * 4 )) >> + >> +# Load lots of enclaves >> +LARGER=$CAPACITY >> +echo "# Setting up limits." >> +echo "sgx_epc $SMALL" > $CG_MISC_ROOT/$TEST_CG_SUB1/misc.max >> +echo "sgx_epc $LARGE" > $CG_MISC_ROOT/$TEST_CG_SUB2/misc.max >> +echo "sgx_epc $LARGER" > $CG_MISC_ROOT/$TEST_CG_SUB4/misc.max >> + >> +timestamp=$(date +%Y%m%d_%H%M%S) >> + >> +test_cmd="./test_sgx -t unclobbered_vdso_oversubscribed" >> + >> +wait_check_process_status() { >> + local pid=$1 >> + local check_for_success=$2 # If 1, check for success; >> + # If 0, check for failure >> + wait "$pid" >> + local status=$? >> + >> + if [[ $check_for_success -eq 1 && $status -eq 0 ]]; then >> + echo "# Process $pid succeeded." >> + return 0 >> + elif [[ $check_for_success -eq 0 && $status -ne 0 ]]; then >> + echo "# Process $pid returned failure." >> + return 0 >> + fi >> + return 1 >> +} >> + >> +wai >> wait_and_detect_for_any() { > > what is "any"? > > Maybe for some key functions could have short documentation what they > are and for what test uses them. I cannot possibly remember all of this > just by hints such as "this waits for Any" ;-) > Will add comments > I don't think there is actual kernel guideline to engineer the script > to work with just ash but at least for me that would inevitably > increase my motivation to test this patch set more rather than less . Ok Thanks Haitao
On Thu Mar 28, 2024 at 5:54 AM EET, Haitao Huang wrote: > On Wed, 27 Mar 2024 07:55:34 -0500, Jarkko Sakkinen <jarkko@kernel.org> > wrote: > > > On Mon, 2024-02-05 at 13:06 -0800, Haitao Huang wrote: > >> The scripts rely on cgroup-tools package from libcgroup [1]. > >> > >> To run selftests for epc cgroup: > >> > >> sudo ./run_epc_cg_selftests.sh > >> > >> To watch misc cgroup 'current' changes during testing, run this in a > >> separate terminal: > >> > >> ./watch_misc_for_tests.sh current > >> > >> With different cgroups, the script starts one or multiple concurrent > >> SGX > >> selftests, each to run one unclobbered_vdso_oversubscribed test.Each > >> of such test tries to load an enclave of EPC size equal to the EPC > >> capacity available on the platform. The script checks results against > >> the expectation set for each cgroup and reports success or failure. > >> > >> The script creates 3 different cgroups at the beginning with > >> following > >> expectations: > >> > >> 1) SMALL - intentionally small enough to fail the test loading an > >> enclave of size equal to the capacity. > >> 2) LARGE - large enough to run up to 4 concurrent tests but fail some > >> if > >> more than 4 concurrent tests are run. The script starts 4 expecting > >> at > >> least one test to pass, and then starts 5 expecting at least one test > >> to fail. > >> 3) LARGER - limit is the same as the capacity, large enough to run > >> lots of > >> concurrent tests. The script starts 8 of them and expects all pass. > >> Then it reruns the same test with one process randomly killed and > >> usage checked to be zero after all process exit. > >> > >> The script also includes a test with low mem_cg limit and LARGE > >> sgx_epc > >> limit to verify that the RAM used for per-cgroup reclamation is > >> charged > >> to a proper mem_cg. > >> > >> [1] https://github.com/libcgroup/libcgroup/blob/main/README > >> > >> Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com> > >> --- > >> V7: > >> - Added memcontrol test. > >> > >> V5: > >> - Added script with automatic results checking, remove the > >> interactive > >> script. > >> - The script can run independent from the series below. > >> --- > >> .../selftests/sgx/run_epc_cg_selftests.sh | 246 > >> ++++++++++++++++++ > >> .../selftests/sgx/watch_misc_for_tests.sh | 13 + > >> 2 files changed, 259 insertions(+) > >> create mode 100755 > >> tools/testing/selftests/sgx/run_epc_cg_selftests.sh > >> create mode 100755 > >> tools/testing/selftests/sgx/watch_misc_for_tests.sh > >> > >> diff --git a/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > >> b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > >> new file mode 100755 > >> index 000000000000..e027bf39f005 > >> --- /dev/null > >> +++ b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > >> @@ -0,0 +1,246 @@ > >> +#!/bin/bash > > > > This is not portable and neither does hold in the wild. > > > > It does not even often hold as it is not uncommon to place bash > > to the path /usr/bin/bash. If I recall correctly, e.g. NixOS has > > a path that is neither of those two. > > > > Should be #!/usr/bin/env bash > > > > That is POSIX compatible form. > > > > Sure > > > Just got around trying to test this in NUC7 so looking into this in > > more detail. > > Thanks. Could you please check if this version works for you? > > https://github.com/haitaohuang/linux/commit/3c424b841cf3cf66b085a424f4b537fbc3bbff6f > > > > > That said can you make the script work with just "#!/usr/bin/env sh" > > and make sure that it is busybox ash compatible? > > Yes. > > > > > I don't see any necessity to make this bash only and it adds to the > > compilation time of the image. Otherwise lot of this could be tested > > just with qemu+bzImage+busybox(inside initramfs). > > > > will still need cgroup-tools as you pointed out later. Compiling from its > upstream code OK? Can you explain why you need it? What is the thing you cannot do without it? BR, Jarkko
On Thu Mar 28, 2024 at 2:57 AM EET, Haitao Huang wrote: > On Wed, 27 Mar 2024 11:56:35 -0500, Jarkko Sakkinen <jarkko@kernel.org> > wrote: > > > On Wed Mar 27, 2024 at 2:55 PM EET, Jarkko Sakkinen wrote: > >> On Mon, 2024-02-05 at 13:06 -0800, Haitao Huang wrote: > >> > The scripts rely on cgroup-tools package from libcgroup [1]. > >> > > >> > To run selftests for epc cgroup: > >> > > >> > sudo ./run_epc_cg_selftests.sh > >> > > >> > To watch misc cgroup 'current' changes during testing, run this in a > >> > separate terminal: > >> > > >> > ./watch_misc_for_tests.sh current > >> > > >> > With different cgroups, the script starts one or multiple concurrent > >> > SGX > >> > selftests, each to run one unclobbered_vdso_oversubscribed test.> Each > >> > of such test tries to load an enclave of EPC size equal to the EPC > >> > capacity available on the platform. The script checks results against > >> > the expectation set for each cgroup and reports success or failure. > >> > > >> > The script creates 3 different cgroups at the beginning with > >> > following > >> > expectations: > >> > > >> > 1) SMALL - intentionally small enough to fail the test loading an > >> > enclave of size equal to the capacity. > >> > 2) LARGE - large enough to run up to 4 concurrent tests but fail some > >> > if > >> > more than 4 concurrent tests are run. The script starts 4 expecting > >> > at > >> > least one test to pass, and then starts 5 expecting at least one test > >> > to fail. > >> > 3) LARGER - limit is the same as the capacity, large enough to run > >> > lots of > >> > concurrent tests. The script starts 8 of them and expects all pass. > >> > Then it reruns the same test with one process randomly killed and > >> > usage checked to be zero after all process exit. > >> > > >> > The script also includes a test with low mem_cg limit and LARGE > >> > sgx_epc > >> > limit to verify that the RAM used for per-cgroup reclamation is > >> > charged > >> > to a proper mem_cg. > >> > > >> > [1] https://github.com/libcgroup/libcgroup/blob/main/README > >> > > >> > Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com> > >> > --- > >> > V7: > >> > - Added memcontrol test. > >> > > >> > V5: > >> > - Added script with automatic results checking, remove the > >> > interactive > >> > script. > >> > - The script can run independent from the series below. > >> > --- > >> > .../selftests/sgx/run_epc_cg_selftests.sh | 246 > >> > ++++++++++++++++++ > >> > .../selftests/sgx/watch_misc_for_tests.sh | 13 + > >> > 2 files changed, 259 insertions(+) > >> > create mode 100755 > >> > tools/testing/selftests/sgx/run_epc_cg_selftests.sh > >> > create mode 100755 > >> > tools/testing/selftests/sgx/watch_misc_for_tests.sh > >> > > >> > diff --git a/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > >> > b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > >> > new file mode 100755 > >> > index 000000000000..e027bf39f005 > >> > --- /dev/null > >> > +++ b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > >> > @@ -0,0 +1,246 @@ > >> > +#!/bin/bash > >> > >> This is not portable and neither does hold in the wild. > >> > >> It does not even often hold as it is not uncommon to place bash > >> to the path /usr/bin/bash. If I recall correctly, e.g. NixOS has > >> a path that is neither of those two. > >> > >> Should be #!/usr/bin/env bash > >> > >> That is POSIX compatible form. > >> > >> Just got around trying to test this in NUC7 so looking into this in > >> more detail. > >> > >> That said can you make the script work with just "#!/usr/bin/env sh" > >> and make sure that it is busybox ash compatible? > >> > >> I don't see any necessity to make this bash only and it adds to the > >> compilation time of the image. Otherwise lot of this could be tested > >> just with qemu+bzImage+busybox(inside initramfs). > >> > >> Now you are adding fully glibc shenanigans for the sake of syntax > >> sugar. > >> > >> > +# SPDX-License-Identifier: GPL-2.0 > >> > +# Copyright(c) 2023 Intel Corporation. > >> > + > >> > +TEST_ROOT_CG=selftest > >> > +cgcreate -g misc:$TEST_ROOT_CG > >> > >> How do you know that cgcreate exists? It is used a lot in the script > >> with no check for the existence. Please fix e.g. with "command -v > >> cgreate". > >> > >> > +if [ $? -ne 0 ]; then > >> > + echo "# Please make sure cgroup-tools is installed, and misc > >> > cgroup is mounted." > >> > + exit 1 > >> > +fi > >> > >> And please do not do it this way. Also, please remove the advice for > >> "cgroups-tool". This is not meant to be debian only. Better would be > >> to e.g. point out the URL of the upstream project. > >> > >> And yeah the whole message should be based on "command -v", not like > >> this. > >> > >> > +TEST_CG_SUB1=$TEST_ROOT_CG/test1 > >> > +TEST_CG_SUB2=$TEST_ROOT_CG/test2 > >> > +# We will only set limit in test1 and run tests in test3 > >> > +TEST_CG_SUB3=$TEST_ROOT_CG/test1/test3 > >> > +TEST_CG_SUB4=$TEST_ROOT_CG/test4 > >> > + > >> > +cgcreate -g misc:$TEST_CG_SUB1 > >> > >> > >> > >> > +cgcreate -g misc:$TEST_CG_SUB2 > >> > +cgcreate -g misc:$TEST_CG_SUB3 > >> > +cgcreate -g misc:$TEST_CG_SUB4 > >> > + > >> > +# Default to V2 > >> > +CG_MISC_ROOT=/sys/fs/cgroup > >> > +CG_MEM_ROOT=/sys/fs/cgroup > >> > +CG_V1=0 > >> > +if [ ! -d "/sys/fs/cgroup/misc" ]; then > >> > + echo "# cgroup V2 is in use." > >> > +else > >> > + echo "# cgroup V1 is in use." > >> > >> Is "#" prefix a standard for kselftest? I don't know this, thus asking. > >> > >> > + CG_MISC_ROOT=/sys/fs/cgroup/misc > >> > + CG_MEM_ROOT=/sys/fs/cgroup/memory > >> > + CG_V1=1 > >> > >> Have you checked what is the indentation policy for bash scripts inside > >> kernel tree. I don't know what it is. That's why I'm asking. > >> > >> > +fi > >> > + > >> > +CAPACITY=$(grep "sgx_epc" "$CG_MISC_ROOT/misc.capacity" | awk > >> > '{print $2}') > >> > +# This is below number of VA pages needed for enclave of capacity > >> > size. So > >> > +# should fail oversubscribed cases > >> > +SMALL=$(( CAPACITY / 512 )) > >> > + > >> > +# At least load one enclave of capacity size successfully, maybe up > >> > to 4. > >> > +# But some may fail if we run more than 4 concurrent enclaves of > >> > capacity size. > >> > +LARGE=$(( SMALL * 4 )) > >> > + > >> > +# Load lots of enclaves > >> > +LARGER=$CAPACITY > >> > +echo "# Setting up limits." > >> > +echo "sgx_epc $SMALL" > $CG_MISC_ROOT/$TEST_CG_SUB1/misc.max > >> > +echo "sgx_epc $LARGE" > $CG_MISC_ROOT/$TEST_CG_SUB2/misc.max > >> > +echo "sgx_epc $LARGER" > $CG_MISC_ROOT/$TEST_CG_SUB4/misc.max > >> > + > >> > +timestamp=$(date +%Y%m%d_%H%M%S) > >> > + > >> > +test_cmd="./test_sgx -t unclobbered_vdso_oversubscribed" > >> > + > >> > +wait_check_process_status() { > >> > + local pid=$1 > >> > + local check_for_success=$2 # If 1, check for success; > >> > + # If 0, check for failure > >> > + wait "$pid" > >> > + local status=$? > >> > + > >> > + if [[ $check_for_success -eq 1 && $status -eq 0 ]]; then > >> > + echo "# Process $pid succeeded." > >> > + return 0 > >> > + elif [[ $check_for_success -eq 0 && $status -ne 0 ]]; then > >> > + echo "# Process $pid returned failure." > >> > + return 0 > >> > + fi > >> > + return 1 > >> > +} > >> > + > >> > +wai > >> > wait_and_detect_for_any() { > >> > >> what is "any"? > >> > >> Maybe for some key functions could have short documentation what they > >> are and for what test uses them. I cannot possibly remember all of this > >> just by hints such as "this waits for Any" ;-) > >> > >> I don't think there is actual kernel guideline to engineer the script > >> to work with just ash but at least for me that would inevitably > >> increase my motivation to test this patch set more rather than less. > > > > I also wonder is cgroup-tools dependency absolutely required or could > > you just have a function that would interact with sysfs? > > I should have checked email before hit the send button for v10 :-). > > It'd be more complicated and less readable to do all the stuff without the > cgroup-tools, esp cgexec. I checked dependency, cgroup-tools only depends > on libc so I hope this would not cause too much inconvenience. As per cgroup-tools, please prove this. It makes the job for more complicated *for you* and you are making the job more complicated to every possible person in the planet running any kernel QA. I weight the latter more than the former. And it is exactly the reason why we did custom user space kselftest in the first place. Let's keep the tradition. All I can say is that kselftest is unfinished in its current form. What is "esp cgexec"? BR, Jarkko
On Sat Mar 30, 2024 at 1:23 PM EET, Jarkko Sakkinen wrote: > On Thu Mar 28, 2024 at 2:57 AM EET, Haitao Huang wrote: > > On Wed, 27 Mar 2024 11:56:35 -0500, Jarkko Sakkinen <jarkko@kernel.org> > > wrote: > > > > > On Wed Mar 27, 2024 at 2:55 PM EET, Jarkko Sakkinen wrote: > > >> On Mon, 2024-02-05 at 13:06 -0800, Haitao Huang wrote: > > >> > The scripts rely on cgroup-tools package from libcgroup [1]. > > >> > > > >> > To run selftests for epc cgroup: > > >> > > > >> > sudo ./run_epc_cg_selftests.sh > > >> > > > >> > To watch misc cgroup 'current' changes during testing, run this in a > > >> > separate terminal: > > >> > > > >> > ./watch_misc_for_tests.sh current > > >> > > > >> > With different cgroups, the script starts one or multiple concurrent > > >> > SGX > > >> > selftests, each to run one unclobbered_vdso_oversubscribed test.> Each > > >> > of such test tries to load an enclave of EPC size equal to the EPC > > >> > capacity available on the platform. The script checks results against > > >> > the expectation set for each cgroup and reports success or failure. > > >> > > > >> > The script creates 3 different cgroups at the beginning with > > >> > following > > >> > expectations: > > >> > > > >> > 1) SMALL - intentionally small enough to fail the test loading an > > >> > enclave of size equal to the capacity. > > >> > 2) LARGE - large enough to run up to 4 concurrent tests but fail some > > >> > if > > >> > more than 4 concurrent tests are run. The script starts 4 expecting > > >> > at > > >> > least one test to pass, and then starts 5 expecting at least one test > > >> > to fail. > > >> > 3) LARGER - limit is the same as the capacity, large enough to run > > >> > lots of > > >> > concurrent tests. The script starts 8 of them and expects all pass. > > >> > Then it reruns the same test with one process randomly killed and > > >> > usage checked to be zero after all process exit. > > >> > > > >> > The script also includes a test with low mem_cg limit and LARGE > > >> > sgx_epc > > >> > limit to verify that the RAM used for per-cgroup reclamation is > > >> > charged > > >> > to a proper mem_cg. > > >> > > > >> > [1] https://github.com/libcgroup/libcgroup/blob/main/README > > >> > > > >> > Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com> > > >> > --- > > >> > V7: > > >> > - Added memcontrol test. > > >> > > > >> > V5: > > >> > - Added script with automatic results checking, remove the > > >> > interactive > > >> > script. > > >> > - The script can run independent from the series below. > > >> > --- > > >> > .../selftests/sgx/run_epc_cg_selftests.sh | 246 > > >> > ++++++++++++++++++ > > >> > .../selftests/sgx/watch_misc_for_tests.sh | 13 + > > >> > 2 files changed, 259 insertions(+) > > >> > create mode 100755 > > >> > tools/testing/selftests/sgx/run_epc_cg_selftests.sh > > >> > create mode 100755 > > >> > tools/testing/selftests/sgx/watch_misc_for_tests.sh > > >> > > > >> > diff --git a/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > > >> > b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > > >> > new file mode 100755 > > >> > index 000000000000..e027bf39f005 > > >> > --- /dev/null > > >> > +++ b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > > >> > @@ -0,0 +1,246 @@ > > >> > +#!/bin/bash > > >> > > >> This is not portable and neither does hold in the wild. > > >> > > >> It does not even often hold as it is not uncommon to place bash > > >> to the path /usr/bin/bash. If I recall correctly, e.g. NixOS has > > >> a path that is neither of those two. > > >> > > >> Should be #!/usr/bin/env bash > > >> > > >> That is POSIX compatible form. > > >> > > >> Just got around trying to test this in NUC7 so looking into this in > > >> more detail. > > >> > > >> That said can you make the script work with just "#!/usr/bin/env sh" > > >> and make sure that it is busybox ash compatible? > > >> > > >> I don't see any necessity to make this bash only and it adds to the > > >> compilation time of the image. Otherwise lot of this could be tested > > >> just with qemu+bzImage+busybox(inside initramfs). > > >> > > >> Now you are adding fully glibc shenanigans for the sake of syntax > > >> sugar. > > >> > > >> > +# SPDX-License-Identifier: GPL-2.0 > > >> > +# Copyright(c) 2023 Intel Corporation. > > >> > + > > >> > +TEST_ROOT_CG=selftest > > >> > +cgcreate -g misc:$TEST_ROOT_CG > > >> > > >> How do you know that cgcreate exists? It is used a lot in the script > > >> with no check for the existence. Please fix e.g. with "command -v > > >> cgreate". > > >> > > >> > +if [ $? -ne 0 ]; then > > >> > + echo "# Please make sure cgroup-tools is installed, and misc > > >> > cgroup is mounted." > > >> > + exit 1 > > >> > +fi > > >> > > >> And please do not do it this way. Also, please remove the advice for > > >> "cgroups-tool". This is not meant to be debian only. Better would be > > >> to e.g. point out the URL of the upstream project. > > >> > > >> And yeah the whole message should be based on "command -v", not like > > >> this. > > >> > > >> > +TEST_CG_SUB1=$TEST_ROOT_CG/test1 > > >> > +TEST_CG_SUB2=$TEST_ROOT_CG/test2 > > >> > +# We will only set limit in test1 and run tests in test3 > > >> > +TEST_CG_SUB3=$TEST_ROOT_CG/test1/test3 > > >> > +TEST_CG_SUB4=$TEST_ROOT_CG/test4 > > >> > + > > >> > +cgcreate -g misc:$TEST_CG_SUB1 > > >> > > >> > > >> > > >> > +cgcreate -g misc:$TEST_CG_SUB2 > > >> > +cgcreate -g misc:$TEST_CG_SUB3 > > >> > +cgcreate -g misc:$TEST_CG_SUB4 > > >> > + > > >> > +# Default to V2 > > >> > +CG_MISC_ROOT=/sys/fs/cgroup > > >> > +CG_MEM_ROOT=/sys/fs/cgroup > > >> > +CG_V1=0 > > >> > +if [ ! -d "/sys/fs/cgroup/misc" ]; then > > >> > + echo "# cgroup V2 is in use." > > >> > +else > > >> > + echo "# cgroup V1 is in use." > > >> > > >> Is "#" prefix a standard for kselftest? I don't know this, thus asking. > > >> > > >> > + CG_MISC_ROOT=/sys/fs/cgroup/misc > > >> > + CG_MEM_ROOT=/sys/fs/cgroup/memory > > >> > + CG_V1=1 > > >> > > >> Have you checked what is the indentation policy for bash scripts inside > > >> kernel tree. I don't know what it is. That's why I'm asking. > > >> > > >> > +fi > > >> > + > > >> > +CAPACITY=$(grep "sgx_epc" "$CG_MISC_ROOT/misc.capacity" | awk > > >> > '{print $2}') > > >> > +# This is below number of VA pages needed for enclave of capacity > > >> > size. So > > >> > +# should fail oversubscribed cases > > >> > +SMALL=$(( CAPACITY / 512 )) > > >> > + > > >> > +# At least load one enclave of capacity size successfully, maybe up > > >> > to 4. > > >> > +# But some may fail if we run more than 4 concurrent enclaves of > > >> > capacity size. > > >> > +LARGE=$(( SMALL * 4 )) > > >> > + > > >> > +# Load lots of enclaves > > >> > +LARGER=$CAPACITY > > >> > +echo "# Setting up limits." > > >> > +echo "sgx_epc $SMALL" > $CG_MISC_ROOT/$TEST_CG_SUB1/misc.max > > >> > +echo "sgx_epc $LARGE" > $CG_MISC_ROOT/$TEST_CG_SUB2/misc.max > > >> > +echo "sgx_epc $LARGER" > $CG_MISC_ROOT/$TEST_CG_SUB4/misc.max > > >> > + > > >> > +timestamp=$(date +%Y%m%d_%H%M%S) > > >> > + > > >> > +test_cmd="./test_sgx -t unclobbered_vdso_oversubscribed" > > >> > + > > >> > +wait_check_process_status() { > > >> > + local pid=$1 > > >> > + local check_for_success=$2 # If 1, check for success; > > >> > + # If 0, check for failure > > >> > + wait "$pid" > > >> > + local status=$? > > >> > + > > >> > + if [[ $check_for_success -eq 1 && $status -eq 0 ]]; then > > >> > + echo "# Process $pid succeeded." > > >> > + return 0 > > >> > + elif [[ $check_for_success -eq 0 && $status -ne 0 ]]; then > > >> > + echo "# Process $pid returned failure." > > >> > + return 0 > > >> > + fi > > >> > + return 1 > > >> > +} > > >> > + > > >> > +wai > > >> > wait_and_detect_for_any() { > > >> > > >> what is "any"? > > >> > > >> Maybe for some key functions could have short documentation what they > > >> are and for what test uses them. I cannot possibly remember all of this > > >> just by hints such as "this waits for Any" ;-) > > >> > > >> I don't think there is actual kernel guideline to engineer the script > > >> to work with just ash but at least for me that would inevitably > > >> increase my motivation to test this patch set more rather than less. > > > > > > I also wonder is cgroup-tools dependency absolutely required or could > > > you just have a function that would interact with sysfs? > > > > I should have checked email before hit the send button for v10 :-). > > > > It'd be more complicated and less readable to do all the stuff without the > > cgroup-tools, esp cgexec. I checked dependency, cgroup-tools only depends > > on libc so I hope this would not cause too much inconvenience. > > As per cgroup-tools, please prove this. It makes the job for more > complicated *for you* and you are making the job more complicated > to every possible person in the planet running any kernel QA. > > I weight the latter more than the former. And it is exactly the > reason why we did custom user space kselftest in the first place. > Let's keep the tradition. All I can say is that kselftest is > unfinished in its current form. > > What is "esp cgexec"? Also in kselftest we don't drive ultimate simplicity, we drive efficient CI/QA. By open coding something like subset of cgroup-tools needed to run the test you also help us later on to backtrack the kernel changes. With cgroups-tools you would have to use strace to get the same info. BR, Jarkko
On Sat, 30 Mar 2024 06:15:14 -0500, Jarkko Sakkinen <jarkko@kernel.org> wrote: > On Thu Mar 28, 2024 at 5:54 AM EET, Haitao Huang wrote: >> On Wed, 27 Mar 2024 07:55:34 -0500, Jarkko Sakkinen <jarkko@kernel.org> >> wrote: >> >> > On Mon, 2024-02-05 at 13:06 -0800, Haitao Huang wrote: >> >> The scripts rely on cgroup-tools package from libcgroup [1]. >> >> >> >> To run selftests for epc cgroup: >> >> >> >> sudo ./run_epc_cg_selftests.sh >> >> >> >> To watch misc cgroup 'current' changes during testing, run this in a >> >> separate terminal: >> >> >> >> ./watch_misc_for_tests.sh current >> >> >> >> With different cgroups, the script starts one or multiple concurrent >> >> SGX >> >> selftests, each to run one unclobbered_vdso_oversubscribed test.Each >> >> of such test tries to load an enclave of EPC size equal to the EPC >> >> capacity available on the platform. The script checks results against >> >> the expectation set for each cgroup and reports success or failure. >> >> >> >> The script creates 3 different cgroups at the beginning with >> >> following >> >> expectations: >> >> >> >> 1) SMALL - intentionally small enough to fail the test loading an >> >> enclave of size equal to the capacity. >> >> 2) LARGE - large enough to run up to 4 concurrent tests but fail some >> >> if >> >> more than 4 concurrent tests are run. The script starts 4 expecting >> >> at >> >> least one test to pass, and then starts 5 expecting at least one test >> >> to fail. >> >> 3) LARGER - limit is the same as the capacity, large enough to run >> >> lots of >> >> concurrent tests. The script starts 8 of them and expects all pass. >> >> Then it reruns the same test with one process randomly killed and >> >> usage checked to be zero after all process exit. >> >> >> >> The script also includes a test with low mem_cg limit and LARGE >> >> sgx_epc >> >> limit to verify that the RAM used for per-cgroup reclamation is >> >> charged >> >> to a proper mem_cg. >> >> >> >> [1] https://github.com/libcgroup/libcgroup/blob/main/README >> >> >> >> Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com> >> >> --- >> >> V7: >> >> - Added memcontrol test. >> >> >> >> V5: >> >> - Added script with automatic results checking, remove the >> >> interactive >> >> script. >> >> - The script can run independent from the series below. >> >> --- >> >> .../selftests/sgx/run_epc_cg_selftests.sh | 246 >> >> ++++++++++++++++++ >> >> .../selftests/sgx/watch_misc_for_tests.sh | 13 + >> >> 2 files changed, 259 insertions(+) >> >> create mode 100755 >> >> tools/testing/selftests/sgx/run_epc_cg_selftests.sh >> >> create mode 100755 >> >> tools/testing/selftests/sgx/watch_misc_for_tests.sh >> >> >> >> diff --git a/tools/testing/selftests/sgx/run_epc_cg_selftests.sh >> >> b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh >> >> new file mode 100755 >> >> index 000000000000..e027bf39f005 >> >> --- /dev/null >> >> +++ b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh >> >> @@ -0,0 +1,246 @@ >> >> +#!/bin/bash >> > >> > This is not portable and neither does hold in the wild. >> > >> > It does not even often hold as it is not uncommon to place bash >> > to the path /usr/bin/bash. If I recall correctly, e.g. NixOS has >> > a path that is neither of those two. >> > >> > Should be #!/usr/bin/env bash >> > >> > That is POSIX compatible form. >> > >> >> Sure >> >> > Just got around trying to test this in NUC7 so looking into this in >> > more detail. >> >> Thanks. Could you please check if this version works for you? >> >> https://github.com/haitaohuang/linux/commit/3c424b841cf3cf66b085a424f4b537fbc3bbff6f >> >> > >> > That said can you make the script work with just "#!/usr/bin/env sh" >> > and make sure that it is busybox ash compatible? >> >> Yes. >> >> > >> > I don't see any necessity to make this bash only and it adds to the >> > compilation time of the image. Otherwise lot of this could be tested >> > just with qemu+bzImage+busybox(inside initramfs). >> > >> >> will still need cgroup-tools as you pointed out later. Compiling from >> its >> upstream code OK? > > Can you explain why you need it? > > What is the thing you cannot do without it? > > BR, Jarkko > I did not find a nice way to start a process in a specified cgroup like cgexec does. I could wrap the test in a shell: move the current shell to a new cgroup then do exec to run the test app. But that seems cumbersome as I need to spawn many shells, and check results of them. Another option is create my own cgexec, which I'm sure will be very similar to cgexec code. Was not sure how far we want to go with this. I can experiment with the shell wrapper idea and see how bad it can be and fall back to implement cgexec? Open to suggestions. Thanks Haitao
On Sat Mar 30, 2024 at 5:32 PM EET, Haitao Huang wrote: > On Sat, 30 Mar 2024 06:15:14 -0500, Jarkko Sakkinen <jarkko@kernel.org> > wrote: > > > On Thu Mar 28, 2024 at 5:54 AM EET, Haitao Huang wrote: > >> On Wed, 27 Mar 2024 07:55:34 -0500, Jarkko Sakkinen <jarkko@kernel.org> > >> wrote: > >> > >> > On Mon, 2024-02-05 at 13:06 -0800, Haitao Huang wrote: > >> >> The scripts rely on cgroup-tools package from libcgroup [1]. > >> >> > >> >> To run selftests for epc cgroup: > >> >> > >> >> sudo ./run_epc_cg_selftests.sh > >> >> > >> >> To watch misc cgroup 'current' changes during testing, run this in a > >> >> separate terminal: > >> >> > >> >> ./watch_misc_for_tests.sh current > >> >> > >> >> With different cgroups, the script starts one or multiple concurrent > >> >> SGX > >> >> selftests, each to run one unclobbered_vdso_oversubscribed test.Each > >> >> of such test tries to load an enclave of EPC size equal to the EPC > >> >> capacity available on the platform. The script checks results against > >> >> the expectation set for each cgroup and reports success or failure. > >> >> > >> >> The script creates 3 different cgroups at the beginning with > >> >> following > >> >> expectations: > >> >> > >> >> 1) SMALL - intentionally small enough to fail the test loading an > >> >> enclave of size equal to the capacity. > >> >> 2) LARGE - large enough to run up to 4 concurrent tests but fail some > >> >> if > >> >> more than 4 concurrent tests are run. The script starts 4 expecting > >> >> at > >> >> least one test to pass, and then starts 5 expecting at least one test > >> >> to fail. > >> >> 3) LARGER - limit is the same as the capacity, large enough to run > >> >> lots of > >> >> concurrent tests. The script starts 8 of them and expects all pass. > >> >> Then it reruns the same test with one process randomly killed and > >> >> usage checked to be zero after all process exit. > >> >> > >> >> The script also includes a test with low mem_cg limit and LARGE > >> >> sgx_epc > >> >> limit to verify that the RAM used for per-cgroup reclamation is > >> >> charged > >> >> to a proper mem_cg. > >> >> > >> >> [1] https://github.com/libcgroup/libcgroup/blob/main/README > >> >> > >> >> Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com> > >> >> --- > >> >> V7: > >> >> - Added memcontrol test. > >> >> > >> >> V5: > >> >> - Added script with automatic results checking, remove the > >> >> interactive > >> >> script. > >> >> - The script can run independent from the series below. > >> >> --- > >> >> .../selftests/sgx/run_epc_cg_selftests.sh | 246 > >> >> ++++++++++++++++++ > >> >> .../selftests/sgx/watch_misc_for_tests.sh | 13 + > >> >> 2 files changed, 259 insertions(+) > >> >> create mode 100755 > >> >> tools/testing/selftests/sgx/run_epc_cg_selftests.sh > >> >> create mode 100755 > >> >> tools/testing/selftests/sgx/watch_misc_for_tests.sh > >> >> > >> >> diff --git a/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > >> >> b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > >> >> new file mode 100755 > >> >> index 000000000000..e027bf39f005 > >> >> --- /dev/null > >> >> +++ b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > >> >> @@ -0,0 +1,246 @@ > >> >> +#!/bin/bash > >> > > >> > This is not portable and neither does hold in the wild. > >> > > >> > It does not even often hold as it is not uncommon to place bash > >> > to the path /usr/bin/bash. If I recall correctly, e.g. NixOS has > >> > a path that is neither of those two. > >> > > >> > Should be #!/usr/bin/env bash > >> > > >> > That is POSIX compatible form. > >> > > >> > >> Sure > >> > >> > Just got around trying to test this in NUC7 so looking into this in > >> > more detail. > >> > >> Thanks. Could you please check if this version works for you? > >> > >> https://github.com/haitaohuang/linux/commit/3c424b841cf3cf66b085a424f4b537fbc3bbff6f > >> > >> > > >> > That said can you make the script work with just "#!/usr/bin/env sh" > >> > and make sure that it is busybox ash compatible? > >> > >> Yes. > >> > >> > > >> > I don't see any necessity to make this bash only and it adds to the > >> > compilation time of the image. Otherwise lot of this could be tested > >> > just with qemu+bzImage+busybox(inside initramfs). > >> > > >> > >> will still need cgroup-tools as you pointed out later. Compiling from > >> its > >> upstream code OK? > > > > Can you explain why you need it? > > > > What is the thing you cannot do without it? > > > > BR, Jarkko > > > I did not find a nice way to start a process in a specified cgroup like > cgexec does. I could wrap the test in a shell: move the current shell to a > new cgroup then do exec to run the test app. But that seems cumbersome as > I need to spawn many shells, and check results of them. Another option is > create my own cgexec, which I'm sure will be very similar to cgexec code. > Was not sure how far we want to go with this. > > I can experiment with the shell wrapper idea and see how bad it can be and > fall back to implement cgexec? Open to suggestions. I guess there's only few variants of how cgexec is invoked right? The first thing we need to do is what exact steps those variants perform. After that we an decide how to implement exactly those variants. E.g. without knowing do we need to perform any syscalls or can this all done through sysfs affects somewhat how to proceed. Right now if I want to build e.g. image with BuildRoot I'd need to patch the existing recipe to add new dependencies in order to execute these tests, and probably do the same for every project that can package selftests to image. BR, Jarkko
On Sun, 31 Mar 2024 11:19:04 -0500, Jarkko Sakkinen <jarkko@kernel.org> wrote: > On Sat Mar 30, 2024 at 5:32 PM EET, Haitao Huang wrote: >> On Sat, 30 Mar 2024 06:15:14 -0500, Jarkko Sakkinen <jarkko@kernel.org> >> wrote: >> >> > On Thu Mar 28, 2024 at 5:54 AM EET, Haitao Huang wrote: >> >> On Wed, 27 Mar 2024 07:55:34 -0500, Jarkko Sakkinen >> <jarkko@kernel.org> >> >> wrote: >> >> >> >> > On Mon, 2024-02-05 at 13:06 -0800, Haitao Huang wrote: >> >> >> The scripts rely on cgroup-tools package from libcgroup [1]. >> >> >> >> >> >> To run selftests for epc cgroup: >> >> >> >> >> >> sudo ./run_epc_cg_selftests.sh >> >> >> >> >> >> To watch misc cgroup 'current' changes during testing, run this >> in a >> >> >> separate terminal: >> >> >> >> >> >> ./watch_misc_for_tests.sh current >> >> >> >> >> >> With different cgroups, the script starts one or multiple >> concurrent >> >> >> SGX >> >> >> selftests, each to run one unclobbered_vdso_oversubscribed >> test.Each >> >> >> of such test tries to load an enclave of EPC size equal to the EPC >> >> >> capacity available on the platform. The script checks results >> against >> >> >> the expectation set for each cgroup and reports success or >> failure. >> >> >> >> >> >> The script creates 3 different cgroups at the beginning with >> >> >> following >> >> >> expectations: >> >> >> >> >> >> 1) SMALL - intentionally small enough to fail the test loading an >> >> >> enclave of size equal to the capacity. >> >> >> 2) LARGE - large enough to run up to 4 concurrent tests but fail >> some >> >> >> if >> >> >> more than 4 concurrent tests are run. The script starts 4 >> expecting >> >> >> at >> >> >> least one test to pass, and then starts 5 expecting at least one >> test >> >> >> to fail. >> >> >> 3) LARGER - limit is the same as the capacity, large enough to run >> >> >> lots of >> >> >> concurrent tests. The script starts 8 of them and expects all >> pass. >> >> >> Then it reruns the same test with one process randomly killed and >> >> >> usage checked to be zero after all process exit. >> >> >> >> >> >> The script also includes a test with low mem_cg limit and LARGE >> >> >> sgx_epc >> >> >> limit to verify that the RAM used for per-cgroup reclamation is >> >> >> charged >> >> >> to a proper mem_cg. >> >> >> >> >> >> [1] https://github.com/libcgroup/libcgroup/blob/main/README >> >> >> >> >> >> Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com> >> >> >> --- >> >> >> V7: >> >> >> - Added memcontrol test. >> >> >> >> >> >> V5: >> >> >> - Added script with automatic results checking, remove the >> >> >> interactive >> >> >> script. >> >> >> - The script can run independent from the series below. >> >> >> --- >> >> >> .../selftests/sgx/run_epc_cg_selftests.sh | 246 >> >> >> ++++++++++++++++++ >> >> >> .../selftests/sgx/watch_misc_for_tests.sh | 13 + >> >> >> 2 files changed, 259 insertions(+) >> >> >> create mode 100755 >> >> >> tools/testing/selftests/sgx/run_epc_cg_selftests.sh >> >> >> create mode 100755 >> >> >> tools/testing/selftests/sgx/watch_misc_for_tests.sh >> >> >> >> >> >> diff --git a/tools/testing/selftests/sgx/run_epc_cg_selftests.sh >> >> >> b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh >> >> >> new file mode 100755 >> >> >> index 000000000000..e027bf39f005 >> >> >> --- /dev/null >> >> >> +++ b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh >> >> >> @@ -0,0 +1,246 @@ >> >> >> +#!/bin/bash >> >> > >> >> > This is not portable and neither does hold in the wild. >> >> > >> >> > It does not even often hold as it is not uncommon to place bash >> >> > to the path /usr/bin/bash. If I recall correctly, e.g. NixOS has >> >> > a path that is neither of those two. >> >> > >> >> > Should be #!/usr/bin/env bash >> >> > >> >> > That is POSIX compatible form. >> >> > >> >> >> >> Sure >> >> >> >> > Just got around trying to test this in NUC7 so looking into this in >> >> > more detail. >> >> >> >> Thanks. Could you please check if this version works for you? >> >> >> >> >> https://github.com/haitaohuang/linux/commit/3c424b841cf3cf66b085a424f4b537fbc3bbff6f >> >> >> >> > >> >> > That said can you make the script work with just "#!/usr/bin/env >> sh" >> >> > and make sure that it is busybox ash compatible? >> >> >> >> Yes. >> >> >> >> > >> >> > I don't see any necessity to make this bash only and it adds to the >> >> > compilation time of the image. Otherwise lot of this could be >> tested >> >> > just with qemu+bzImage+busybox(inside initramfs). >> >> > >> >> >> >> will still need cgroup-tools as you pointed out later. Compiling from >> >> its >> >> upstream code OK? >> > >> > Can you explain why you need it? >> > >> > What is the thing you cannot do without it? >> > >> > BR, Jarkko >> > >> I did not find a nice way to start a process in a specified cgroup like >> cgexec does. I could wrap the test in a shell: move the current shell >> to a >> new cgroup then do exec to run the test app. But that seems cumbersome >> as >> I need to spawn many shells, and check results of them. Another option >> is >> create my own cgexec, which I'm sure will be very similar to cgexec >> code. >> Was not sure how far we want to go with this. >> >> I can experiment with the shell wrapper idea and see how bad it can be >> and >> fall back to implement cgexec? Open to suggestions. > > I guess there's only few variants of how cgexec is invoked right? > > The first thing we need to do is what exact steps those variants > perform. > > After that we an decide how to implement exactly those variants. > > E.g. without knowing do we need to perform any syscalls or can > this all done through sysfs affects somewhat how to proceed. > > Right now if I want to build e.g. image with BuildRoot I'd need > to patch the existing recipe to add new dependencies in order to > execute these tests, and probably do the same for every project > that can package selftests to image. > > BR, Jarkko > Can be done through sysfs without syscalls. I implemented the wrapper shell and it looks not as bad I expected. Will send an add-on patch with that change and other changes to address your comments so far for the test scripts. If we agree on the approach, I'll squash it with this one later. Thanks Haitao
On Sun Mar 31, 2024 at 8:35 PM EEST, Haitao Huang wrote: > On Sun, 31 Mar 2024 11:19:04 -0500, Jarkko Sakkinen <jarkko@kernel.org> > wrote: > > > On Sat Mar 30, 2024 at 5:32 PM EET, Haitao Huang wrote: > >> On Sat, 30 Mar 2024 06:15:14 -0500, Jarkko Sakkinen <jarkko@kernel.org> > >> wrote: > >> > >> > On Thu Mar 28, 2024 at 5:54 AM EET, Haitao Huang wrote: > >> >> On Wed, 27 Mar 2024 07:55:34 -0500, Jarkko Sakkinen > >> <jarkko@kernel.org> > >> >> wrote: > >> >> > >> >> > On Mon, 2024-02-05 at 13:06 -0800, Haitao Huang wrote: > >> >> >> The scripts rely on cgroup-tools package from libcgroup [1]. > >> >> >> > >> >> >> To run selftests for epc cgroup: > >> >> >> > >> >> >> sudo ./run_epc_cg_selftests.sh > >> >> >> > >> >> >> To watch misc cgroup 'current' changes during testing, run this > >> in a > >> >> >> separate terminal: > >> >> >> > >> >> >> ./watch_misc_for_tests.sh current > >> >> >> > >> >> >> With different cgroups, the script starts one or multiple > >> concurrent > >> >> >> SGX > >> >> >> selftests, each to run one unclobbered_vdso_oversubscribed > >> test.Each > >> >> >> of such test tries to load an enclave of EPC size equal to the EPC > >> >> >> capacity available on the platform. The script checks results > >> against > >> >> >> the expectation set for each cgroup and reports success or > >> failure. > >> >> >> > >> >> >> The script creates 3 different cgroups at the beginning with > >> >> >> following > >> >> >> expectations: > >> >> >> > >> >> >> 1) SMALL - intentionally small enough to fail the test loading an > >> >> >> enclave of size equal to the capacity. > >> >> >> 2) LARGE - large enough to run up to 4 concurrent tests but fail > >> some > >> >> >> if > >> >> >> more than 4 concurrent tests are run. The script starts 4 > >> expecting > >> >> >> at > >> >> >> least one test to pass, and then starts 5 expecting at least one > >> test > >> >> >> to fail. > >> >> >> 3) LARGER - limit is the same as the capacity, large enough to run > >> >> >> lots of > >> >> >> concurrent tests. The script starts 8 of them and expects all > >> pass. > >> >> >> Then it reruns the same test with one process randomly killed and > >> >> >> usage checked to be zero after all process exit. > >> >> >> > >> >> >> The script also includes a test with low mem_cg limit and LARGE > >> >> >> sgx_epc > >> >> >> limit to verify that the RAM used for per-cgroup reclamation is > >> >> >> charged > >> >> >> to a proper mem_cg. > >> >> >> > >> >> >> [1] https://github.com/libcgroup/libcgroup/blob/main/README > >> >> >> > >> >> >> Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com> > >> >> >> --- > >> >> >> V7: > >> >> >> - Added memcontrol test. > >> >> >> > >> >> >> V5: > >> >> >> - Added script with automatic results checking, remove the > >> >> >> interactive > >> >> >> script. > >> >> >> - The script can run independent from the series below. > >> >> >> --- > >> >> >> .../selftests/sgx/run_epc_cg_selftests.sh | 246 > >> >> >> ++++++++++++++++++ > >> >> >> .../selftests/sgx/watch_misc_for_tests.sh | 13 + > >> >> >> 2 files changed, 259 insertions(+) > >> >> >> create mode 100755 > >> >> >> tools/testing/selftests/sgx/run_epc_cg_selftests.sh > >> >> >> create mode 100755 > >> >> >> tools/testing/selftests/sgx/watch_misc_for_tests.sh > >> >> >> > >> >> >> diff --git a/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > >> >> >> b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > >> >> >> new file mode 100755 > >> >> >> index 000000000000..e027bf39f005 > >> >> >> --- /dev/null > >> >> >> +++ b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh > >> >> >> @@ -0,0 +1,246 @@ > >> >> >> +#!/bin/bash > >> >> > > >> >> > This is not portable and neither does hold in the wild. > >> >> > > >> >> > It does not even often hold as it is not uncommon to place bash > >> >> > to the path /usr/bin/bash. If I recall correctly, e.g. NixOS has > >> >> > a path that is neither of those two. > >> >> > > >> >> > Should be #!/usr/bin/env bash > >> >> > > >> >> > That is POSIX compatible form. > >> >> > > >> >> > >> >> Sure > >> >> > >> >> > Just got around trying to test this in NUC7 so looking into this in > >> >> > more detail. > >> >> > >> >> Thanks. Could you please check if this version works for you? > >> >> > >> >> > >> https://github.com/haitaohuang/linux/commit/3c424b841cf3cf66b085a424f4b537fbc3bbff6f > >> >> > >> >> > > >> >> > That said can you make the script work with just "#!/usr/bin/env > >> sh" > >> >> > and make sure that it is busybox ash compatible? > >> >> > >> >> Yes. > >> >> > >> >> > > >> >> > I don't see any necessity to make this bash only and it adds to the > >> >> > compilation time of the image. Otherwise lot of this could be > >> tested > >> >> > just with qemu+bzImage+busybox(inside initramfs). > >> >> > > >> >> > >> >> will still need cgroup-tools as you pointed out later. Compiling from > >> >> its > >> >> upstream code OK? > >> > > >> > Can you explain why you need it? > >> > > >> > What is the thing you cannot do without it? > >> > > >> > BR, Jarkko > >> > > >> I did not find a nice way to start a process in a specified cgroup like > >> cgexec does. I could wrap the test in a shell: move the current shell > >> to a > >> new cgroup then do exec to run the test app. But that seems cumbersome > >> as > >> I need to spawn many shells, and check results of them. Another option > >> is > >> create my own cgexec, which I'm sure will be very similar to cgexec > >> code. > >> Was not sure how far we want to go with this. > >> > >> I can experiment with the shell wrapper idea and see how bad it can be > >> and > >> fall back to implement cgexec? Open to suggestions. > > > > I guess there's only few variants of how cgexec is invoked right? > > > > The first thing we need to do is what exact steps those variants > > perform. > > > > After that we an decide how to implement exactly those variants. > > > > E.g. without knowing do we need to perform any syscalls or can > > this all done through sysfs affects somewhat how to proceed. > > > > Right now if I want to build e.g. image with BuildRoot I'd need > > to patch the existing recipe to add new dependencies in order to > > execute these tests, and probably do the same for every project > > that can package selftests to image. > > > > BR, Jarkko > > > Can be done through sysfs without syscalls. I implemented the wrapper > shell and it looks not as bad I expected. > Will send an add-on patch with that change and other changes to address > your comments so far for the test scripts. > If we agree on the approach, I'll squash it with this one later. So think this way. By having these open coded we can check in detail that the feature works exactly how we would like. Those tools add an abstraction layer which makes it harder to evaluate this as a whole. Also, that renders away all possible issues related to different versions of 3rd party tools and their possible incompatibilities. If pure bash for some reason would turn out to be unmanageable nothing prevents to e.g. implement a small tool in C or Python that does the tasks required. E.g. for testing TPM2 chips I implemented custom TPM2 user space just for kselftest, and in SGX we implemented "bare metal" enclave implementation with the same rationale. BR, Jarkko
Hello. On Sat, Mar 30, 2024 at 01:26:08PM +0200, Jarkko Sakkinen <jarkko@kernel.org> wrote: > > > It'd be more complicated and less readable to do all the stuff without the > > > cgroup-tools, esp cgexec. I checked dependency, cgroup-tools only depends > > > on libc so I hope this would not cause too much inconvenience. > > > > As per cgroup-tools, please prove this. It makes the job for more > > complicated *for you* and you are making the job more complicated > > to every possible person in the planet running any kernel QA. > > > > I weight the latter more than the former. And it is exactly the > > reason why we did custom user space kselftest in the first place. > > Let's keep the tradition. All I can say is that kselftest is > > unfinished in its current form. > > > > What is "esp cgexec"? > > Also in kselftest we don't drive ultimate simplicity, we drive > efficient CI/QA. By open coding something like subset of > cgroup-tools needed to run the test you also help us later > on to backtrack the kernel changes. With cgroups-tools you > would have to use strace to get the same info. FWIW, see also functions in tools/testing/selftests/cgroup/cgroup_util.{h,c}. They likely cover what you need already -- if the tests are in C. (I admit that stuff in tools/testing/selftests/cgroup/ is best understood with strace.) HTH, Michal
On Tue Apr 2, 2024 at 2:23 PM EEST, Michal Koutný wrote: > Hello. > > On Sat, Mar 30, 2024 at 01:26:08PM +0200, Jarkko Sakkinen <jarkko@kernel.org> wrote: > > > > It'd be more complicated and less readable to do all the stuff without the > > > > cgroup-tools, esp cgexec. I checked dependency, cgroup-tools only depends > > > > on libc so I hope this would not cause too much inconvenience. > > > > > > As per cgroup-tools, please prove this. It makes the job for more > > > complicated *for you* and you are making the job more complicated > > > to every possible person in the planet running any kernel QA. > > > > > > I weight the latter more than the former. And it is exactly the > > > reason why we did custom user space kselftest in the first place. > > > Let's keep the tradition. All I can say is that kselftest is > > > unfinished in its current form. > > > > > > What is "esp cgexec"? > > > > Also in kselftest we don't drive ultimate simplicity, we drive > > efficient CI/QA. By open coding something like subset of > > cgroup-tools needed to run the test you also help us later > > on to backtrack the kernel changes. With cgroups-tools you > > would have to use strace to get the same info. > > FWIW, see also functions in > tools/testing/selftests/cgroup/cgroup_util.{h,c}. > They likely cover what you need already -- if the tests are in C. > > (I admit that stuff in tools/testing/selftests/cgroup/ is best > understood with strace.) Thanks! My conclusions are that: 1. We probably cannot move the test part of cgroup test itself given the enclave payload dependency. 2. I think it makes sense to still follow the same pattern as other cgroups test and re-use cgroup_util.[ch] functionaltiy. So yeah I guess we need two test programs instead of one. Something along the lines: 1. main.[ch] -> test_sgx.[ch] 2. introduce test_sgx_cgroup.c And test_sgx_cgroup.c would be implement similar test as the shell script and would follow the structure of existing cgroups tests. > > HTH, > Michal BR, Jarkko
On 3/30/24 04:23, Jarkko Sakkinen wrote: >>> I also wonder is cgroup-tools dependency absolutely required or could >>> you just have a function that would interact with sysfs? >> I should have checked email before hit the send button for v10
On Tue, 02 Apr 2024 06:58:40 -0500, Jarkko Sakkinen <jarkko@kernel.org> wrote: > On Tue Apr 2, 2024 at 2:23 PM EEST, Michal Koutný wrote: >> Hello. >> >> On Sat, Mar 30, 2024 at 01:26:08PM +0200, Jarkko Sakkinen >> <jarkko@kernel.org> wrote: >> > > > It'd be more complicated and less readable to do all the stuff >> without the >> > > > cgroup-tools, esp cgexec. I checked dependency, cgroup-tools only >> depends >> > > > on libc so I hope this would not cause too much inconvenience. >> > > >> > > As per cgroup-tools, please prove this. It makes the job for more >> > > complicated *for you* and you are making the job more complicated >> > > to every possible person in the planet running any kernel QA. >> > > >> > > I weight the latter more than the former. And it is exactly the >> > > reason why we did custom user space kselftest in the first place. >> > > Let's keep the tradition. All I can say is that kselftest is >> > > unfinished in its current form. >> > > >> > > What is "esp cgexec"? >> > >> > Also in kselftest we don't drive ultimate simplicity, we drive >> > efficient CI/QA. By open coding something like subset of >> > cgroup-tools needed to run the test you also help us later >> > on to backtrack the kernel changes. With cgroups-tools you >> > would have to use strace to get the same info. >> >> FWIW, see also functions in >> tools/testing/selftests/cgroup/cgroup_util.{h,c}. >> They likely cover what you need already -- if the tests are in C. >> >> (I admit that stuff in tools/testing/selftests/cgroup/ is best >> understood with strace.) > > Thanks! > > My conclusions are that: > > 1. We probably cannot move the test part of cgroup test itself > given the enclave payload dependency. > 2. I think it makes sense to still follow the same pattern as > other cgroups test and re-use cgroup_util.[ch] functionaltiy. > > So yeah I guess we need two test programs instead of one. > > Something along the lines: > > 1. main.[ch] -> test_sgx.[ch] > 2. introduce test_sgx_cgroup.c > > And test_sgx_cgroup.c would be implement similar test as the shell > script and would follow the structure of existing cgroups tests. > >> >> HTH, >> Michal > > BR, Jarkko > Do we really want to have it implemented in c? There are much fewer lines of code in shell scripts. Note we are not really testing basic cgroup stuff. All we needed were creating/deleting cgroups and set limits which I think have been demonstrated feasible in the ash scripts now. Given Dave's comments, and test scripts being working and cover the cases needed IMHO, I don't see much need to move to c code. I can add more cases if needed and fall back a c implementation later if any case can't be implemented in scripts. How about that? Haitao
On Tue, Apr 02, 2024 at 11:20:21AM -0500, Haitao Huang <haitao.huang@linux.intel.com> wrote: > Do we really want to have it implemented in c? I only pointed to the available C boilerplate. > There are much fewer lines of > code in shell scripts. Note we are not really testing basic cgroup stuff. > All we needed were creating/deleting cgroups and set limits which I think > have been demonstrated feasible in the ash scripts now. I assume you refer to Message-Id: <20240331174442.51019-1-haitao.huang@linux.intel.com> right? Could it be even simpler if you didn't stick to cgtools APIs and v1 compatibility? Reducing ash_cgexec.sh to something like echo 0 >$R/$1/cgroup.procs shift exec "$@" (with some small builerplate for $R and previous mkdirs) Thanks, Michal
On Tue, 02 Apr 2024 12:40:03 -0500, Michal Koutný <mkoutny@suse.com> wrote: > On Tue, Apr 02, 2024 at 11:20:21AM -0500, Haitao Huang > <haitao.huang@linux.intel.com> wrote: >> Do we really want to have it implemented in c? > > I only pointed to the available C boilerplate. > >> There are much fewer lines of >> code in shell scripts. Note we are not really testing basic cgroup >> stuff. >> All we needed were creating/deleting cgroups and set limits which I >> think >> have been demonstrated feasible in the ash scripts now. > > I assume you refer to > Message-Id: <20240331174442.51019-1-haitao.huang@linux.intel.com> > right? > > Could it be even simpler if you didn't stick to cgtools APIs and v1 > compatibility? > > Reducing ash_cgexec.sh to something like > echo 0 >$R/$1/cgroup.procs > shift > exec "$@" > (with some small builerplate for $R and previous mkdirs) > Yes, Thanks for the suggestion. Haitao
On Tue Apr 2, 2024 at 6:42 PM EEST, Dave Hansen wrote: > On 3/30/24 04:23, Jarkko Sakkinen wrote: > >>> I also wonder is cgroup-tools dependency absolutely required or could > >>> you just have a function that would interact with sysfs? > >> I should have checked email before hit the send button for v10
On Tue Apr 2, 2024 at 7:20 PM EEST, Haitao Huang wrote: > On Tue, 02 Apr 2024 06:58:40 -0500, Jarkko Sakkinen <jarkko@kernel.org> > wrote: > > > On Tue Apr 2, 2024 at 2:23 PM EEST, Michal Koutný wrote: > >> Hello. > >> > >> On Sat, Mar 30, 2024 at 01:26:08PM +0200, Jarkko Sakkinen > >> <jarkko@kernel.org> wrote: > >> > > > It'd be more complicated and less readable to do all the stuff > >> without the > >> > > > cgroup-tools, esp cgexec. I checked dependency, cgroup-tools only > >> depends > >> > > > on libc so I hope this would not cause too much inconvenience. > >> > > > >> > > As per cgroup-tools, please prove this. It makes the job for more > >> > > complicated *for you* and you are making the job more complicated > >> > > to every possible person in the planet running any kernel QA. > >> > > > >> > > I weight the latter more than the former. And it is exactly the > >> > > reason why we did custom user space kselftest in the first place. > >> > > Let's keep the tradition. All I can say is that kselftest is > >> > > unfinished in its current form. > >> > > > >> > > What is "esp cgexec"? > >> > > >> > Also in kselftest we don't drive ultimate simplicity, we drive > >> > efficient CI/QA. By open coding something like subset of > >> > cgroup-tools needed to run the test you also help us later > >> > on to backtrack the kernel changes. With cgroups-tools you > >> > would have to use strace to get the same info. > >> > >> FWIW, see also functions in > >> tools/testing/selftests/cgroup/cgroup_util.{h,c}. > >> They likely cover what you need already -- if the tests are in C. > >> > >> (I admit that stuff in tools/testing/selftests/cgroup/ is best > >> understood with strace.) > > > > Thanks! > > > > My conclusions are that: > > > > 1. We probably cannot move the test part of cgroup test itself > > given the enclave payload dependency. > > 2. I think it makes sense to still follow the same pattern as > > other cgroups test and re-use cgroup_util.[ch] functionaltiy. > > > > So yeah I guess we need two test programs instead of one. > > > > Something along the lines: > > > > 1. main.[ch] -> test_sgx.[ch] > > 2. introduce test_sgx_cgroup.c > > > > And test_sgx_cgroup.c would be implement similar test as the shell > > script and would follow the structure of existing cgroups tests. > > > >> > >> HTH, > >> Michal > > > > BR, Jarkko > > > Do we really want to have it implemented in c? There are much fewer lines > of code in shell scripts. Note we are not really testing basic cgroup > stuff. All we needed were creating/deleting cgroups and set limits which I > think have been demonstrated feasible in the ash scripts now. > > Given Dave's comments, and test scripts being working and cover the cases > needed IMHO, I don't see much need to move to c code. I can add more cases > if needed and fall back a c implementation later if any case can't be > implemented in scripts. How about that? We can settle to: ash + no dependencies. I guess you have for that all the work done already. BR, Jarkko
On Tue Apr 2, 2024 at 8:40 PM EEST, Michal Koutný wrote: > On Tue, Apr 02, 2024 at 11:20:21AM -0500, Haitao Huang <haitao.huang@linux.intel.com> wrote: > > Do we really want to have it implemented in c? > > I only pointed to the available C boilerplate. > > > There are much fewer lines of > > code in shell scripts. Note we are not really testing basic cgroup stuff. > > All we needed were creating/deleting cgroups and set limits which I think > > have been demonstrated feasible in the ash scripts now. > > I assume you refer to > Message-Id: <20240331174442.51019-1-haitao.huang@linux.intel.com> > right? > > Could it be even simpler if you didn't stick to cgtools APIs and v1 > compatibility? > > Reducing ash_cgexec.sh to something like > echo 0 >$R/$1/cgroup.procs > shift > exec "$@" > (with some small builerplate for $R and previous mkdirs) I already asked about necessity of v1 in some response, and fully support this idea. Then cgexec can simply be a function wrapping along the lines what you proposed. BR, Jarkko
diff --git a/tools/testing/selftests/sgx/run_epc_cg_selftests.sh b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh new file mode 100755 index 000000000000..e027bf39f005 --- /dev/null +++ b/tools/testing/selftests/sgx/run_epc_cg_selftests.sh @@ -0,0 +1,246 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright(c) 2023 Intel Corporation. + +TEST_ROOT_CG=selftest +cgcreate -g misc:$TEST_ROOT_CG +if [ $? -ne 0 ]; then + echo "# Please make sure cgroup-tools is installed, and misc cgroup is mounted." + exit 1 +fi +TEST_CG_SUB1=$TEST_ROOT_CG/test1 +TEST_CG_SUB2=$TEST_ROOT_CG/test2 +# We will only set limit in test1 and run tests in test3 +TEST_CG_SUB3=$TEST_ROOT_CG/test1/test3 +TEST_CG_SUB4=$TEST_ROOT_CG/test4 + +cgcreate -g misc:$TEST_CG_SUB1 +cgcreate -g misc:$TEST_CG_SUB2 +cgcreate -g misc:$TEST_CG_SUB3 +cgcreate -g misc:$TEST_CG_SUB4 + +# Default to V2 +CG_MISC_ROOT=/sys/fs/cgroup +CG_MEM_ROOT=/sys/fs/cgroup +CG_V1=0 +if [ ! -d "/sys/fs/cgroup/misc" ]; then + echo "# cgroup V2 is in use." +else + echo "# cgroup V1 is in use." + CG_MISC_ROOT=/sys/fs/cgroup/misc + CG_MEM_ROOT=/sys/fs/cgroup/memory + CG_V1=1 +fi + +CAPACITY=$(grep "sgx_epc" "$CG_MISC_ROOT/misc.capacity" | awk '{print $2}') +# This is below number of VA pages needed for enclave of capacity size. So +# should fail oversubscribed cases +SMALL=$(( CAPACITY / 512 )) + +# At least load one enclave of capacity size successfully, maybe up to 4. +# But some may fail if we run more than 4 concurrent enclaves of capacity size. +LARGE=$(( SMALL * 4 )) + +# Load lots of enclaves +LARGER=$CAPACITY +echo "# Setting up limits." +echo "sgx_epc $SMALL" > $CG_MISC_ROOT/$TEST_CG_SUB1/misc.max +echo "sgx_epc $LARGE" > $CG_MISC_ROOT/$TEST_CG_SUB2/misc.max +echo "sgx_epc $LARGER" > $CG_MISC_ROOT/$TEST_CG_SUB4/misc.max + +timestamp=$(date +%Y%m%d_%H%M%S) + +test_cmd="./test_sgx -t unclobbered_vdso_oversubscribed" + +wait_check_process_status() { + local pid=$1 + local check_for_success=$2 # If 1, check for success; + # If 0, check for failure + wait "$pid" + local status=$? + + if [[ $check_for_success -eq 1 && $status -eq 0 ]]; then + echo "# Process $pid succeeded." + return 0 + elif [[ $check_for_success -eq 0 && $status -ne 0 ]]; then + echo "# Process $pid returned failure." + return 0 + fi + return 1 +} + +wait_and_detect_for_any() { + local pids=("$@") + local check_for_success=$1 # If 1, check for success; + # If 0, check for failure + local detected=1 # 0 for success detection + + for pid in "${pids[@]:1}"; do + if wait_check_process_status "$pid" "$check_for_success"; then + detected=0 + # Wait for other processes to exit + fi + done + + return $detected +} + +echo "# Start unclobbered_vdso_oversubscribed with SMALL limit, expecting failure..." +# Always use leaf node of misc cgroups so it works for both v1 and v2 +# these may fail on OOM +cgexec -g misc:$TEST_CG_SUB3 $test_cmd >cgtest_small_$timestamp.log 2>&1 +if [[ $? -eq 0 ]]; then + echo "# Fail on SMALL limit, not expecting any test passes." + cgdelete -r -g misc:$TEST_ROOT_CG + exit 1 +else + echo "# Test failed as expected." +fi + +echo "# PASSED SMALL limit." + +echo "# Start 4 concurrent unclobbered_vdso_oversubscribed tests with LARGE limit, + expecting at least one success...." + +pids=() +for i in {1..4}; do + ( + cgexec -g misc:$TEST_CG_SUB2 $test_cmd >cgtest_large_positive_$timestamp.$i.log 2>&1 + ) & + pids+=($!) +done + + +if wait_and_detect_for_any 1 "${pids[@]}"; then + echo "# PASSED LARGE limit positive testing." +else + echo "# Failed on LARGE limit positive testing, no test passes." + cgdelete -r -g misc:$TEST_ROOT_CG + exit 1 +fi + +echo "# Start 5 concurrent unclobbered_vdso_oversubscribed tests with LARGE limit, + expecting at least one failure...." +pids=() +for i in {1..5}; do + ( + cgexec -g misc:$TEST_CG_SUB2 $test_cmd >cgtest_large_negative_$timestamp.$i.log 2>&1 + ) & + pids+=($!) +done + +if wait_and_detect_for_any 0 "${pids[@]}"; then + echo "# PASSED LARGE limit negative testing." +else + echo "# Failed on LARGE limit negative testing, no test fails." + cgdelete -r -g misc:$TEST_ROOT_CG + exit 1 +fi + +echo "# Start 8 concurrent unclobbered_vdso_oversubscribed tests with LARGER limit, + expecting no failure...." +pids=() +for i in {1..8}; do + ( + cgexec -g misc:$TEST_CG_SUB4 $test_cmd >cgtest_larger_$timestamp.$i.log 2>&1 + ) & + pids+=($!) +done + +if wait_and_detect_for_any 0 "${pids[@]}"; then + echo "# Failed on LARGER limit, at least one test fails." + cgdelete -r -g misc:$TEST_ROOT_CG + exit 1 +else + echo "# PASSED LARGER limit tests." +fi + +echo "# Start 8 concurrent unclobbered_vdso_oversubscribed tests with LARGER limit, + randomly kill one, expecting no failure...." +pids=() +for i in {1..8}; do + ( + cgexec -g misc:$TEST_CG_SUB4 $test_cmd >cgtest_larger_kill_$timestamp.$i.log 2>&1 + ) & + pids+=($!) +done + +sleep $((RANDOM % 10 + 5)) + +# Randomly select a PID to kill +RANDOM_INDEX=$((RANDOM % 8)) +PID_TO_KILL=${pids[RANDOM_INDEX]} + +kill $PID_TO_KILL +echo "# Killed process with PID: $PID_TO_KILL" + +any_failure=0 +for pid in "${pids[@]}"; do + wait "$pid" + status=$? + if [ "$pid" != "$PID_TO_KILL" ]; then + if [[ $status -ne 0 ]]; then + echo "# Process $pid returned failure." + any_failure=1 + fi + fi +done + +if [[ $any_failure -ne 0 ]]; then + echo "# Failed on random killing, at least one test fails." + cgdelete -r -g misc:$TEST_ROOT_CG + exit 1 +fi +echo "# PASSED LARGER limit test with a process randomly killed." + +cgcreate -g memory:$TEST_CG_SUB2 +if [ $? -ne 0 ]; then + echo "# Failed creating memory controller." + cgdelete -r -g misc:$TEST_ROOT_CG + exit 1 +fi +MEM_LIMIT_TOO_SMALL=$((CAPACITY - 2 * LARGE)) + +if [[ $CG_V1 -eq 0 ]]; then + echo "$MEM_LIMIT_TOO_SMALL" > $CG_MEM_ROOT/$TEST_CG_SUB2/memory.max +else + echo "$MEM_LIMIT_TOO_SMALL" > $CG_MEM_ROOT/$TEST_CG_SUB2/memory.limit_in_bytes + echo "$MEM_LIMIT_TOO_SMALL" > $CG_MEM_ROOT/$TEST_CG_SUB2/memory.memsw.limit_in_bytes +fi + +echo "# Start 4 concurrent unclobbered_vdso_oversubscribed tests with LARGE EPC limit, + and too small RAM limit, expecting all failures...." +pids=() +for i in {1..4}; do + ( + cgexec -g memory:$TEST_CG_SUB2 -g misc:$TEST_CG_SUB2 $test_cmd \ + >cgtest_large_oom_$timestamp.$i.log 2>&1 + ) & + pids+=($!) +done + +if wait_and_detect_for_any 1 "${pids[@]}"; then + echo "# Failed on tests with memcontrol, some tests did not fail." + cgdelete -r -g misc:$TEST_ROOT_CG + if [[ $CG_V1 -ne 0 ]]; then + cgdelete -r -g memory:$TEST_ROOT_CG + fi + exit 1 +else + echo "# PASSED LARGE limit tests with memcontrol." +fi + +sleep 2 + +USAGE=$(grep '^sgx_epc' "$CG_MISC_ROOT/$TEST_ROOT_CG/misc.current" | awk '{print $2}') +if [ "$USAGE" -ne 0 ]; then + echo "# Failed: Final usage is $USAGE, not 0." +else + echo "# PASSED leakage check." + echo "# PASSED ALL cgroup limit tests, cleanup cgroups..." +fi +cgdelete -r -g misc:$TEST_ROOT_CG +if [[ $CG_V1 -ne 0 ]]; then + cgdelete -r -g memory:$TEST_ROOT_CG +fi +echo "# done." diff --git a/tools/testing/selftests/sgx/watch_misc_for_tests.sh b/tools/testing/selftests/sgx/watch_misc_for_tests.sh new file mode 100755 index 000000000000..dbd38f346e7b --- /dev/null +++ b/tools/testing/selftests/sgx/watch_misc_for_tests.sh @@ -0,0 +1,13 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright(c) 2023 Intel Corporation. + +if [ -z "$1" ] + then + echo "No argument supplied, please provide 'max', 'current' or 'events'" + exit 1 +fi + +watch -n 1 "find /sys/fs/cgroup -wholename */test*/misc.$1 -exec sh -c \ + 'echo \"\$1:\"; cat \"\$1\"' _ {} \;" +
The scripts rely on cgroup-tools package from libcgroup [1]. To run selftests for epc cgroup: sudo ./run_epc_cg_selftests.sh To watch misc cgroup 'current' changes during testing, run this in a separate terminal: ./watch_misc_for_tests.sh current With different cgroups, the script starts one or multiple concurrent SGX selftests, each to run one unclobbered_vdso_oversubscribed test. Each of such test tries to load an enclave of EPC size equal to the EPC capacity available on the platform. The script checks results against the expectation set for each cgroup and reports success or failure. The script creates 3 different cgroups at the beginning with following expectations: 1) SMALL - intentionally small enough to fail the test loading an enclave of size equal to the capacity. 2) LARGE - large enough to run up to 4 concurrent tests but fail some if more than 4 concurrent tests are run. The script starts 4 expecting at least one test to pass, and then starts 5 expecting at least one test to fail. 3) LARGER - limit is the same as the capacity, large enough to run lots of concurrent tests. The script starts 8 of them and expects all pass. Then it reruns the same test with one process randomly killed and usage checked to be zero after all process exit. The script also includes a test with low mem_cg limit and LARGE sgx_epc limit to verify that the RAM used for per-cgroup reclamation is charged to a proper mem_cg. [1] https://github.com/libcgroup/libcgroup/blob/main/README Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com> --- V7: - Added memcontrol test. V5: - Added script with automatic results checking, remove the interactive script. - The script can run independent from the series below. --- .../selftests/sgx/run_epc_cg_selftests.sh | 246 ++++++++++++++++++ .../selftests/sgx/watch_misc_for_tests.sh | 13 + 2 files changed, 259 insertions(+) create mode 100755 tools/testing/selftests/sgx/run_epc_cg_selftests.sh create mode 100755 tools/testing/selftests/sgx/watch_misc_for_tests.sh