From mboxrd@z Thu Jan  1 00:00:00 1970
From: Cyril Hrubis <chrubis@suse.cz>
Date: Mon, 20 May 2019 16:41:45 +0200
Subject: [LTP] [PATCH v2] syscalls/move_page12.c: Add new regression test
In-Reply-To: <1555404533-2316-1-git-send-email-xuyang2018.jy@cn.fujitsu.com>
References: <1512100108-28359-2-git-send-email-yangx.jy@cn.fujitsu.com>
 <1555404533-2316-1-git-send-email-xuyang2018.jy@cn.fujitsu.com>
Message-ID: <20190520144145.GC28976@rei.lan>
List-Id: <ltp.lists.linux.it>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: ltp@lists.linux.it

Hi!
>  #include <errno.h>
> @@ -49,9 +54,17 @@
>  #define PATH_MEMINFO	"/proc/meminfo"
>  #define PATH_NR_HUGEPAGES	"/proc/sys/vm/nr_hugepages"
>  #define PATH_HUGEPAGES	"/sys/kernel/mm/hugepages/"
> -#define TEST_PAGES	2
> +#define PATH_PAGEMAP	"/proc/self/pagemap"
>  #define TEST_NODES	2
>  
> +static struct tcase {
> +	int tpages;
> +	int offline;
> +} tcases[] = {
> +	{2, 0},
> +	{2, 1},
> +};
> +
>  static int pgsz, hpsz;
>  static long orig_hugepages = -1;
>  static char path_hugepages_node1[PATH_MAX];
> @@ -61,9 +74,21 @@ static long orig_hugepages_node2 = -1;
>  static unsigned int node1, node2;
>  static void *addr;
>  
> -static void do_child(void)
> +static void do_soft_offline(int tpgs)
>  {
> -	int test_pages = TEST_PAGES * hpsz / pgsz;
> +	if (madvise(addr, tpgs * hpsz, MADV_SOFT_OFFLINE) == -1) {
> +		if (errno == EINVAL) {
> +			tst_brk(TCONF,
> +				"madvise() didn't support MADV_SOFT_OFFLINE");

Can we change this to tst_res() and return a value from this function so
that the second test could be skipped without exitting the whole test?

The thing is that as far as we are implementing more than one testcase
in a test unsupported features that are only relevant for a one test
should not exit the whole testcase in case of looping (the -i
parameter).

> +		} else {
> +			tst_brk(TBROK | TERRNO, "madvise() failed");
> +		}
> +	}
> +}
> +
> +static void do_child(int tpgs)
> +{
> +	int test_pages = tpgs * hpsz / pgsz;
>  	int i, j;
>  	int *nodes, *status;
>  	void **pages;
> @@ -96,34 +121,42 @@ static void do_child(void)
>  	exit(0);
>  }
>  
> -static void do_test(void)
> +static void do_test(unsigned int n)
>  {
>  	int i;
>  	pid_t cpid = -1;
>  	int status;
>  	unsigned int twenty_percent = (tst_timeout_remaining() / 5);
>  
> -	addr = SAFE_MMAP(NULL, TEST_PAGES * hpsz, PROT_READ | PROT_WRITE,
> +	if (tcases[n].offline) {
> +		if (access(PATH_PAGEMAP, F_OK))
> +			tst_brk(TCONF, "pagemap was not supported");

Here as well, can we do tst_res() and return instead?

> +	}
> +
> +	addr = SAFE_MMAP(NULL, tcases[n].tpages * hpsz, PROT_READ | PROT_WRITE,
>  		MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
>  
> -	SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
> +	SAFE_MUNMAP(addr, tcases[n].tpages * hpsz);
>  
>  	cpid = SAFE_FORK();
>  	if (cpid == 0)
> -		do_child();
> +		do_child(tcases[n].tpages);
>  
>  	for (i = 0; i < LOOPS; i++) {
>  		void *ptr;
>  
> -		ptr = SAFE_MMAP(NULL, TEST_PAGES * hpsz,
> +		ptr = SAFE_MMAP(NULL, tcases[n].tpages * hpsz,
>  			PROT_READ | PROT_WRITE,
>  			MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
>  		if (ptr != addr)
>  			tst_brk(TBROK, "Failed to mmap at desired addr");
>  
> -		memset(addr, 0, TEST_PAGES * hpsz);
> +		memset(addr, 0, tcases[n].tpages * hpsz);
> +
> +		if (tcases[n].offline)
> +			do_soft_offline(tcases[n].tpages);


And here we should check the return value and return if the madvise in
the function has returned EINVAL.


> -		SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
> +		SAFE_MUNMAP(addr, tcases[n].tpages * hpsz);
>  
>  		if (tst_timeout_remaining() < twenty_percent)
>  			break;
> @@ -266,7 +299,8 @@ static struct tst_test test = {
>  	.forks_child = 1,
>  	.setup = setup,
>  	.cleanup = cleanup,
> -	.test_all = do_test,
> +	.test = do_test,
> +	.tcnt = ARRAY_SIZE(tcases),
>  };
>  
>  #else

Otherwise it looks good.

-- 
Cyril Hrubis
chrubis@suse.cz