In some distributions, the two files used in lib/tst_pid.c are not available, but cgroups still imposes a task limit far smaller than the kernel pid_max. If the cgroup sysfs is mounted, we can use it to retrieve the current task limit imposed to the process. Implement the retrieval of this limit.
This can be done by first checking /proc/self/cgroup to get the cgroup the process is in, which will be a path under the cgroup sysfs. To get the path to the cgroup sysfs, check /proc/self/mountinfo. Finally, concatenate those two paths with pid.max to get the full path to the file containing the limit.
This patch changes the way read_session_pids_limit is called, not passing a format string to be completed anymore, but is still used the same way. A following patch will update this function.
This fixes failures for msgstress04.
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com --- lib/tst_pid.c | 53 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 10 deletions(-)
diff --git a/lib/tst_pid.c b/lib/tst_pid.c index 21cadef2a..f140acbc0 100644 --- a/lib/tst_pid.c +++ b/lib/tst_pid.c @@ -31,8 +31,6 @@ #include "tst_safe_macros.h"
#define PID_MAX_PATH "/proc/sys/kernel/pid_max" -#define CGROUPS_V1_SLICE_FMT "/sys/fs/cgroup/pids/user.slice/user-%d.slice/pids.max" -#define CGROUPS_V2_SLICE_FMT "/sys/fs/cgroup/user.slice/user-%d.slice/pids.max" /* Leave some available processes for the OS */ #define PIDS_RESERVE 50
@@ -96,18 +94,53 @@ static int read_session_pids_limit(const char *path_fmt, int uid,
static int get_session_pids_limit(void (*cleanup_fn) (void)) { - int max_pids, uid; + char path[PATH_MAX + 1]; + char cgroup_pids[PATH_MAX + 1]; + char catchall; + int uid, ret = 0;
uid = get_session_uid(); - max_pids = read_session_pids_limit(CGROUPS_V2_SLICE_FMT, uid, cleanup_fn); - if (max_pids < 0) - max_pids = read_session_pids_limit(CGROUPS_V1_SLICE_FMT, uid, - cleanup_fn); + /* Check for generic cgroup v1 pid.max */ + ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup", + "%*d:pids:%s\n", cgroup_pids); + /* + * This is a bit of a hack of scanf format strings. Indeed, if all + * conversion specifications have been matched the return of scanf will be + * the same whether any outstanding literal characters match or not. + * As we want to match the literal part, we can add a catchall after it + * so that it won't be counted if the literal part doesn't match. + * This makes the macro go to the next line until the catchall, thus + * the literal parts, is matched. + * + * Assume that the root of the mount is '/'. It can be anything, + * but it should be '/' on any normal system. + */ + if (!ret) + ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo", + "%*s %*s %*s %*s %s %*s %*s - %*s cgroup %*[rw],pid%c", + path, &catchall); + + if (!ret) { + strncat(path, cgroup_pids, PATH_MAX); + strncat(path, "/pids.max", PATH_MAX); + return read_session_pids_limit(path, uid, cleanup_fn); + }
- if (max_pids < 0) - return -1; + /* Check for generic cgroup v2 pid.max */ + ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup", + "%*d::%s\n", cgroup_pids); + if (!ret) + ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo", + "%*s %*s %*s %*s %s %*s %*s - cgroup2 %c", + path, &catchall); + + if (!ret) { + strncat(path, cgroup_pids, PATH_MAX); + strncat(path, "/pids.max", PATH_MAX); + return read_session_pids_limit(path, uid, cleanup_fn); + }
- return max_pids; + return -1; }
int tst_get_free_pids_(void (*cleanup_fn) (void))
A cgroup resource limitation can be either a number or "max". It means that the cgroup will not be limited _more_ than it already is. This can mean that it will use the kernel limit for the resource, if it exists, or the limit of a parent cgroup.
This patch reworks "read_session_pids_limit" to go up the cgroup hierarchy if it encounters a "max" value rather than a numerical one, using the kernel limit in the event where it doesn't find any.
Clean up uid related code as it is not used anymore.
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com Co-developed-by: Beata Michalska beata.michalsk@arm.com --- lib/tst_pid.c | 96 +++++++++++++++++++++++++++++---------------------- 1 file changed, 54 insertions(+), 42 deletions(-)
diff --git a/lib/tst_pid.c b/lib/tst_pid.c index f140acbc0..107c47e39 100644 --- a/lib/tst_pid.c +++ b/lib/tst_pid.c @@ -43,50 +43,69 @@ pid_t tst_get_unused_pid_(void (*cleanup_fn) (void)) return pid; }
-/* - * Get the effective session UID - either one invoking current test via sudo - * or the real UID. - */ -static unsigned int get_session_uid(void) +static int __read_pids_limit(const char *path, void (*cleanup_fn) (void)) { - const char *sudo_uid; + char max_pids_value[100]; + int max_pids;
- sudo_uid = getenv("SUDO_UID"); - if (sudo_uid) { - unsigned int real_uid; - int ret; + if (access(path, R_OK) != 0) { + tst_resm(TINFO, "Cannot read session user limits from '%s'", path); + return -1; + }
- ret = sscanf(sudo_uid, "%u", &real_uid); - if (ret == 1) - return real_uid; + SAFE_FILE_SCANF(cleanup_fn, path, "%s", max_pids_value); + if (strcmp(max_pids_value, "max")) { + max_pids = SAFE_STRTOL(max_pids_value, 0, INT_MAX); + tst_resm(TINFO, "Found limit of processes %d (from %s)", + max_pids, path); + } else { + max_pids = -1; }
- return getuid(); + return max_pids; }
-static int read_session_pids_limit(const char *path_fmt, int uid, - void (*cleanup_fn) (void)) +/* + * Take the path to the cgroup mount and to the current cgroup pid controller + * and try to find the PID limit imposed by cgroup. + * Go up the cgroup hierarchy if needed, otherwise use the kernel PID limit. + */ +static int read_session_pids_limit(const char *cgroup_mount, + const char *cgroup_path, void (*cleanup_fn) (void)) { - int max_pids, ret; - char max_pid_value[100]; - char path[PATH_MAX]; - - ret = snprintf(path, sizeof(path), path_fmt, uid); + int ret, cgroup_depth = 0, max_pids = -1; + char path[PATH_MAX + 1], file_path[PATH_MAX + 1]; + const char *sub_path = cgroup_path; + + /* Find the number of groups we can go up. */ + do { + cgroup_depth += 1; + sub_path++; + sub_path = strchr(sub_path, '/'); + } while (sub_path); + + ret = snprintf(path, sizeof(path), "%s%s", cgroup_mount, cgroup_path); if (ret < 0 || (size_t)ret >= sizeof(path)) return -1;
- if (access(path, R_OK) != 0) { - tst_resm(TINFO, "Cannot read session user limits from '%s'", path); - return -1; + for (int i = 0 ; i < cgroup_depth ; i++) { + /* Create a path to read from. */ + ret = snprintf(file_path, sizeof(file_path), "%s/pids.max", path); + if (ret < 0 || (size_t)ret >= sizeof(file_path)) + return -1; + + max_pids = __read_pids_limit(file_path, cleanup_fn); + if (max_pids >= 0) + return max_pids; + + strncat(path, "/..", PATH_MAX); }
- SAFE_FILE_SCANF(cleanup_fn, path, "%s", max_pid_value); - if (!strcmp(max_pid_value, "max")) { + if (max_pids < 0) { + /* Read kernel imposed limits */ SAFE_FILE_SCANF(cleanup_fn, PID_MAX_PATH, "%d", &max_pids); - tst_resm(TINFO, "Found limit of processes %d (from %s=max)", max_pids, path); - } else { - max_pids = SAFE_STRTOL(max_pid_value, 0, INT_MAX); - tst_resm(TINFO, "Found limit of processes %d (from %s)", max_pids, path); + tst_resm(TINFO, "Using kernel processes limit of %d", + max_pids); }
return max_pids; @@ -97,9 +116,8 @@ static int get_session_pids_limit(void (*cleanup_fn) (void)) char path[PATH_MAX + 1]; char cgroup_pids[PATH_MAX + 1]; char catchall; - int uid, ret = 0; + int ret = 0;
- uid = get_session_uid(); /* Check for generic cgroup v1 pid.max */ ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup", "%*d:pids:%s\n", cgroup_pids); @@ -120,11 +138,8 @@ static int get_session_pids_limit(void (*cleanup_fn) (void)) "%*s %*s %*s %*s %s %*s %*s - %*s cgroup %*[rw],pid%c", path, &catchall);
- if (!ret) { - strncat(path, cgroup_pids, PATH_MAX); - strncat(path, "/pids.max", PATH_MAX); - return read_session_pids_limit(path, uid, cleanup_fn); - } + if (!ret) + return read_session_pids_limit(path, cgroup_pids, cleanup_fn);
/* Check for generic cgroup v2 pid.max */ ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup", @@ -134,11 +149,8 @@ static int get_session_pids_limit(void (*cleanup_fn) (void)) "%*s %*s %*s %*s %s %*s %*s - cgroup2 %c", path, &catchall);
- if (!ret) { - strncat(path, cgroup_pids, PATH_MAX); - strncat(path, "/pids.max", PATH_MAX); - return read_session_pids_limit(path, uid, cleanup_fn); - } + if (!ret) + return read_session_pids_limit(path, cgroup_pids, cleanup_fn);
return -1; }
It appears that msgstress03 doesn't account for all PIDs that its children can use, as it expects the tasks will terminate quickly and not reach the PID limit. On some systems, this assumption can be invalid and the PID limit will be hit. Change the limit to account for all possible children at once, knowning that each child will fork as well.
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com --- testcases/kernel/syscalls/ipc/msgstress/msgstress03.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/testcases/kernel/syscalls/ipc/msgstress/msgstress03.c b/testcases/kernel/syscalls/ipc/msgstress/msgstress03.c index 3cb70ab18..0c46927b8 100644 --- a/testcases/kernel/syscalls/ipc/msgstress/msgstress03.c +++ b/testcases/kernel/syscalls/ipc/msgstress/msgstress03.c @@ -109,7 +109,7 @@ int main(int argc, char **argv) } }
- free_pids = tst_get_free_pids(cleanup); + free_pids = tst_get_free_pids(cleanup) / 2; if (nprocs >= free_pids) { tst_resm(TINFO, "Requested number of processes higher than limit (%d > %d), "
There is one script-based test in kernel/syscalls which is test_ioctl. It will run, but the reported status might be incorrect because of missing test framework binaries it depends on. Add the target for those dependencies to the build instructions in the README.
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com --- README.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/README.rst b/README.rst index 4aebadd35..8b6e3a776 100644 --- a/README.rst +++ b/README.rst @@ -97,9 +97,9 @@ Assumptions: *TRIPLE* & *MORELLO_SUPPORT* in `Building Musl`) LTP_BUILD - out-of-tree build path (created by LTP's build.sh script if needed) LTP_INSTALL - destination path where LTP tests are to be installed - TARGETS - build targets, currently only pan and testcases/kernel/syscalls - are supported, it can be further narrowed down for specific syscall - testcases: + TARGETS - build targets, currently only pan, tools/apicmds and + testcases/kernel/syscalls are supported, it can be further narrowed + down for specific syscall testcases: TARGETS="pan testcases/kernel/syscalls/${syscall}" TARGET_FEATURE - morello+c64 (purecap) only: -march=morello+c64
@@ -117,7 +117,7 @@ Assumptions: export CONFIGURE_OPT_EXTRA="--prefix=/ --host=aarch64-linux-gnu --disable-metadata --without-numa"
MAKE_OPTS="TST_NEWER_64_SYSCALL=no TST_COMPAT_16_SYSCALL=no" \ - TARGETS="pan testcases/kernel/syscalls" BUILD_DIR="$LTP_BUILD" \ + TARGETS="pan tools/apicmds testcases/kernel/syscalls" BUILD_DIR="$LTP_BUILD" \ CFLAGS="$CFLAGS" LDFLAGS="$LDFLAGS" \ ./build.sh -t cross -o out -ip "${LTP_INSTALL}"
On Mon, Feb 06, 2023 at 05:54:31PM +0000, Teo Couprie Diaz wrote:
There is one script-based test in kernel/syscalls which is test_ioctl. It will run, but the reported status might be incorrect because of missing test framework binaries it depends on. Add the target for those dependencies to the build instructions in the README.
Apologies for being picky here but could we rephrase the commit message a bit to smth along the lines of: Add 'tools/apicmds' to build targets to satisfy the dependency for those tests that do rely on it, namely test_ioctl, which despite being shell-based one does not follow the usual approach of using the test.sh test library, relying on apicmds binaries instead. This should be fixed at some point though.
?
--- BR B.
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com
README.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/README.rst b/README.rst index 4aebadd35..8b6e3a776 100644 --- a/README.rst +++ b/README.rst @@ -97,9 +97,9 @@ Assumptions: *TRIPLE* & *MORELLO_SUPPORT* in `Building Musl`) LTP_BUILD - out-of-tree build path (created by LTP's build.sh script if needed) LTP_INSTALL - destination path where LTP tests are to be installed
- TARGETS - build targets, currently only pan and testcases/kernel/syscalls
are supported, it can be further narrowed down for specific syscall
testcases:
- TARGETS - build targets, currently only pan, tools/apicmds and
testcases/kernel/syscalls are supported, it can be further narrowed
TARGET_FEATURE - morello+c64 (purecap) only: -march=morello+c64down for specific syscall testcases: TARGETS="pan testcases/kernel/syscalls/${syscall}"
@@ -117,7 +117,7 @@ Assumptions: export CONFIGURE_OPT_EXTRA="--prefix=/ --host=aarch64-linux-gnu --disable-metadata --without-numa" MAKE_OPTS="TST_NEWER_64_SYSCALL=no TST_COMPAT_16_SYSCALL=no" \
- TARGETS="pan testcases/kernel/syscalls" BUILD_DIR="$LTP_BUILD" \
- TARGETS="pan tools/apicmds testcases/kernel/syscalls" BUILD_DIR="$LTP_BUILD" \ CFLAGS="$CFLAGS" LDFLAGS="$LDFLAGS" \ ./build.sh -t cross -o out -ip "${LTP_INSTALL}"
2.25.1
linux-morello-ltp mailing list -- linux-morello-ltp@op-lists.linaro.org To unsubscribe send an email to linux-morello-ltp-leave@op-lists.linaro.org
On 13/02/2023 16:29, Beata Michalska wrote:
On Mon, Feb 06, 2023 at 05:54:31PM +0000, Teo Couprie Diaz wrote:
There is one script-based test in kernel/syscalls which is test_ioctl. It will run, but the reported status might be incorrect because of missing test framework binaries it depends on. Add the target for those dependencies to the build instructions in the README.
Apologies for being picky here but could we rephrase the commit message a bit to smth along the lines of:
No worries, the commit message could clearly be better !
Add 'tools/apicmds' to build targets to satisfy the dependency for those tests that do rely on it, namely test_ioctl, which despite being shell-based one does not follow the usual approach of using the test.sh test library, relying on apicmds binaries instead. This should be fixed at some point though.
?
I mostly agree, but it could be more explicit. What do you think of :
Add 'tools/apicmds' to build targets to satisfy the dependency for the tests that do rely on it. Namely: test_ioctl which, despite being shell-based, does not follow the usual approach of using the test.sh test library, instead relying on the legacy approach of using apicmds binaries. It should be reworked to use the new framework at some point though.
?
Happy to send a new patch separately if you wish, to be honest this shouldn't have been part of the same series anyway. Thanks for the proposed change Téo
BR B.
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com
README.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/README.rst b/README.rst index 4aebadd35..8b6e3a776 100644 --- a/README.rst +++ b/README.rst @@ -97,9 +97,9 @@ Assumptions: *TRIPLE* & *MORELLO_SUPPORT* in `Building Musl`) LTP_BUILD - out-of-tree build path (created by LTP's build.sh script if needed) LTP_INSTALL - destination path where LTP tests are to be installed
- TARGETS - build targets, currently only pan and testcases/kernel/syscalls
are supported, it can be further narrowed down for specific syscall
testcases:
- TARGETS - build targets, currently only pan, tools/apicmds and
testcases/kernel/syscalls are supported, it can be further narrowed
down for specific syscall testcases: TARGETS="pan testcases/kernel/syscalls/${syscall}" TARGET_FEATURE - morello+c64 (purecap) only: -march=morello+c64
@@ -117,7 +117,7 @@ Assumptions: export CONFIGURE_OPT_EXTRA="--prefix=/ --host=aarch64-linux-gnu --disable-metadata --without-numa" MAKE_OPTS="TST_NEWER_64_SYSCALL=no TST_COMPAT_16_SYSCALL=no" \
- TARGETS="pan testcases/kernel/syscalls" BUILD_DIR="$LTP_BUILD" \
- TARGETS="pan tools/apicmds testcases/kernel/syscalls" BUILD_DIR="$LTP_BUILD" \ CFLAGS="$CFLAGS" LDFLAGS="$LDFLAGS" \ ./build.sh -t cross -o out -ip "${LTP_INSTALL}"
2.25.1
linux-morello-ltp mailing list -- linux-morello-ltp@op-lists.linaro.org To unsubscribe send an email to linux-morello-ltp-leave@op-lists.linaro.org
On Mon, Feb 13, 2023 at 04:41:53PM +0000, Teo Couprie Diaz wrote:
On 13/02/2023 16:29, Beata Michalska wrote:
On Mon, Feb 06, 2023 at 05:54:31PM +0000, Teo Couprie Diaz wrote:
There is one script-based test in kernel/syscalls which is test_ioctl. It will run, but the reported status might be incorrect because of missing test framework binaries it depends on. Add the target for those dependencies to the build instructions in the README.
Apologies for being picky here but could we rephrase the commit message a bit to smth along the lines of:
No worries, the commit message could clearly be better !
Add 'tools/apicmds' to build targets to satisfy the dependency for those tests that do rely on it, namely test_ioctl, which despite being shell-based one does not follow the usual approach of using the test.sh test library, relying on apicmds binaries instead. This should be fixed at some point though.
?
I mostly agree, but it could be more explicit. What do you think of :
Add 'tools/apicmds' to build targets to satisfy the dependency for the tests that do rely on it. Namely: test_ioctl which, despite being shell-based, does not follow the usual approach of using the test.sh test library, instead relying on the legacy approach of using apicmds binaries. It should be reworked to use the new framework at some point though.
?
Perfect!
Happy to send a new patch separately if you wish, to be honest this shouldn't have been part of the same series anyway. Thanks for the proposed change
Sounds good. Thanks
--- BR B.
Téo
BR B.
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com
README.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/README.rst b/README.rst index 4aebadd35..8b6e3a776 100644 --- a/README.rst +++ b/README.rst @@ -97,9 +97,9 @@ Assumptions: *TRIPLE* & *MORELLO_SUPPORT* in `Building Musl`) LTP_BUILD - out-of-tree build path (created by LTP's build.sh script if needed) LTP_INSTALL - destination path where LTP tests are to be installed
- TARGETS - build targets, currently only pan and testcases/kernel/syscalls
are supported, it can be further narrowed down for specific syscall
testcases:
- TARGETS - build targets, currently only pan, tools/apicmds and
testcases/kernel/syscalls are supported, it can be further narrowed
down for specific syscall testcases: TARGETS="pan testcases/kernel/syscalls/${syscall}" TARGET_FEATURE - morello+c64 (purecap) only: -march=morello+c64
@@ -117,7 +117,7 @@ Assumptions: export CONFIGURE_OPT_EXTRA="--prefix=/ --host=aarch64-linux-gnu --disable-metadata --without-numa" MAKE_OPTS="TST_NEWER_64_SYSCALL=no TST_COMPAT_16_SYSCALL=no" \
- TARGETS="pan testcases/kernel/syscalls" BUILD_DIR="$LTP_BUILD" \
- TARGETS="pan tools/apicmds testcases/kernel/syscalls" BUILD_DIR="$LTP_BUILD" \ CFLAGS="$CFLAGS" LDFLAGS="$LDFLAGS" \ ./build.sh -t cross -o out -ip "${LTP_INSTALL}"
-- 2.25.1
linux-morello-ltp mailing list -- linux-morello-ltp@op-lists.linaro.org To unsubscribe send an email to linux-morello-ltp-leave@op-lists.linaro.org
On 06/02/2023 18:54, Teo Couprie Diaz wrote:
In some distributions, the two files used in lib/tst_pid.c are not available, but cgroups still imposes a task limit far smaller than the kernel pid_max. If the cgroup sysfs is mounted, we can use it to retrieve the current task limit imposed to the process. Implement the retrieval of this limit.
This can be done by first checking /proc/self/cgroup to get the cgroup the process is in, which will be a path under the cgroup sysfs. To get the path to the cgroup sysfs, check /proc/self/mountinfo. Finally, concatenate those two paths with pid.max to get the full path to the file containing the limit.
This patch changes the way read_session_pids_limit is called, not passing a format string to be completed anymore, but is still used the same way. A following patch will update this function.
This fixes failures for msgstress04.
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com
lib/tst_pid.c | 53 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 10 deletions(-)
Tested the series on my board running Arch Linux (meaning cgroup v2), all msgstress* and ioctl* tests pass as expected. For the whole series:
Tested-by: Kevin Brodsky kevin.brodsky@arm.com
Kevin
diff --git a/lib/tst_pid.c b/lib/tst_pid.c index 21cadef2a..f140acbc0 100644 --- a/lib/tst_pid.c +++ b/lib/tst_pid.c @@ -31,8 +31,6 @@ #include "tst_safe_macros.h" #define PID_MAX_PATH "/proc/sys/kernel/pid_max" -#define CGROUPS_V1_SLICE_FMT "/sys/fs/cgroup/pids/user.slice/user-%d.slice/pids.max" -#define CGROUPS_V2_SLICE_FMT "/sys/fs/cgroup/user.slice/user-%d.slice/pids.max" /* Leave some available processes for the OS */ #define PIDS_RESERVE 50 @@ -96,18 +94,53 @@ static int read_session_pids_limit(const char *path_fmt, int uid, static int get_session_pids_limit(void (*cleanup_fn) (void)) {
- int max_pids, uid;
- char path[PATH_MAX + 1];
- char cgroup_pids[PATH_MAX + 1];
- char catchall;
- int uid, ret = 0;
uid = get_session_uid();
- max_pids = read_session_pids_limit(CGROUPS_V2_SLICE_FMT, uid, cleanup_fn);
- if (max_pids < 0)
max_pids = read_session_pids_limit(CGROUPS_V1_SLICE_FMT, uid,
cleanup_fn);
- /* Check for generic cgroup v1 pid.max */
- ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
"%*d:pids:%s\n", cgroup_pids);
- /*
* This is a bit of a hack of scanf format strings. Indeed, if all
* conversion specifications have been matched the return of scanf will be
* the same whether any outstanding literal characters match or not.
* As we want to match the literal part, we can add a catchall after it
* so that it won't be counted if the literal part doesn't match.
* This makes the macro go to the next line until the catchall, thus
* the literal parts, is matched.
*
* Assume that the root of the mount is '/'. It can be anything,
* but it should be '/' on any normal system.
*/
- if (!ret)
ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo",
"%*s %*s %*s %*s %s %*s %*s - %*s cgroup %*[rw],pid%c",
path, &catchall);
- if (!ret) {
strncat(path, cgroup_pids, PATH_MAX);
strncat(path, "/pids.max", PATH_MAX);
return read_session_pids_limit(path, uid, cleanup_fn);
- }
- if (max_pids < 0)
return -1;
- /* Check for generic cgroup v2 pid.max */
- ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
"%*d::%s\n", cgroup_pids);
- if (!ret)
ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo",
"%*s %*s %*s %*s %s %*s %*s - cgroup2 %c",
path, &catchall);
- if (!ret) {
strncat(path, cgroup_pids, PATH_MAX);
strncat(path, "/pids.max", PATH_MAX);
return read_session_pids_limit(path, uid, cleanup_fn);
- }
- return max_pids;
- return -1;
} int tst_get_free_pids_(void (*cleanup_fn) (void))
On 10/02/2023 12:15, Kevin Brodsky wrote:
On 06/02/2023 18:54, Teo Couprie Diaz wrote:
In some distributions, the two files used in lib/tst_pid.c are not available, but cgroups still imposes a task limit far smaller than the kernel pid_max. If the cgroup sysfs is mounted, we can use it to retrieve the current task limit imposed to the process. Implement the retrieval of this limit.
This can be done by first checking /proc/self/cgroup to get the cgroup the process is in, which will be a path under the cgroup sysfs. To get the path to the cgroup sysfs, check /proc/self/mountinfo. Finally, concatenate those two paths with pid.max to get the full path to the file containing the limit.
This patch changes the way read_session_pids_limit is called, not passing a format string to be completed anymore, but is still used the same way. A following patch will update this function.
This fixes failures for msgstress04.
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com
lib/tst_pid.c | 53 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 10 deletions(-)
Tested the series on my board running Arch Linux (meaning cgroup v2), all msgstress* and ioctl* tests pass as expected. For the whole series:
Tested-by: Kevin Brodsky kevin.brodsky@arm.com
Kevin
Great, thanks a lot for testing with cgroup v2 Kevin !
Cheers, Téo
diff --git a/lib/tst_pid.c b/lib/tst_pid.c index 21cadef2a..f140acbc0 100644 --- a/lib/tst_pid.c +++ b/lib/tst_pid.c @@ -31,8 +31,6 @@ #include "tst_safe_macros.h" #define PID_MAX_PATH "/proc/sys/kernel/pid_max" -#define CGROUPS_V1_SLICE_FMT "/sys/fs/cgroup/pids/user.slice/user-%d.slice/pids.max" -#define CGROUPS_V2_SLICE_FMT "/sys/fs/cgroup/user.slice/user-%d.slice/pids.max" /* Leave some available processes for the OS */ #define PIDS_RESERVE 50 @@ -96,18 +94,53 @@ static int read_session_pids_limit(const char *path_fmt, int uid, static int get_session_pids_limit(void (*cleanup_fn) (void)) {
- int max_pids, uid;
- char path[PATH_MAX + 1];
- char cgroup_pids[PATH_MAX + 1];
- char catchall;
- int uid, ret = 0;
uid = get_session_uid();
- max_pids = read_session_pids_limit(CGROUPS_V2_SLICE_FMT, uid, cleanup_fn);
- if (max_pids < 0)
max_pids = read_session_pids_limit(CGROUPS_V1_SLICE_FMT, uid,
cleanup_fn);
- /* Check for generic cgroup v1 pid.max */
- ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
"%*d:pids:%s\n", cgroup_pids);
- /*
* This is a bit of a hack of scanf format strings. Indeed, if all
* conversion specifications have been matched the return of scanf will be
* the same whether any outstanding literal characters match or not.
* As we want to match the literal part, we can add a catchall after it
* so that it won't be counted if the literal part doesn't match.
* This makes the macro go to the next line until the catchall, thus
* the literal parts, is matched.
*
* Assume that the root of the mount is '/'. It can be anything,
* but it should be '/' on any normal system.
*/
- if (!ret)
ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo",
"%*s %*s %*s %*s %s %*s %*s - %*s cgroup %*[rw],pid%c",
path, &catchall);
- if (!ret) {
strncat(path, cgroup_pids, PATH_MAX);
strncat(path, "/pids.max", PATH_MAX);
return read_session_pids_limit(path, uid, cleanup_fn);
- }
- if (max_pids < 0)
return -1;
- /* Check for generic cgroup v2 pid.max */
- ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
"%*d::%s\n", cgroup_pids);
- if (!ret)
ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo",
"%*s %*s %*s %*s %s %*s %*s - cgroup2 %c",
path, &catchall);
- if (!ret) {
strncat(path, cgroup_pids, PATH_MAX);
strncat(path, "/pids.max", PATH_MAX);
return read_session_pids_limit(path, uid, cleanup_fn);
- }
- return max_pids;
- return -1; }
int tst_get_free_pids_(void (*cleanup_fn) (void))
Hi Teo, Still: s/programatically/programmatically On Mon, Feb 06, 2023 at 05:54:28PM +0000, Teo Couprie Diaz wrote:
In some distributions, the two files used in lib/tst_pid.c are not available, but cgroups still imposes a task limit far smaller than the kernel pid_max. If the cgroup sysfs is mounted, we can use it to retrieve the current task limit imposed to the process. Implement the retrieval of this limit.
This can be done by first checking /proc/self/cgroup to get the cgroup the process is in, which will be a path under the cgroup sysfs. To get the path to the cgroup sysfs, check /proc/self/mountinfo. Finally, concatenate those two paths with pid.max to get the full path to the file containing the limit.
This patch changes the way read_session_pids_limit is called, not passing a format string to be completed anymore, but is still used the same way. A following patch will update this function.
This fixes failures for msgstress04.
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com
lib/tst_pid.c | 53 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 10 deletions(-)
diff --git a/lib/tst_pid.c b/lib/tst_pid.c index 21cadef2a..f140acbc0 100644 --- a/lib/tst_pid.c +++ b/lib/tst_pid.c @@ -31,8 +31,6 @@ #include "tst_safe_macros.h" #define PID_MAX_PATH "/proc/sys/kernel/pid_max" -#define CGROUPS_V1_SLICE_FMT "/sys/fs/cgroup/pids/user.slice/user-%d.slice/pids.max" -#define CGROUPS_V2_SLICE_FMT "/sys/fs/cgroup/user.slice/user-%d.slice/pids.max" /* Leave some available processes for the OS */ #define PIDS_RESERVE 50 @@ -96,18 +94,53 @@ static int read_session_pids_limit(const char *path_fmt, int uid, static int get_session_pids_limit(void (*cleanup_fn) (void)) {
- int max_pids, uid;
- char path[PATH_MAX + 1];
- char cgroup_pids[PATH_MAX + 1];
- char catchall;
- int uid, ret = 0;
uid = get_session_uid();
- max_pids = read_session_pids_limit(CGROUPS_V2_SLICE_FMT, uid, cleanup_fn);
- if (max_pids < 0)
max_pids = read_session_pids_limit(CGROUPS_V1_SLICE_FMT, uid,
cleanup_fn);
- /* Check for generic cgroup v1 pid.max */
- ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
"%*d:pids:%s\n", cgroup_pids);
- /*
* This is a bit of a hack of scanf format strings. Indeed, if all
* conversion specifications have been matched the return of scanf will be
* the same whether any outstanding literal characters match or not.
* As we want to match the literal part, we can add a catchall after it
* so that it won't be counted if the literal part doesn't match.
* This makes the macro go to the next line until the catchall, thus
* the literal parts, is matched.
s/parts/part or s/is matched/are matched
I can make both changes when applying the patch so no need of respin, if you are ok with that.
*
* Assume that the root of the mount is '/'. It can be anything,
* but it should be '/' on any normal system.
*/
- if (!ret)
ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo",
"%*s %*s %*s %*s %s %*s %*s - %*s cgroup %*[rw],pid%c",
path, &catchall);
I think here we need to swap the "%*s cgroup": as per the mountinfo docs: the first filed after the '-' separator is the filesystem type, being cgroup in our case, followed by filesystem specific information, which may or may not be 'cgroup'. While testing cgroup v1 this filed will actually be 'pids' (see below) which means with the current state there will be no match: 23 22 0:21 / /sys/fs/cgroup/pids rw,relatime - cgroup pids rw,pids
I can make that change as well, though I'd like the confirmation from your side before I apply the patches.
--- BR B.
- if (!ret) {
strncat(path, cgroup_pids, PATH_MAX);
strncat(path, "/pids.max", PATH_MAX);
return read_session_pids_limit(path, uid, cleanup_fn);
- }
- if (max_pids < 0)
return -1;
- /* Check for generic cgroup v2 pid.max */
- ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
"%*d::%s\n", cgroup_pids);
- if (!ret)
ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo",
"%*s %*s %*s %*s %s %*s %*s - cgroup2 %c",
path, &catchall);
- if (!ret) {
strncat(path, cgroup_pids, PATH_MAX);
strncat(path, "/pids.max", PATH_MAX);
return read_session_pids_limit(path, uid, cleanup_fn);
- }
- return max_pids;
- return -1;
} int tst_get_free_pids_(void (*cleanup_fn) (void)) -- 2.25.1
linux-morello-ltp mailing list -- linux-morello-ltp@op-lists.linaro.org To unsubscribe send an email to linux-morello-ltp-leave@op-lists.linaro.org
Hi Beata,
On 13/02/2023 15:59, Beata Michalska wrote:
Hi Teo, Still: s/programatically/programmatically
Indeed, I realized after sending the patch. My mistake !
On Mon, Feb 06, 2023 at 05:54:28PM +0000, Teo Couprie Diaz wrote:
In some distributions, the two files used in lib/tst_pid.c are not available, but cgroups still imposes a task limit far smaller than the kernel pid_max. If the cgroup sysfs is mounted, we can use it to retrieve the current task limit imposed to the process. Implement the retrieval of this limit.
This can be done by first checking /proc/self/cgroup to get the cgroup the process is in, which will be a path under the cgroup sysfs. To get the path to the cgroup sysfs, check /proc/self/mountinfo. Finally, concatenate those two paths with pid.max to get the full path to the file containing the limit.
This patch changes the way read_session_pids_limit is called, not passing a format string to be completed anymore, but is still used the same way. A following patch will update this function.
This fixes failures for msgstress04.
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com
lib/tst_pid.c | 53 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 10 deletions(-)
diff --git a/lib/tst_pid.c b/lib/tst_pid.c index 21cadef2a..f140acbc0 100644 --- a/lib/tst_pid.c +++ b/lib/tst_pid.c @@ -31,8 +31,6 @@ #include "tst_safe_macros.h" #define PID_MAX_PATH "/proc/sys/kernel/pid_max" -#define CGROUPS_V1_SLICE_FMT "/sys/fs/cgroup/pids/user.slice/user-%d.slice/pids.max" -#define CGROUPS_V2_SLICE_FMT "/sys/fs/cgroup/user.slice/user-%d.slice/pids.max" /* Leave some available processes for the OS */ #define PIDS_RESERVE 50 @@ -96,18 +94,53 @@ static int read_session_pids_limit(const char *path_fmt, int uid, static int get_session_pids_limit(void (*cleanup_fn) (void)) {
- int max_pids, uid;
- char path[PATH_MAX + 1];
- char cgroup_pids[PATH_MAX + 1];
- char catchall;
- int uid, ret = 0;
uid = get_session_uid();
- max_pids = read_session_pids_limit(CGROUPS_V2_SLICE_FMT, uid, cleanup_fn);
- if (max_pids < 0)
max_pids = read_session_pids_limit(CGROUPS_V1_SLICE_FMT, uid,
cleanup_fn);
- /* Check for generic cgroup v1 pid.max */
- ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
"%*d:pids:%s\n", cgroup_pids);
- /*
* This is a bit of a hack of scanf format strings. Indeed, if all
* conversion specifications have been matched the return of scanf will be
* the same whether any outstanding literal characters match or not.
* As we want to match the literal part, we can add a catchall after it
* so that it won't be counted if the literal part doesn't match.
* This makes the macro go to the next line until the catchall, thus
* the literal parts, is matched.
s/parts/part or s/is matched/are matched
Isn't the subject "the catchall", thus singular would be correct ? I'm really not sure, so if you feel this is wrong and needs to be changed I'm happy for you to do it.
I can make both changes when applying the patch so no need of respin, if you are ok with that.
*
* Assume that the root of the mount is '/'. It can be anything,
* but it should be '/' on any normal system.
*/
- if (!ret)
ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo",
"%*s %*s %*s %*s %s %*s %*s - %*s cgroup %*[rw],pid%c",
path, &catchall);
I think here we need to swap the "%*s cgroup": as per the mountinfo docs: the first filed after the '-' separator is the filesystem type, being cgroup in our case, followed by filesystem specific information, which may or may not be 'cgroup'. While testing cgroup v1 this filed will actually be 'pids' (see below) which means with the current state there will be no match: 23 22 0:21 / /sys/fs/cgroup/pids rw,relatime - cgroup pids rw,pids
Ah, you are absolutely right indeed. Interestingly I don't see the same behavior as you do : both on my laptop and on the Morello board I have `cgroup cgroup`, not `cgroup pids`. But again you are correct the second field could be totally different and flipping to ` - cgroup %*s` is needed.
I can make that change as well, though I'd like the confirmation from your side before I apply the patches.
Happy for you to do it, thanks a lot for the review and catching my inversion ! Téo
BR B.
- if (!ret) {
strncat(path, cgroup_pids, PATH_MAX);
strncat(path, "/pids.max", PATH_MAX);
return read_session_pids_limit(path, uid, cleanup_fn);
- }
- if (max_pids < 0)
return -1;
- /* Check for generic cgroup v2 pid.max */
- ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
"%*d::%s\n", cgroup_pids);
- if (!ret)
ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo",
"%*s %*s %*s %*s %s %*s %*s - cgroup2 %c",
path, &catchall);
- if (!ret) {
strncat(path, cgroup_pids, PATH_MAX);
strncat(path, "/pids.max", PATH_MAX);
return read_session_pids_limit(path, uid, cleanup_fn);
- }
- return max_pids;
- return -1; }
int tst_get_free_pids_(void (*cleanup_fn) (void)) -- 2.25.1
linux-morello-ltp mailing list -- linux-morello-ltp@op-lists.linaro.org To unsubscribe send an email to linux-morello-ltp-leave@op-lists.linaro.org
On Mon, Feb 13, 2023 at 04:24:27PM +0000, Teo Couprie Diaz wrote:
Hi Beata,
On 13/02/2023 15:59, Beata Michalska wrote:
Hi Teo, Still: s/programatically/programmatically
Indeed, I realized after sending the patch. My mistake !
On Mon, Feb 06, 2023 at 05:54:28PM +0000, Teo Couprie Diaz wrote:
In some distributions, the two files used in lib/tst_pid.c are not available, but cgroups still imposes a task limit far smaller than the kernel pid_max. If the cgroup sysfs is mounted, we can use it to retrieve the current task limit imposed to the process. Implement the retrieval of this limit.
This can be done by first checking /proc/self/cgroup to get the cgroup the process is in, which will be a path under the cgroup sysfs. To get the path to the cgroup sysfs, check /proc/self/mountinfo. Finally, concatenate those two paths with pid.max to get the full path to the file containing the limit.
This patch changes the way read_session_pids_limit is called, not passing a format string to be completed anymore, but is still used the same way. A following patch will update this function.
This fixes failures for msgstress04.
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com
lib/tst_pid.c | 53 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 10 deletions(-)
diff --git a/lib/tst_pid.c b/lib/tst_pid.c index 21cadef2a..f140acbc0 100644 --- a/lib/tst_pid.c +++ b/lib/tst_pid.c @@ -31,8 +31,6 @@ #include "tst_safe_macros.h" #define PID_MAX_PATH "/proc/sys/kernel/pid_max" -#define CGROUPS_V1_SLICE_FMT "/sys/fs/cgroup/pids/user.slice/user-%d.slice/pids.max" -#define CGROUPS_V2_SLICE_FMT "/sys/fs/cgroup/user.slice/user-%d.slice/pids.max" /* Leave some available processes for the OS */ #define PIDS_RESERVE 50 @@ -96,18 +94,53 @@ static int read_session_pids_limit(const char *path_fmt, int uid, static int get_session_pids_limit(void (*cleanup_fn) (void)) {
- int max_pids, uid;
- char path[PATH_MAX + 1];
- char cgroup_pids[PATH_MAX + 1];
- char catchall;
- int uid, ret = 0; uid = get_session_uid();
- max_pids = read_session_pids_limit(CGROUPS_V2_SLICE_FMT, uid, cleanup_fn);
- if (max_pids < 0)
max_pids = read_session_pids_limit(CGROUPS_V1_SLICE_FMT, uid,
cleanup_fn);
- /* Check for generic cgroup v1 pid.max */
- ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
"%*d:pids:%s\n", cgroup_pids);
- /*
* This is a bit of a hack of scanf format strings. Indeed, if all
* conversion specifications have been matched the return of scanf will be
* the same whether any outstanding literal characters match or not.
* As we want to match the literal part, we can add a catchall after it
* so that it won't be counted if the literal part doesn't match.
* This makes the macro go to the next line until the catchall, thus
* the literal parts, is matched.
s/parts/part or s/is matched/are matched
Isn't the subject "the catchall", thus singular would be correct ? I'm really not sure, so if you feel this is wrong and needs to be changed I'm happy for you to do it.
It reads to me like matched refers to 'literal parts', not 'catchall' but that's probably just me. Will make the other changes while applying. Thanks!
--- BR B.
I can make both changes when applying the patch so no need of respin, if you are ok with that.
*
* Assume that the root of the mount is '/'. It can be anything,
* but it should be '/' on any normal system.
*/
- if (!ret)
ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo",
"%*s %*s %*s %*s %s %*s %*s - %*s cgroup %*[rw],pid%c",
path, &catchall);
I think here we need to swap the "%*s cgroup": as per the mountinfo docs: the first filed after the '-' separator is the filesystem type, being cgroup in our case, followed by filesystem specific information, which may or may not be 'cgroup'. While testing cgroup v1 this filed will actually be 'pids' (see below) which means with the current state there will be no match: 23 22 0:21 / /sys/fs/cgroup/pids rw,relatime - cgroup pids rw,pids
Ah, you are absolutely right indeed. Interestingly I don't see the same behavior as you do : both on my laptop and on the Morello board I have `cgroup cgroup`, not `cgroup pids`. But again you are correct the second field could be totally different and flipping to ` - cgroup %*s` is needed.
I can make that change as well, though I'd like the confirmation from your side before I apply the patches.
Happy for you to do it, thanks a lot for the review and catching my inversion ! Téo
BR B.
- if (!ret) {
strncat(path, cgroup_pids, PATH_MAX);
strncat(path, "/pids.max", PATH_MAX);
return read_session_pids_limit(path, uid, cleanup_fn);
- }
- if (max_pids < 0)
return -1;
- /* Check for generic cgroup v2 pid.max */
- ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
"%*d::%s\n", cgroup_pids);
- if (!ret)
ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo",
"%*s %*s %*s %*s %s %*s %*s - cgroup2 %c",
path, &catchall);
- if (!ret) {
strncat(path, cgroup_pids, PATH_MAX);
strncat(path, "/pids.max", PATH_MAX);
return read_session_pids_limit(path, uid, cleanup_fn);
- }
- return max_pids;
- return -1; } int tst_get_free_pids_(void (*cleanup_fn) (void))
-- 2.25.1
linux-morello-ltp mailing list -- linux-morello-ltp@op-lists.linaro.org To unsubscribe send an email to linux-morello-ltp-leave@op-lists.linaro.org
linux-morello-ltp@op-lists.linaro.org