[LTP] [PATCH v5 0/3] syscalls/msgstress: fixes for small systems

[LTP] [PATCH v5 0/3] syscalls/msgstress: fixes for small systems

[Krzysztof Kozlowski]

Hi,

Changes since v4:
1. Drop applied patches.
2. Add patch 1/3 moving code to lib.
3. Change limit 500->50 in patch 3/3.

Changes since v3:
1. Adjust subject of patch 2/4.
2. Don't use TEST() macro in 2/4.
3. Remove debug messages in 2/4.
4. Return unsigned int in get_session_uid() as it cannot fail in 2/4.

Changes since v2:
1. Resend due to messed up previous submission.

Changes since v1:
1. Move the code reading cgroups session limit to lib/tst_pid.c to
   existing tst_get_free_pids_().
2. Allow reading session limits from cgroups v2.
3. Add patch 1/4 - typo fix.
4. Add patch 4/4 with the buffer/reserve of pids.

Best regards,
Krzysztof

Krzysztof Kozlowski (3):
  lib/tst_pid: simplify error handling by callers of tst_get_free_pids()
  syscalls/msgstress03: fix fork failure on small memory systems
  syscalls/msgstress: tune limit of processes for small machines

 lib/tst_pid.c                                       | 13 +++++++++----
 .../kernel/syscalls/ipc/msgstress/msgstress03.c     | 10 +++++++++-
 .../kernel/syscalls/ipc/msgstress/msgstress04.c     |  6 ------
 3 files changed, 18 insertions(+), 11 deletions(-)

-- 
2.30.2

[LTP] [PATCH v5 1/3] lib/tst_pid: simplify error handling by callers of tst_get_free_pids()

[Krzysztof Kozlowski]

Handle errors of getting free pids directly in the library instead of
tst_get_free_pids() caller.

Suggested-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 lib/tst_pid.c                                         | 9 ++++++---
 testcases/kernel/syscalls/ipc/msgstress/msgstress04.c | 6 ------
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/lib/tst_pid.c b/lib/tst_pid.c
index c408172675a7..23753988ca57 100644
--- a/lib/tst_pid.c
+++ b/lib/tst_pid.c
@@ -107,15 +107,14 @@ int tst_get_free_pids_(void (*cleanup_fn) (void))
 
 	f = popen("ps -eT | wc -l", "r");
 	if (!f) {
-		tst_resm(TBROK, "Could not run 'ps' to calculate used " "pids");
+		tst_brkm(TBROK, cleanup_fn, "Could not run 'ps' to calculate used pids");
 		return -1;
 	}
 	rc = fscanf(f, "%i", &used_pids);
 	pclose(f);
 
 	if (rc != 1 || used_pids < 0) {
-		tst_resm(TBROK, "Could not read output of 'ps' to "
-			 "calculate used pids");
+		tst_brkm(TBROK, cleanup_fn, "Could not read output of 'ps' to calculate used pids");
 		return -1;
 	}
 
@@ -128,5 +127,9 @@ int tst_get_free_pids_(void (*cleanup_fn) (void))
 	/* max_pids contains the maximum PID + 1,
 	 * used_pids contains used PIDs + 1,
 	 * so this additional '1' is eliminated by the substraction */
+	if (used_pids >= max_pids) {
+		tst_brkm(TBROK, cleanup_fn, "No free pids");
+		return 0;
+	}
 	return max_pids - used_pids;
 }
diff --git a/testcases/kernel/syscalls/ipc/msgstress/msgstress04.c b/testcases/kernel/syscalls/ipc/msgstress/msgstress04.c
index f1c124990cb1..b9ebf9035c6d 100644
--- a/testcases/kernel/syscalls/ipc/msgstress/msgstress04.c
+++ b/testcases/kernel/syscalls/ipc/msgstress/msgstress04.c
@@ -413,12 +413,6 @@ void setup(void)
 	tst_resm(TINFO, "Found %d available message queues", MSGMNI);
 
 	free_pids = tst_get_free_pids(cleanup);
-	if (free_pids < 0) {
-		tst_brkm(TBROK, cleanup, "Can't obtain free_pid count");
-	} else if (!free_pids) {
-		tst_brkm(TBROK, cleanup, "No free pids");
-	}
-
 	/* We don't use more than a half of available pids.
 	 * For each child we fork up to 2*maxnkids grandchildren. */
 	maxnprocs = (free_pids / 2) / (1 + 2 * maxnkids);
-- 
2.30.2

[LTP] [PATCH v5 2/3] syscalls/msgstress03: fix fork failure on small memory systems

[Krzysztof Kozlowski]

