| Message ID | 20250507154543.22309-1-mdoucha@suse.cz |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | mmap_24-1: Change vm.max_map_count if needed | expand |
| Context | Check | Description |
|---|---|---|
| ltpci/debian_stable_s390x-linux-gnu-gcc_s390x | success | success |
| ltpci/debian_stable_aarch64-linux-gnu-gcc_arm64 | success | success |
| ltpci/debian_stable_powerpc64le-linux-gnu-gcc_ppc64el | success | success |
| ltpci/debian_stable_gcc | success | success |
| ltpci/debian_stable_gcc | success | success |
| ltpci/ubuntu_bionic_gcc | success | success |
| ltpci/fedora_latest_clang | success | success |
| ltpci/debian_testing_clang | success | success |
| ltpci/debian_testing_gcc | success | success |
| ltpci/opensuse-leap_latest_gcc | success | success |
| ltpci/debian_oldstable_clang | success | success |
| ltpci/quay-io-centos-centos_stream9_gcc | success | success |
| ltpci/alpine_latest_gcc | success | success |
| ltpci/opensuse-archive_42-2_gcc | success | success |
| ltpci/ubuntu_jammy_gcc | success | success |
| ltpci/debian_oldstable_gcc | success | success |
Hi Martin, > If vm.max_map_count system parameter is too high, mmap24-1 may get > killed by OOM. Set the parameter to a reasonable low value so that > mmap() quickly fails as expected. LGTM. Reviewed-by: Petr Vorel <pvorel@suse.cz> The only thing which bothers me is that writing /proc/sys/vm/max_map_count requires root. But I'm not sure if it's worth to add more code to check UID 0. @Li @Cyril: WDYT? Kind regards, Petr
On Thu, May 8, 2025 at 1:53 AM Petr Vorel <pvorel@suse.cz> wrote: > Hi Martin, > > > If vm.max_map_count system parameter is too high, mmap24-1 may get > > killed by OOM. Set the parameter to a reasonable low value so that > > mmap() quickly fails as expected. > > LGTM. > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > The only thing which bothers me is that writing /proc/sys/vm/max_map_count > requires root. But I'm not sure if it's worth to add more code to check > UID 0. > > @Li @Cyril: WDYT? > There are many ways to avoid OOM (e.g., overcommit_memory, oom_score_adj, ulimit, etc.). However, the purpose of the mmap_24-1.c test is to exhaust the virtual address space and trigger ENOMEM. Limiting vm.max_map_count may prevent OOM, but it changes the failure cause to hitting the map count limit, not actual address space exhaustion, which might be away from the test's intent. I guess using setrlimit(RLIMIT_AS, ...) is more appropriate here, as it enforces address space limits while preserving the original test goal.
Hi! > diff --git a/testcases/open_posix_testsuite/conformance/interfaces/mmap/24-1.c b/testcases/open_posix_testsuite/conformance/interfaces/mmap/24-1.c > index 6cc8349e1..91677d289 100644 > --- a/testcases/open_posix_testsuite/conformance/interfaces/mmap/24-1.c > +++ b/testcases/open_posix_testsuite/conformance/interfaces/mmap/24-1.c > @@ -31,12 +31,40 @@ > #include <stdint.h> > #include "posixtest.h" > > +#define MAX_MAP_COUNT_PATH "/proc/sys/vm/max_map_count" > +#define MAP_COUNT_LIMIT 65530 > + > +void proc_write_val(const char *path, size_t val) > +{ > + FILE *procfile; > + > + procfile = fopen(path, "r+"); > + > + if (!procfile) > + return; We should print a message that we failed to open the file for the update here, otherwise it's not clear if we managed to update the limit or not. > + fprintf(procfile, "%zu", val); > + fclose(procfile); > +} > + > int main(void) > { > char tmpfname[256]; > void *pa; > - size_t len; > + size_t len, max_map_count = 0; > int fd; > + FILE *procfile; > + > + /* Change vm.max_map_count to avoid OOM */ > + procfile = fopen(MAX_MAP_COUNT_PATH, "r+"); > + > + if (procfile) { > + fscanf(procfile, "%zu", &max_map_count); > + fclose(procfile); > + } > + > + if (max_map_count > MAP_COUNT_LIMIT) > + proc_write_val(MAX_MAP_COUNT_PATH, MAP_COUNT_LIMIT); > > /* Size of the shared memory object */ > size_t shm_size = 1024; > @@ -78,5 +106,10 @@ int main(void) > > close(fd); > printf("Test FAILED: Did not get ENOMEM as expected\n"); > + > + /* Restore original vm.max_map_count */ > + if (max_map_count > MAP_COUNT_LIMIT) > + proc_write_val(MAX_MAP_COUNT_PATH, max_map_count); Otherwise it looks good: Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
On 07. 05. 25 19:53, Petr Vorel wrote: > Hi Martin, > >> If vm.max_map_count system parameter is too high, mmap24-1 may get >> killed by OOM. Set the parameter to a reasonable low value so that >> mmap() quickly fails as expected. > > LGTM. > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > The only thing which bothers me is that writing /proc/sys/vm/max_map_count > requires root. But I'm not sure if it's worth to add more code to check UID 0. We don't need a check, the test will simply fail to open the file and move on to the main test case. But I'll add an info message if that happens.
> On 07. 05. 25 19:53, Petr Vorel wrote: > > Hi Martin, > > > If vm.max_map_count system parameter is too high, mmap24-1 may get > > > killed by OOM. Set the parameter to a reasonable low value so that > > > mmap() quickly fails as expected. > > LGTM. > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > The only thing which bothers me is that writing /proc/sys/vm/max_map_count > > requires root. But I'm not sure if it's worth to add more code to check UID 0. > We don't need a check, the test will simply fail to open the file and move > on to the main test case. But I'll add an info message if that happens. Testing without root with too high /proc/sys/vm/max_map_count will fail due not able to lower down it. But sure, explaining message is IMHO good enough, thanks! Kind regards, Petr
On 09. 05. 25 6:03, Li Wang wrote: > There aremany ways toavoid OOM (e.g., overcommit_memory, oom_score_adj, > ulimit, etc.). However, the purpose of the mmap_24-1.c test is to > exhaust thevirtual > address space and trigger ENOMEM. > > Limiting vm.max_map_count may prevent OOM, but it changes the failure > cause to > hitting themap count limit, not actual address space exhaustion, which > might be away > from thetest's intent. > > Iguess using setrlimit(RLIMIT_AS, ...) is more appropriate here, as it > enforces address > space limits while preserving the original test goal. 64bit address space is so large that exhausting it requires 512+GB of page table structures alone (for the 48bit variant). And kernel will not return ENOMEM when it runs out of free memory for pagetable structures, it'll trigger OOM. The mmap_24-1 test started failing after the default vm.max_map_count increased in one of our tested product so this is the limit we want to change here. But you could copy the test and change RLIMIT_AS there. That would also be a valid test case.
Martin Doucha <mdoucha@suse.cz> wrote: > On 09. 05. 25 6:03, Li Wang wrote: > > There aremany ways toavoid OOM (e.g., overcommit_memory, oom_score_adj, > > ulimit, etc.). However, the purpose of the mmap_24-1.c test is to > > exhaust thevirtual > > address space and trigger ENOMEM. > > > > Limiting vm.max_map_count may prevent OOM, but it changes the failure > > cause to > > hitting themap count limit, not actual address space exhaustion, which > > might be away > > from thetest's intent. > > > > Iguess using setrlimit(RLIMIT_AS, ...) is more appropriate here, as it > > enforces address > > space limits while preserving the original test goal. > > 64bit address space is so large that exhausting it requires 512+GB of > page table structures alone (for the 48bit variant). And kernel will not > return ENOMEM when it runs out of free memory for pagetable structures, > it'll trigger OOM. Indeed. > > The mmap_24-1 test started failing after the default vm.max_map_count > increased in one of our tested product so this is the limit we want to That suggests the issue is actually due to the process reaching the vm.max_map_count limit, which results in ENOMEM errors in your regularly tested product. Give the fact, I agree with you patch, it addresses the root cause effectively. But I recommend updating the code comments to reflect that the failure comes from exceeding the maximum number of memory mappings, not overall memory exhaustion. > change here. But you could copy the test and change RLIMIT_AS there. > That would also be a valid test case. > > -- > Martin Doucha mdoucha@suse.cz > SW Quality Engineer > SUSE LINUX, s.r.o. > CORSO IIa > Krizikova 148/34 > 186 00 Prague 8 > Czech Republic >
diff --git a/testcases/open_posix_testsuite/conformance/interfaces/mmap/24-1.c b/testcases/open_posix_testsuite/conformance/interfaces/mmap/24-1.c index 6cc8349e1..91677d289 100644 --- a/testcases/open_posix_testsuite/conformance/interfaces/mmap/24-1.c +++ b/testcases/open_posix_testsuite/conformance/interfaces/mmap/24-1.c @@ -31,12 +31,40 @@ #include <stdint.h> #include "posixtest.h" +#define MAX_MAP_COUNT_PATH "/proc/sys/vm/max_map_count" +#define MAP_COUNT_LIMIT 65530 + +void proc_write_val(const char *path, size_t val) +{ + FILE *procfile; + + procfile = fopen(path, "r+"); + + if (!procfile) + return; + + fprintf(procfile, "%zu", val); + fclose(procfile); +} + int main(void) { char tmpfname[256]; void *pa; - size_t len; + size_t len, max_map_count = 0; int fd; + FILE *procfile; + + /* Change vm.max_map_count to avoid OOM */ + procfile = fopen(MAX_MAP_COUNT_PATH, "r+"); + + if (procfile) { + fscanf(procfile, "%zu", &max_map_count); + fclose(procfile); + } + + if (max_map_count > MAP_COUNT_LIMIT) + proc_write_val(MAX_MAP_COUNT_PATH, MAP_COUNT_LIMIT); /* Size of the shared memory object */ size_t shm_size = 1024; @@ -78,5 +106,10 @@ int main(void) close(fd); printf("Test FAILED: Did not get ENOMEM as expected\n"); + + /* Restore original vm.max_map_count */ + if (max_map_count > MAP_COUNT_LIMIT) + proc_write_val(MAX_MAP_COUNT_PATH, max_map_count); + return PTS_FAIL; }
If vm.max_map_count system parameter is too high, mmap24-1 may get killed by OOM. Set the parameter to a reasonable low value so that mmap() quickly fails as expected. Signed-off-by: Martin Doucha <mdoucha@suse.cz> --- .../conformance/interfaces/mmap/24-1.c | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-)