[LTP] [PATCH v4 0/4] syscalls/msgstress: fixes for small systems

[LTP] [PATCH v4 0/4] syscalls/msgstress: fixes for small systems

[Krzysztof Kozlowski]

Hi,

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 (4):
  include/tst_pid.h: fix language typo (subtraction)
  tst_pid.c: fix fork failure on small memory systems
  syscalls/msgstress03: fix fork failure on small memory systems
  syscalls/msgstress: tune limit of processes for small machines

 include/tst_pid.h                             |  6 +-
 lib/tst_pid.c                                 | 72 ++++++++++++++++++-
 .../syscalls/ipc/msgstress/msgstress03.c      | 15 +++-
 3 files changed, 89 insertions(+), 4 deletions(-)

-- 
2.27.0

[LTP] [PATCH v4 1/4] include/tst_pid.h: fix language typo (subtraction)

[Krzysztof Kozlowski]

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 include/tst_pid.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/tst_pid.h b/include/tst_pid.h
index 9ba1abb27b7c..6c42f73a57e7 100644
--- a/include/tst_pid.h
+++ b/include/tst_pid.h
@@ -13,7 +13,7 @@
 pid_t tst_get_unused_pid_(void (*cleanup_fn)(void));
 
 /*
- * Returns number of free pids by substarction of the number of pids
+ * Returns number of free pids by subtraction of the number of pids
  * currently used ('ps -eT') from max_pids
  */
 int tst_get_free_pids_(void (*cleanup_fn)(void));
-- 
2.27.0

[LTP] [PATCH v4 2/4] tst_pid.c: fix fork failure on small memory systems

[Krzysztof Kozlowski]

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

    Fork failure in the first child of child group 4396
    Fork failure in the second child of child group 4413
    msgstress04    1  TFAIL  :  msgstress04.c:222: Child exit status = 1

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.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 include/tst_pid.h |  4 ++-
 lib/tst_pid.c     | 70 ++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/include/tst_pid.h b/include/tst_pid.h
index 6c42f73a57e7..8d999a655f1a 100644
--- a/include/tst_pid.h
+++ b/include/tst_pid.h
@@ -14,7 +14,9 @@ pid_t tst_get_unused_pid_(void (*cleanup_fn)(void));
 
 /*
  * Returns number of free pids by subtraction of the number of pids
- * currently used ('ps -eT') from max_pids
+ * currently used ('ps -eT') from maximum number of processes.
+ * The limit of processes come from kernel pid_max and cgroup session limits
+ * (e.g. configured by systemd user.slice).
  */
 int tst_get_free_pids_(void (*cleanup_fn)(void));
 
diff --git a/lib/tst_pid.c b/lib/tst_pid.c
index 9568cc9e91d2..c408172675a7 100644
--- a/lib/tst_pid.c
+++ b/lib/tst_pid.c
@@ -18,14 +18,20 @@
  *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
+#include <stdio.h>
+#include <stdlib.h>
 #include <sys/types.h>
+#include <unistd.h>
 #include "test.h"
 #include "tst_pid.h"
 #include "old_safe_file_ops.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"
 
 pid_t tst_get_unused_pid_(void (*cleanup_fn) (void))
 {
@@ -36,10 +42,68 @@ 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)
+{
+	const char *sudo_uid;
+
+	sudo_uid = getenv("SUDO_UID");
+	if (sudo_uid) {
+		unsigned int real_uid;
+		int ret;
+
+		ret = sscanf(sudo_uid, "%u", &real_uid);
+		if (ret == 1)
+			return real_uid;
+	}
+
+	return getuid();
+}
+
+static int read_session_pids_limit(const char *path_fmt, int uid,
+				   void (*cleanup_fn) (void))
+{
+	int max_pids, ret;
+	char path[PATH_MAX];
+
+	ret = snprintf(path, sizeof(path), path_fmt, uid);
+	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;
+	}
+
+	SAFE_FILE_SCANF(cleanup_fn, path, "%d", &max_pids);
+	tst_resm(TINFO, "Found limit of processes %d (from %s)", max_pids, path);
+
+	return max_pids;
+}
+
+static int get_session_pids_limit(void (*cleanup_fn) (void))
+{
+	int max_pids, uid;
+
+	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);
+
+	if (max_pids < 0)
+		return -1;
+
+	return max_pids;
+}
+
 int tst_get_free_pids_(void (*cleanup_fn) (void))
 {
 	FILE *f;
-	int rc, used_pids, max_pids;
+	int rc, used_pids, max_pids, max_session_pids;
 
 	f = popen("ps -eT | wc -l", "r");
 	if (!f) {
@@ -57,6 +121,10 @@ int tst_get_free_pids_(void (*cleanup_fn) (void))
 
 	SAFE_FILE_SCANF(cleanup_fn, PID_MAX_PATH, "%d", &max_pids);
 
+	max_session_pids = get_session_pids_limit(cleanup_fn);
+	if ((max_session_pids > 0) && (max_session_pids < max_pids))
+		max_pids = max_session_pids;
+
 	/* max_pids contains the maximum PID + 1,
 	 * used_pids contains used PIDs + 1,
 	 * so this additional '1' is eliminated by the substraction */
-- 
2.27.0

[LTP] [PATCH v4 3/4] 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>
---
 .../kernel/syscalls/ipc/msgstress/msgstress03.c   | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/ipc/msgstress/msgstress03.c b/testcases/kernel/syscalls/ipc/msgstress/msgstress03.c
index 294b401b1b38..18e50e35ee07 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,19 @@ int main(int argc, char **argv)
 		}
 	}
 
