Re: [LTP] [PATCH v2 2/3] hugemmap10: Add private MAP_NORESERVE test mode

[Li Wang <li.wang@linux.dev>]

Huaisheng Ye wrote:

> Add -n to apply MAP_NORESERVE only to private mappings. Shared
> mappings retain their original flags and reservation behavior.
> 
> Use the private mapping flags for both the reservation probe and test
> mappings. As a result, private_resv and its TDEBUG report describe whether
> the private mappings actually in use create a hugetlb reservation at
> mmap() time. This is true both for the kernel default and when -n forces
> fault-time reservations.
> 
> The option lets the test exercise the fault-time private reservation path
> used by upstream kernels without booting an older kernel.

The new mode is useful, but it should also be added to runtest/hugetlb,
otherwise the fault-time path is never exercised by the default test run:

--- a/runtest/hugetlb
+++ b/runtest/hugetlb
@@ -13,6 +13,7 @@ hugemmap07 hugemmap07
 hugemmap08 hugemmap08
 hugemmap09 hugemmap09
 hugemmap10 hugemmap10
+hugemmap10 hugemmap10 -n

> 
> Signed-off-by: Huaisheng Ye <yehuaisheng@open-hieco.net>
> ---
>  .../kernel/mem/hugetlb/hugemmap/hugemmap10.c  | 28 +++++++++++++++++--
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap10.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap10.c
> index b22e11f41..1a5b0fd2f 100644
> --- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap10.c
> +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap10.c
> @@ -31,6 +31,8 @@
>  
>  static long hpage_size;
>  static int private_resv;
> +static int private_noreserve;

The private_noreserve variable looks redundant. We can simply use
private_noreserve_opt directly, since it is already NULL when -n is
not given and non-NULL when it is.

	if (private_noreserve_opt && (flags & MAP_TYPE) == MAP_PRIVATE)
		return flags | MAP_NORESERVE;

> +static char *private_noreserve_opt;
>  
>  #define NR_SLOTS	2
>  #define SL_SETUP	0
> @@ -90,6 +92,16 @@ static int mapping_creates_reservation(int flags)
>  	return (flags & MAP_SHARED) || private_resv;
>  }
>  
> +/* Exercise the fault-time reservation path used by older kernels. */
> +static int private_mmap_flags(int flags)
> +{
> +	if (private_noreserve &&
> +	    (flags & MAP_TYPE) == MAP_PRIVATE)
> +		return flags | MAP_NORESERVE;
> +
> +	return flags;
> +}

We need to add the MAP_TYPE definition to lapi/mmap.h to avoid
compilation failures on systems where glibc (<sys/mman.h>)
doesn't support it.

#ifndef MAP_TYPE
#define MAP_TYPE 0x0f
#endif

> +
>  static unsigned long read_allowed_free_hugepages(void)
>  {
>  	char path[PATH_MAX];
> @@ -155,7 +167,8 @@ static int kernel_has_private_reservations(void)
>  	read_meminfo_huge(&t, &f, &r, &s);
>  	fd = tst_creat_unlinked(MNTPOINT, 0, 0600);
>  
> -	p = SAFE_MMAP(NULL, hpage_size, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0);
> +	p = SAFE_MMAP(NULL, hpage_size, PROT_READ|PROT_WRITE,
> +		      private_mmap_flags(MAP_PRIVATE), fd, 0);
>  
>  	read_meminfo_huge(&nt, &nf, &nr, &ns);
>  
> @@ -283,8 +296,9 @@ static int map_(int s, int hpages, int flags, char *desc, int line)
>  	map_size[s] = hpages * hpage_size;
>  	if (creates_reservation)
>  		allowed_free = read_allowed_free_hugepages();
> -	map_addr[s] = SAFE_MMAP(NULL, map_size[s], PROT_READ|PROT_WRITE, flags,
> -				map_fd[s], 0);
> +	map_addr[s] = SAFE_MMAP(NULL, map_size[s], PROT_READ|PROT_WRITE,
> +			private_mmap_flags(flags),
> +			map_fd[s], 0);
>  	touched[s] = 0;
>  
>  	et = prev_total;
> @@ -533,7 +547,10 @@ static void setup(void)
>  {
>  	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
>  	SAFE_FILE_PRINTF(PATH_VM_OVERCOMMIT_HPAGES, "%lu", tst_hugepages);

> +	private_noreserve = !!private_noreserve_opt;
Drop this line.

>  	private_resv = kernel_has_private_reservations();
> +	tst_res(TDEBUG, "MAP_PRIVATE mmap() %s a hugetlb reservation",
> +		private_resv ? "creates" : "does not create");
>  }
>  
>  static void cleanup(void)
> @@ -543,6 +560,11 @@ static void cleanup(void)
>  
>  static struct tst_test test = {
>  	.needs_root = 1,
> +	.options = (struct tst_option[]) {
> +		{"n", &private_noreserve_opt,
> +		 "Map private VMAs with MAP_NORESERVE"},
> +		{}
> +	},
>  	.mntpoint = MNTPOINT,
>  	.needs_hugetlbfs = 1,
>  	.save_restore = (const struct tst_path_val[]) {
> -- 
> 2.52.0
> 

-- 
Regards,
Li Wang

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