Re: [LTP] [PATCH v3] syscalls/mmap14: Rewrite test using new LTP API

Petr Vorel
2024-03-06T11:25:24Z

Hi Avinesh,

> diff --git a/testcases/kernel/syscalls/mmap/mmap14.c b/testcases/kernel/syscalls/mmap/mmap14.c
...
> -/*
> - * Test Description:
> - * Verify MAP_LOCKED works fine.
> - * "Lock the pages of the mapped region into memory in the manner of mlock(2)."
> +/*\
> + * [Description]
> *
> - * Expected Result:
> - * mmap() should succeed returning the address of the mapped region,
> - * and this region should be locked into memory.
> + * Verify that, mmap() call with MAP_LOCKED flag successfully locks
> + * the mapped pages into memory.
> */
> -#include <stdio.h>
> -#include <sys/mman.h>

> -#include "test.h"
> +#include <stdio.h>
> +#include "tst_test.h"

> -#define TEMPFILE "mmapfile"
> -#define MMAPSIZE (1UL<<20)
> +#define MMAPSIZE (1UL<<22)
I wonder why it was needed to increase the map size.
Anyway, even with 1UL<<20 it's too high for old SLES 15-SP1.

ulimit -l prints 64 (kB => 65536) ...

> +static void setup(void)
> {
> + SAFE_GETRLIMIT(RLIMIT_MEMLOCK, &rlim);
...
> + if (((sz_before * 1024) + MMAPSIZE) > rlim.rlim_cur)
> + tst_brk(TCONF, "Trying to exceed RLIMIT_MEMLOCK limit");
... therefore it TCONF here in the setup function.

But disabling this check it runs OK. I would expect we would get EAGAIN
(according to man "The file has been locked, or too much memory has been
locked") or MAP_FAILED or ENOMEM (but man says: "This implementation will try to
populate (prefault) the whole range but the mmap() call doesn't fail with ENOMEM
if this fails.")

Just experimenting, to increase to 1UL<<40 I get ENOMEM, doing really too huge
1UL<<200 I get EINVAL.

Therefore I would say the check in this way is wrong.
Or, could we just run SAFE_SETRLIMIT(RLIMIT_MEMLOCK, ...) in case of low limit
and increase it in cleanup?

> }

> void getvmlck(unsigned int *lock_sz)
> {
> - int ret;
> char line[LINELEN];
> - FILE *fstatus = NULL;
> + FILE *fp = NULL;

> - fstatus = fopen("/proc/self/status", "r");
> - if (fstatus == NULL)
> - tst_brkm(TFAIL | TERRNO, NULL, "Open dev status failed");
> + fp = fopen("/proc/self/status", "r");
SAFE_FOPEN() (requires #include "tst_safe_stdio.h") and 2 lines below aren't
needed.

> + if (fp == NULL)
> + tst_brk(TFAIL | TERRNO, "could not open status file");

> - while (fgets(line, LINELEN, fstatus) != NULL)
> + while (fgets(line, LINELEN, fp) != NULL) {
> if (strstr(line, "VmLck") != NULL)
> break;
> + }

> - ret = sscanf(line, "%*[^0-9]%d%*[^0-9]", lock_sz);
> - if (ret != 1)
> - tst_brkm(TFAIL | TERRNO, NULL, "Get lock size failed");
> + if (sscanf(line, "%*[^0-9]%d%*[^0-9]", lock_sz) != 1)
> + tst_brk(TFAIL | TERRNO, "Getting locked memory size failed");

I would say this should be tst_brk(TFAIL | TERRNO) (it's not directly related to
the testing, more to the preparation). Also I hope Andrea Manzini will add his
first easyhack for SAFE_SSCANF() and he will use it :).

The rest LGTM.

Kind regards,
Petr

--
Mailing list info: https://lists.linux.it/listinfo/ltp