+	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");
+	}
+	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.27.0

[LTP] [PATCH v4 4/4] 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
500) 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 c408172675a7..338235f13a6a 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 500
 
 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.27.0

[LTP] [PATCH v4 0/4] syscalls/msgstress: fixes for small systems

[Krzysztof Kozlowski]

On 29/06/2021 14:10, Krzysztof Kozlowski wrote:
> Hi,
> 
> 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.


Any comments on this set?

> 
> Krzysztof Kozlowski (4):
>   include/tst_pid.h: fix language typo (subtraction)
>   tst_pid.c: fix fork failure on small memory systems
>   syscalls/msgstress03: fix fork failure on small memory systems
>   syscalls/msgstress: tune limit of processes for small machines
> 
>  include/tst_pid.h                             |  6 +-
>  lib/tst_pid.c                                 | 72 ++++++++++++++++++-
>  .../syscalls/ipc/msgstress/msgstress03.c      | 15 +++-
>  3 files changed, 89 insertions(+), 4 deletions(-)
> 


Best regards,
Krzysztof

[LTP] [PATCH v4 0/4] syscalls/msgstress: fixes for small systems

[Cyril Hrubis]

Hi!
> Any comments on this set?

Sorry for the delay. I've pushed the first two patches, I will comment
the rest.

-- 
Cyril Hrubis
chrubis@suse.cz

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

[Cyril Hrubis]

Hi!
> +	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");
> +	}

This looks like copy&paste from msgstress04.c, can we please move this
snippet to the library function instead?

> +	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.27.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v4 4/4] 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
> interfere with the test.  Instead leave some reserve (hard-coded to
> 500) for the OS so the test won't fail on fork failure.

Isn't 500 far too many? I would expect that 50 would be more than
enough.

-- 
Cyril Hrubis
chrubis@suse.cz

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

[Cyril Hrubis]

Hi!
And we do have a patchset that converts msgstress* to new library as
well which merges msgstress03 and msgstress01. I will try to have a look
into that as well.

http://patchwork.ozlabs.org/project/ltp/list/?series=233661

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v4 4/4] syscalls/msgstress: tune limit of processes for small machines

[Krzysztof Kozlowski]

On 19/08/2021 18:00, 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
>> interfere with the test.  Instead leave some reserve (hard-coded to
>> 500) for the OS so the test won't fail on fork failure.
> 
> Isn't 500 far too many? I would expect that 50 would be more than
> enough.

I wanted to stay on the safe side to avoid false failures especially
that even with 500 reserve there will be plenty of PIDs to stress the
system.

I can change it to 50.


Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 19/08/2021 17:58, Cyril Hrubis wrote:
> Hi!
>> +	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");
>> +	}
> 
> This looks like copy&paste from msgstress04.c, can we please move this
> snippet to the library function instead?
> 

I can move it.


Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 19/08/2021 18:02, Cyril Hrubis wrote:
> Hi!
> And we do have a patchset that converts msgstress* to new library as
> well which merges msgstress03 and msgstress01. I will try to have a look
> into that as well.
> 
> http://patchwork.ozlabs.org/project/ltp/list/?series=233661

The question then would be who should rebase on whose work :)


Best regards,
Krzysztof

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

[Cyril Hrubis]

Hi!
> > And we do have a patchset that converts msgstress* to new library as
> > well which merges msgstress03 and msgstress01. I will try to have a look
> > into that as well.
> > 
> > http://patchwork.ozlabs.org/project/ltp/list/?series=233661
> 
> The question then would be who should rebase on whose work :)

Given that the patchset is not ready, I will push your fixes first.

-- 
Cyril Hrubis
chrubis@suse.cz