Running syscalls/msgstress03 on a system with less than ~4 GB of RAM fails:

    msgstress03    1  TFAIL  :  msgstress03.c:155: 	Fork failed (may be OK if under stress)

In dmesg:

    LTP: starting msgstress03
    cgroup: fork rejected by pids controller in /user.slice/user-1000.slice/session-1.scope

The reason is cgroups pid limit set by systemd user.slice.  The limit is
set for login session, also for root user.  For example on 2 GB RAM
machine it is set as:
    /sys/fs/cgroup/pids/user.slice/user-0.slice/pids.max:5207

Read the maximum number of pids and adjust the test limit.  For 2 GB RAM
machine with systemd this will result in:

    msgstress03    0  TINFO  :  Found limit of processes 5056 (from /sys/fs/cgroup/pids/user.slice/user-1000.slice/pids.max)
    msgstress03    0  TINFO  :  Requested number of processes higher than user session limit (10000 > 4556), setting to 4556

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 testcases/kernel/syscalls/ipc/msgstress/msgstress03.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/ipc/msgstress/msgstress03.c b/testcases/kernel/syscalls/ipc/msgstress/msgstress03.c
index 294b401b1b38..3cb70ab18a80 100644
--- a/testcases/kernel/syscalls/ipc/msgstress/msgstress03.c
+++ b/testcases/kernel/syscalls/ipc/msgstress/msgstress03.c
@@ -78,7 +78,7 @@ static void usage(void)
 
 int main(int argc, char **argv)
 {
-	int i, j, ok, pid;
+	int i, j, ok, pid, free_pids;
 	int count, status;
 	struct sigaction act;
 
@@ -109,6 +109,14 @@ int main(int argc, char **argv)
 		}
 	}
 
+	free_pids = tst_get_free_pids(cleanup);
+	if (nprocs >= free_pids) {
+		tst_resm(TINFO,
+			 "Requested number of processes higher than limit (%d > %d), "
+			 "setting to %d", nprocs, free_pids, free_pids);
+		nprocs = free_pids;
+	}
+
 	srand(getpid());
 	tid = -1;
 
-- 
2.30.2

[LTP] [PATCH v5 3/3] syscalls/msgstress: tune limit of processes for small machines

[Krzysztof Kozlowski]

Forking the exactly amount of processes as the limit (either from
max_pids or from cgroups) is risky - OS might be doing some work and
interfere with the test.  Instead leave some reserve (hard-coded
to 50) for the OS so the test won't fail on fork failure.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 lib/tst_pid.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/tst_pid.c b/lib/tst_pid.c
index 23753988ca57..435a0c63af34 100644
--- a/lib/tst_pid.c
+++ b/lib/tst_pid.c
@@ -32,6 +32,8 @@
 #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
 
 pid_t tst_get_unused_pid_(void (*cleanup_fn) (void))
 {
@@ -97,7 +99,7 @@ static int get_session_pids_limit(void (*cleanup_fn) (void))
 	if (max_pids < 0)
 		return -1;
 
-	return max_pids;
+	return max_pids > PIDS_RESERVE ? max_pids - PIDS_RESERVE : 0;
 }
 
 int tst_get_free_pids_(void (*cleanup_fn) (void))
-- 
2.30.2

[LTP] [PATCH v5 3/3] syscalls/msgstress: tune limit of processes for small machines

[Cyril Hrubis]

Hi!
> Forking the exactly amount of processes as the limit (either from
> max_pids or from cgroups) is risky - OS might be doing some work and

This comment does not seem to match the code. We apply the reserve to
the cgroup limit only. If we wanted to make sure that the reserve is
applied even in the case where there is no systemd cgroup we would have
to apply it in the tst_get_free_pids_() just before we check if
used_pids >= max_pids instead.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v5 0/3] syscalls/msgstress: fixes for small systems

[Cyril Hrubis]

Hi!
First two patches pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v5 3/3] syscalls/msgstress: tune limit of processes for small machines

[Krzysztof Kozlowski]

On 20/08/2021 15:34, Cyril Hrubis wrote:
> Hi!
>> Forking the exactly amount of processes as the limit (either from
>> max_pids or from cgroups) is risky - OS might be doing some work and
> 
> This comment does not seem to match the code. We apply the reserve to
> the cgroup limit only. If we wanted to make sure that the reserve is
> applied even in the case where there is no systemd cgroup we would have
> to apply it in the tst_get_free_pids_() just before we check if
> used_pids >= max_pids instead.

You are right. I'll move it to tst_get_free_pids_ to handle such case.


Best regards,
Krzysztof