[LTP] [PATCH v2] Add test case to cover the setting resource limit64 for process
Petr Vorel
pvorel@suse.cz
Wed Feb 19 17:44:49 CET 2025
Hi Chunfu Wen,
Besides Andrea's comments, there are other things to improve.
> The test ensures that the process gets the correct signals in the correct order:
> First, it should get SIGXCPU after reaching the soft CPU time limit64.
> Then, if the CPU time exceeds the hard limit, it should receive SIGKILL
> Signed-off-by: chunfuwen <chwen@redhat.com>
> ---
> Changes in v2:
> - Romove test descriptions and trailing line as suggested by Ricardo B. Marlière
> - Added 2025 copyright as suggested by Ricardo B. Marlière
> - Trim down include files as suggested by Ricardo B. Marlière
> - Create new lapi/resource.h residing struct rlimit64 as suggested by Andrea
> - Move setrlimit_u64() syscalls definitions into lapi/resource.h as suggested by Andrea
> - Skip SAFE_* variants as suggested by Andrea
> - use tst_buffers when passing the pointeras suggested by Andrea
> - Link to v1:https://lore.kernel.org/all/20250218023107.1208990-1-chwen@redhat.com/
> ---
> include/lapi/resource.h | 26 ++++
> .../kernel/syscalls/setrlimit/setrlimit07.c | 127 ++++++++++++++++++
> 2 files changed, 153 insertions(+)
> create mode 100644 include/lapi/resource.h
> create mode 100644 testcases/kernel/syscalls/setrlimit/setrlimit07.c
> diff --git a/include/lapi/resource.h b/include/lapi/resource.h
> new file mode 100644
> index 000000000..3310bc934
> --- /dev/null
> +++ b/include/lapi/resource.h
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2025 Red Hat Inc. All Rights Reserved.
> + * Author: Chunfu Wen <chwen@redhat.com>
> + */
> +
> +#ifndef LAPI_RESOURCE_H__
> +#define LAPI_RESOURCE_H__
> +
There should be at the top
#include "config.h"
otherwise #ifndef HAVE_STRUCT_RLIMIT64 is always false.
> +#define _GNU_SOURCE
> +
> +#include "lapi/syscalls.h"
This should include some header, I guess:
#include <sys/resource.h>
because there is already a check.
AC_CHECK_TYPES([struct rlimit64],,,[
#define _LARGEFILE64_SOURCE
#include <sys/resource.h>
])
NOTE: we prefer to uses libc headers instead of kernel headers if possible
(e.g. not using <linux/resource.h>) due libc and kernel headers conflicts.
https://sourceware.org/glibc/wiki/Synchronizing_Headers
> +
> +#ifndef HAVE_STRUCT_RLIMIT64
> +struct rlimit64 {
> + uint64_t rlim_cur;
> + uint64_t rlim_max;
> +};
> +#endif
> +
> +static int setrlimit_u64(int resource, const struct rlimit64 *rlim)
> +{
> + return tst_syscall(__NR_prlimit64, 0, resource, rlim, NULL);
I suppose using raw syscall is really needed, right?
C library/kernel ABI differences
Since glibc 2.13, the glibc getrlimit() and setrlimit() wrapper functions no longer invoke the corresponding system calls, but instead employ prlimit(), for the reasons described in BUGS.
The name of the glibc wrapper function is prlimit(); the underlying system call is prlimit64().
man getrlimit(2) says:
https://man7.org/linux/man-pages/man2/getrlimit.2.html
C library/kernel ABI differences
Since glibc 2.13, the glibc getrlimit() and setrlimit() wrapper
functions no longer invoke the corresponding system calls, but
instead employ prlimit(), for the reasons described in BUGS.
The name of the glibc wrapper function is prlimit(); the
underlying system call is prlimit64().
...
Since glibc 2.13, glibc works around the limitations of the
getrlimit() and setrlimit() system calls by implementing
setrlimit() and getrlimit() as wrapper functions that call
prlimit().
> +}
> +
> +#endif /* LAPI_RESOURCE_H__ */
I would probably separate adding the header into it's own commit, but it's ok to
have everything in a single commit.
> diff --git a/testcases/kernel/syscalls/setrlimit/setrlimit07.c b/testcases/kernel/syscalls/setrlimit/setrlimit07.c
> new file mode 100644
> index 000000000..60a4580da
> --- /dev/null
> +++ b/testcases/kernel/syscalls/setrlimit/setrlimit07.c
> @@ -0,0 +1,127 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2025 Red Hat Inc. All Rights Reserved.
> + * Author: Chunfu Wen <chwen@redhat.com>
> + */
> +
> +/*
> + * Set CPU time limit64 for a process and check its behavior
> + * after reaching CPU time limit64.
> + * 1) Process got SIGXCPU after reaching soft limit of CPU time limit64.
> + * 2) Process got SIGKILL after reaching hard limit of CPU time limit64.
> + */
Please use docs like this:
/*\
* Set CPU time limit64 for a process and check its behavior
* after reaching CPU time limit64.
*
* 1) Process got SIGXCPU after reaching soft limit of CPU time limit64.
* 2) Process got SIGKILL after reaching hard limit of CPU time limit64.
*/
/*\ is for docs to be parsed in metadata/ltp.json and included in
https://linux-test-project.readthedocs.io/en/latest/users/test_catalog.html
Extra space between list is needed to 1) and 2) be filtered as a list (not
inline).
Ideally you build the docs yourself
https://linux-test-project.readthedocs.io/en/latest/developers/documentation.html#building-documentation
and check the output in: doc/html/users/test_catalog.html#setrlimit07
> +
> +#define _GNU_SOURCE
> +
> +#include <sys/wait.h>
> +
> +#include "tst_test.h"
> +
> +#include "lapi/syscalls.h"
> +
> +#include "lapi/resource.h"
very nit: please remove blank lines between inludes
> +
> +#ifndef HAVE_STRUCT_RLIMIT64
> +struct rlimit64 {
> + uint64_t rlim_cur;
> + uint64_t rlim_max;
> +};
> +#endif
This is defined in lapi/resource.h which you include, no need to repeat it here.
> +
> +static struct rlimit64*rlim;
> +
> +static int *end;
> +
> +static void sighandler(int sig)
> +{
> + *end = sig;
> +}
> +
> +static void setup(void)
> +{
> + signal(SIGXCPU, sighandler);
> +
> + end = mmap(NULL, sizeof(int), PROT_READ | PROT_WRITE,
> + MAP_SHARED | MAP_ANONYMOUS, -1, 0);
Please use SAFE_MMAP(), otherwise there would have to be check:
if (end == MAP_FAILED)
tst_brk(TBROK | TERRNO, "mmap failed on nfildes");
> +}
> +
> +static void cleanup(void)
> +{
> + if (end)
> + munmap(end, sizeof(int));
We have SAFE_MUNMAP
> +}
> +/*
> +static int setrlimit_u64(int resource, const struct rlimit64 *rlim)
> +{
> + return tst_syscall(__NR_prlimit64, 0, resource, rlim, NULL);
> +}*/
Hm, commented out code, please remove it.
> +
> +static void verify_setrlimit64(void)
> +{
> + int status;
> + pid_t pid;
> +
> + rlim->rlim_cur = 1;
> + rlim->rlim_max = 2;
> +
> + *end = 0;
> +
> + pid = fork();
> + if (!pid) {
> + TEST(setrlimit_u64(RLIMIT_CPU, rlim));
> + if (TST_RET == -1) {
> + tst_res(TFAIL | TTERRNO,
> + "setrlimit_u64(RLIMIT_CPU) failed");
We have TST_EXP_PASS() in tst_test_macros.h, please use it (or other if this is
not appropriate.
> + exit(1);
> + }
> +
> + alarm(20);
> +
> + while (1)
> + ;
Why this?
> + }
> +
> + waitpid(pid, &status, 0);
Maybe SAFE_WAITPID() ?
> +
> + if (WIFEXITED(status) && WEXITSTATUS(status) == 1)
> + return;
> +
> + if (WIFSIGNALED(status)) {
> + if (WTERMSIG(status) == SIGKILL && *end == SIGXCPU) {
> + tst_res(TPASS,
> + "Got SIGXCPU then SIGKILL after reaching both limit");
> + return;
> + }
> +
> + if (WTERMSIG(status) == SIGKILL && !*end) {
> + tst_res(TFAIL,
> + "Got only SIGKILL after reaching both limit");
> + return;
> + }
> +
> + if (WTERMSIG(status) == SIGALRM && *end == SIGXCPU) {
> + tst_res(TFAIL,
> + "Got only SIGXCPU after reaching both limit");
> + return;
> + }
> +
> + if (WTERMSIG(status) == SIGALRM && !*end) {
> + tst_res(TFAIL,
> + "Got no signal after reaching both limit");
> + return;
> + }
> + }
> +
> + tst_res(TFAIL, "Child %s", tst_strstatus(status));
> +}
> +
> +static struct tst_test test = {
> + .test_all = verify_setrlimit64,
> + .bufs = (struct tst_buffers []) {
> + {&rlim, .size = sizeof(*rlim)},
> + {},
+1 for using guarded buffers.
Please fix the indent.
Last, but not least, code style:
$ make check-setrlimit07
CHECK testcases/kernel/syscalls/setrlimit/setrlimit07.c
setrlimit07.c:31: ERROR: "foo*bar" should be "foo *bar"
setrlimit07.c:54: WARNING: Block comments use * on subsequent lines
setrlimit07.c:57: WARNING: Block comments use a trailing */ on a separate line
setrlimit07.c:120: ERROR: code indent should use tabs where possible
setrlimit07.c:120: WARNING: please, no spaces at the start of a line
setrlimit07.c:121: ERROR: code indent should use tabs where possible
setrlimit07.c:121: WARNING: please, no spaces at the start of a line
setrlimit07.c:122: ERROR: code indent should use tabs where possible
setrlimit07.c:122: WARNING: please, no spaces at the start of a line
Kind regards,
Petr
> + },
> + .setup = setup,
> + .cleanup = cleanup,
> + .forks_child = 1,
> +};
More information about the ltp
mailing list