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

[Petr Vorel <pvorel@suse.cz>]

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