[LTP] [PATCH v4 0/7] Hugetlb:Migrating the libhugetlbfs tests

[LTP] [PATCH v4 0/7] Hugetlb:Migrating the libhugetlbfs tests

[Tarun Sahu]

Hi,
This patch series is in continuation to first part [1]
This series include 5 more tests taken from libhugetlbfs.

Background:
Libhugetlbfs is not being maintained actively, and some distro is dropping
support for it. There are some tests that are good for testing hugetlb
functionality in kernel. These patches include tests from libhugetlbfs.

ref:
 1. https://lore.kernel.org/all/20221104162511.28658-1-tsahu@linux.ibm.com/

v1 -> v2
	1. Fixed comments
v2 -> v3
	1. Changed from setjmp/longjmp logic to check/return logic in
	hugemmap10.c
	2. Added a new patch to create support for custom flags in
	tst_creat_unlinked api.
	3. Added patch to add safe macros for posix_fadvise call.
	4. Updated coding asthetics.
	5. Added description along with the line of failure in hugemmap10.c
v3 -> v4
	1. Removing redundant access call to check if file already exist.
	and removing the retry path, as the case is very rare in PATCH[1/7].

Tarun Sahu (7):
  Hugetlb: Migrating libhugetlbfs noresv-reserve-resv-page
  Hugetlb: Migrating libhugetlbfs noresv-regarded-as-resv
  Hugetlb: Migrating libhugetlbfs private
  Hugetlb: Migrating libhugetlbfs readahead_reserve
  Hugetlb: Migrating libhugetlbfs readback
  Hugetlb: Migrating libhugetlbfs shared
  Hugetlb: Migrating libhugetlbfs shm-fork

 runtest/hugetlb                               |   7 +
 testcases/kernel/mem/.gitignore               |   7 +
 .../kernel/mem/hugetlb/hugefork/hugefork02.c  | 196 ++++++++++++++++++
 .../kernel/mem/hugetlb/hugemmap/hugemmap27.c  | 166 +++++++++++++++
 .../kernel/mem/hugetlb/hugemmap/hugemmap28.c  | 107 ++++++++++
 .../kernel/mem/hugetlb/hugemmap/hugemmap29.c  | 129 ++++++++++++
 .../kernel/mem/hugetlb/hugemmap/hugemmap30.c  | 120 +++++++++++
 .../kernel/mem/hugetlb/hugemmap/hugemmap31.c  |  98 +++++++++
 .../kernel/mem/hugetlb/hugemmap/hugemmap32.c  | 104 ++++++++++
 9 files changed, 934 insertions(+)
 create mode 100644 testcases/kernel/mem/hugetlb/hugefork/hugefork02.c
 create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap27.c
 create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap28.c
 create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap29.c
 create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap30.c
 create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap31.c
 create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap32.c

-- 
2.31.1


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

[LTP] [PATCH v4 1/7] Hugetlb: Add new argument flags in tst_creat_unlinked

[Tarun Sahu]

Some test requires custom flags for file opened by tst_creat_unlinked
along with O_CREAT|O_EXCL|O_RDWR. This patch creates support to pass
custom flags in tst_creat_unlinked.

Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com>
---
 include/tst_test.h                            |  2 +-
 lib/tst_test.c                                | 19 ++++++++++++++-----
 .../kernel/mem/hugetlb/hugemmap/hugemmap07.c  |  2 +-
 .../kernel/mem/hugetlb/hugemmap/hugemmap08.c  |  2 +-
 .../kernel/mem/hugetlb/hugemmap/hugemmap09.c  |  2 +-
 5 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/include/tst_test.h b/include/tst_test.h
index acf2421de..a62515bfe 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -365,7 +365,7 @@ void tst_set_max_runtime(int max_runtime);
  * Create and open a random file inside the given dir path.
  * It unlinks the file after opening and return file descriptor.
  */
-int tst_creat_unlinked(const char *path);
+int tst_creat_unlinked(const char *path, int flags);
 
 /*
  * Returns path to the test temporary directory in a newly allocated buffer.
diff --git a/lib/tst_test.c b/lib/tst_test.c
index b225ba082..d17b15ee8 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1027,18 +1027,27 @@ static void prepare_and_mount_hugetlb_fs(void)
 	mntpoint_mounted = 1;
 }
 
-int tst_creat_unlinked(const char *path)
+int tst_creat_unlinked(const char *path, int flags)
 {
 	char template[PATH_MAX];
+	int len, c, range;
 	int fd;
+	int start[3] = {'0', 'a', 'A'};
 
 	snprintf(template, PATH_MAX, "%s/ltp_%.3sXXXXXX",
 			path, tid);
+	len = strlen(template) - 1;
+
+	srand(time(NULL));
+	while (template[len] == 'X') {
+		c = rand() % 3;
+		range = start[c] == '0' ? 10 : 26;
+		c = start[c] + (rand() % range);
+		template[len--] = (char)c;
+	}
 
-	fd = mkstemp(template);
-	if (fd < 0)
-		tst_brk(TBROK | TERRNO, "mkstemp(%s) failed", template);
-
+	flags |= O_CREAT|O_EXCL|O_RDWR;
+	fd = SAFE_OPEN(template, flags);
 	SAFE_UNLINK(template);
 	return fd;
 }
diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c
index bd0fb440a..3122d5b9d 100644
--- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c
@@ -113,7 +113,7 @@ cleanup:
 static void setup(void)
 {
 	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
-	huge_fd = tst_creat_unlinked(MNTPOINT);
+	huge_fd = tst_creat_unlinked(MNTPOINT, 0);
 }
 
 static void cleanup(void)
diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c
index ce40e7b69..f66b331dc 100644
--- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c
@@ -118,7 +118,7 @@ static void run_test(unsigned int test_type)
 static void setup(void)
 {
 	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
-	huge_fd = tst_creat_unlinked(MNTPOINT);
+	huge_fd = tst_creat_unlinked(MNTPOINT, 0);
 }
 
 static void cleanup(void)
diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c
index 1008395a4..ceb0f64a1 100644
--- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c
@@ -60,7 +60,7 @@ static void run_test(void)
 static void setup(void)
 {
 	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
-	huge_fd = tst_creat_unlinked(MNTPOINT);
+	huge_fd = tst_creat_unlinked(MNTPOINT, 0);
 }
 
 static void cleanup(void)
-- 
2.31.1


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

Re: [LTP] [PATCH v4 1/7] Hugetlb: Add new argument flags in tst_creat_unlinked

[Cyril Hrubis]

Hi!
> Some test requires custom flags for file opened by tst_creat_unlinked
> along with O_CREAT|O_EXCL|O_RDWR. This patch creates support to pass
> custom flags in tst_creat_unlinked.
> 
> Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com>
> ---
>  include/tst_test.h                            |  2 +-
>  lib/tst_test.c                                | 19 ++++++++++++++-----
>  .../kernel/mem/hugetlb/hugemmap/hugemmap07.c  |  2 +-
>  .../kernel/mem/hugetlb/hugemmap/hugemmap08.c  |  2 +-
>  .../kernel/mem/hugetlb/hugemmap/hugemmap09.c  |  2 +-
>  5 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/include/tst_test.h b/include/tst_test.h
> index acf2421de..a62515bfe 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -365,7 +365,7 @@ void tst_set_max_runtime(int max_runtime);
>   * Create and open a random file inside the given dir path.
>   * It unlinks the file after opening and return file descriptor.
>   */
> -int tst_creat_unlinked(const char *path);
> +int tst_creat_unlinked(const char *path, int flags);
>  
>  /*
>   * Returns path to the test temporary directory in a newly allocated buffer.
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index b225ba082..d17b15ee8 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -1027,18 +1027,27 @@ static void prepare_and_mount_hugetlb_fs(void)
>  	mntpoint_mounted = 1;
>  }
>  
> -int tst_creat_unlinked(const char *path)
> +int tst_creat_unlinked(const char *path, int flags)
>  {
>  	char template[PATH_MAX];
> +	int len, c, range;
>  	int fd;
> +	int start[3] = {'0', 'a', 'A'};
>  
>  	snprintf(template, PATH_MAX, "%s/ltp_%.3sXXXXXX",
>  			path, tid);
> +	len = strlen(template) - 1;
> +
> +	srand(time(NULL));
> +	while (template[len] == 'X') {
> +		c = rand() % 3;
> +		range = start[c] == '0' ? 10 : 26;
> +		c = start[c] + (rand() % range);
> +		template[len--] = (char)c;
> +	}
>  
> -	fd = mkstemp(template);
> -	if (fd < 0)
> -		tst_brk(TBROK | TERRNO, "mkstemp(%s) failed", template);
> -
> +	flags |= O_CREAT|O_EXCL|O_RDWR;
> +	fd = SAFE_OPEN(template, flags);

I wonder if it would be easier to add the O_DIRECT flag with F_GETFL and
F_SETFL fcntl(), but I guess that this is fine as it is.

The only potential problem I see is that setting the random seed may
interfere with anything that would print what seed has been used for
random data in order to be able to reproduce the same random sequence if
needed. So maybe I wouldn't attempt to set the seed in this function at
all.

>  	SAFE_UNLINK(template);
>  	return fd;
>  }
> diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c
> index bd0fb440a..3122d5b9d 100644
> --- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c
> +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c
> @@ -113,7 +113,7 @@ cleanup:
>  static void setup(void)
>  {
>  	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
> -	huge_fd = tst_creat_unlinked(MNTPOINT);
> +	huge_fd = tst_creat_unlinked(MNTPOINT, 0);
>  }
>  
>  static void cleanup(void)
> diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c
> index ce40e7b69..f66b331dc 100644
> --- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c
> +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c
> @@ -118,7 +118,7 @@ static void run_test(unsigned int test_type)
>  static void setup(void)
>  {
>  	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
> -	huge_fd = tst_creat_unlinked(MNTPOINT);
> +	huge_fd = tst_creat_unlinked(MNTPOINT, 0);
>  }
>  
>  static void cleanup(void)
> diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c
> index 1008395a4..ceb0f64a1 100644
> --- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c
> +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c
> @@ -60,7 +60,7 @@ static void run_test(void)
>  static void setup(void)
>  {
>  	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
> -	huge_fd = tst_creat_unlinked(MNTPOINT);
> +	huge_fd = tst_creat_unlinked(MNTPOINT, 0);
>  }
>  
>  static void cleanup(void)
> -- 
> 2.31.1
> 

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v4 1/7] Hugetlb: Add new argument flags in tst_creat_unlinked

[Tarun Sahu]

Hi, 
Thanks for reviewing the patch.
On Nov 16 2022, Cyril Hrubis wrote:
> Hi!
> > Some test requires custom flags for file opened by tst_creat_unlinked
> > along with O_CREAT|O_EXCL|O_RDWR. This patch creates support to pass
> > custom flags in tst_creat_unlinked.
> > 
> > Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com>
> > ---
> >  include/tst_test.h                            |  2 +-
> >  lib/tst_test.c                                | 19 ++++++++++++++-----
> >  .../kernel/mem/hugetlb/hugemmap/hugemmap07.c  |  2 +-
> >  .../kernel/mem/hugetlb/hugemmap/hugemmap08.c  |  2 +-
> >  .../kernel/mem/hugetlb/hugemmap/hugemmap09.c  |  2 +-
> >  5 files changed, 18 insertions(+), 9 deletions(-)
> > 
> > diff --git a/include/tst_test.h b/include/tst_test.h
> > index acf2421de..a62515bfe 100644
> > --- a/include/tst_test.h
> > +++ b/include/tst_test.h
> > @@ -365,7 +365,7 @@ void tst_set_max_runtime(int max_runtime);
> >   * Create and open a random file inside the given dir path.
> >   * It unlinks the file after opening and return file descriptor.
> >   */
> > -int tst_creat_unlinked(const char *path);
> > +int tst_creat_unlinked(const char *path, int flags);
> >  
> >  /*
> >   * Returns path to the test temporary directory in a newly allocated buffer.
> > diff --git a/lib/tst_test.c b/lib/tst_test.c
> > index b225ba082..d17b15ee8 100644
> > --- a/lib/tst_test.c
> > +++ b/lib/tst_test.c
> > @@ -1027,18 +1027,27 @@ static void prepare_and_mount_hugetlb_fs(void)
> >  	mntpoint_mounted = 1;
> >  }
> >  
> > -int tst_creat_unlinked(const char *path)
> > +int tst_creat_unlinked(const char *path, int flags)
> >  {
> >  	char template[PATH_MAX];
> > +	int len, c, range;
> >  	int fd;
> > +	int start[3] = {'0', 'a', 'A'};
> >  
> >  	snprintf(template, PATH_MAX, "%s/ltp_%.3sXXXXXX",
> >  			path, tid);
> > +	len = strlen(template) - 1;
> > +
> > +	srand(time(NULL));
> > +	while (template[len] == 'X') {
> > +		c = rand() % 3;
> > +		range = start[c] == '0' ? 10 : 26;
> > +		c = start[c] + (rand() % range);
> > +		template[len--] = (char)c;
> > +	}
> >  
> > -	fd = mkstemp(template);
> > -	if (fd < 0)
> > -		tst_brk(TBROK | TERRNO, "mkstemp(%s) failed", template);
> > -
> > +	flags |= O_CREAT|O_EXCL|O_RDWR;
> > +	fd = SAFE_OPEN(template, flags);
> 
> I wonder if it would be easier to add the O_DIRECT flag with F_GETFL and
> F_SETFL fcntl(), but I guess that this is fine as it is.
> 
> The only potential problem I see is that setting the random seed may
> interfere with anything that would print what seed has been used for
> random data in order to be able to reproduce the same random sequence if
> needed. So maybe I wouldn't attempt to set the seed in this function at
> all.
> 
Ok. can remove it.

One more concern, I have, it wont be thread safe. Is it acceptable?
Though, an alternative is to use mkostemp, but it is only defined in
__GNU_SOURCE, defining it for the whole library tst_test.c doesnt seem good.

instead using rand_r will require _thread static seed to make it
thread safe. but doesn't seem to be worth the effort, if we can simply use,
mkostemp or fcntl to set flags manually later.

What do you think?

> >  	SAFE_UNLINK(template);
> >  	return fd;
> >  }
> > diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c
> > index bd0fb440a..3122d5b9d 100644
> > --- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c
> > +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c
> > @@ -113,7 +113,7 @@ cleanup:
> >  static void setup(void)
> >  {
> >  	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
> > -	huge_fd = tst_creat_unlinked(MNTPOINT);
> > +	huge_fd = tst_creat_unlinked(MNTPOINT, 0);
> >  }
> >  
> >  static void cleanup(void)
> > diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c
> > index ce40e7b69..f66b331dc 100644
> > --- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c
> > +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c
> > @@ -118,7 +118,7 @@ static void run_test(unsigned int test_type)
> >  static void setup(void)
> >  {
> >  	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
> > -	huge_fd = tst_creat_unlinked(MNTPOINT);
> > +	huge_fd = tst_creat_unlinked(MNTPOINT, 0);
> >  }
> >  
> >  static void cleanup(void)
> > diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c
> > index 1008395a4..ceb0f64a1 100644
> > --- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c
> > +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c
> > @@ -60,7 +60,7 @@ static void run_test(void)
> >  static void setup(void)
> >  {
> >  	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
> > -	huge_fd = tst_creat_unlinked(MNTPOINT);
> > +	huge_fd = tst_creat_unlinked(MNTPOINT, 0);
> >  }
> >  
> >  static void cleanup(void)
> > -- 
> > 2.31.1
> > 
> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

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

Re: [LTP] [PATCH v4 1/7] Hugetlb: Add new argument flags in tst_creat_unlinked

[Tarun Sahu]

On Nov 17 2022, Tarun Sahu wrote:
> Hi, 
> Thanks for reviewing the patch.
> On Nov 16 2022, Cyril Hrubis wrote:
> > Hi!
> > > Some test requires custom flags for file opened by tst_creat_unlinked
> > > along with O_CREAT|O_EXCL|O_RDWR. This patch creates support to pass
> > > custom flags in tst_creat_unlinked.
> > > 
> > > Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com>
> > > ---
> > >  include/tst_test.h                            |  2 +-
> > >  lib/tst_test.c                                | 19 ++++++++++++++-----
> > >  .../kernel/mem/hugetlb/hugemmap/hugemmap07.c  |  2 +-
> > >  .../kernel/mem/hugetlb/hugemmap/hugemmap08.c  |  2 +-
> > >  .../kernel/mem/hugetlb/hugemmap/hugemmap09.c  |  2 +-
> > >  5 files changed, 18 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/include/tst_test.h b/include/tst_test.h
> > > index acf2421de..a62515bfe 100644
> > > --- a/include/tst_test.h
> > > +++ b/include/tst_test.h
> > > @@ -365,7 +365,7 @@ void tst_set_max_runtime(int max_runtime);
> > >   * Create and open a random file inside the given dir path.
> > >   * It unlinks the file after opening and return file descriptor.
> > >   */
> > > -int tst_creat_unlinked(const char *path);
> > > +int tst_creat_unlinked(const char *path, int flags);
> > >  
> > >  /*
> > >   * Returns path to the test temporary directory in a newly allocated buffer.
> > > diff --git a/lib/tst_test.c b/lib/tst_test.c
> > > index b225ba082..d17b15ee8 100644
> > > --- a/lib/tst_test.c
> > > +++ b/lib/tst_test.c
> > > @@ -1027,18 +1027,27 @@ static void prepare_and_mount_hugetlb_fs(void)
> > >  	mntpoint_mounted = 1;
> > >  }
> > >  
> > > -int tst_creat_unlinked(const char *path)
> > > +int tst_creat_unlinked(const char *path, int flags)
> > >  {
> > >  	char template[PATH_MAX];
> > > +	int len, c, range;
> > >  	int fd;
> > > +	int start[3] = {'0', 'a', 'A'};
> > >  
> > >  	snprintf(template, PATH_MAX, "%s/ltp_%.3sXXXXXX",
> > >  			path, tid);
> > > +	len = strlen(template) - 1;
> > > +
> > > +	srand(time(NULL));
> > > +	while (template[len] == 'X') {
> > > +		c = rand() % 3;
> > > +		range = start[c] == '0' ? 10 : 26;
> > > +		c = start[c] + (rand() % range);
> > > +		template[len--] = (char)c;
> > > +	}
> > >  
> > > -	fd = mkstemp(template);
> > > -	if (fd < 0)
> > > -		tst_brk(TBROK | TERRNO, "mkstemp(%s) failed", template);
> > > -
> > > +	flags |= O_CREAT|O_EXCL|O_RDWR;
> > > +	fd = SAFE_OPEN(template, flags);
> > 
> > I wonder if it would be easier to add the O_DIRECT flag with F_GETFL and
> > F_SETFL fcntl(), but I guess that this is fine as it is.
> > 
> > The only potential problem I see is that setting the random seed may
> > interfere with anything that would print what seed has been used for
> > random data in order to be able to reproduce the same random sequence if
> > needed. So maybe I wouldn't attempt to set the seed in this function at
> > all.
> > 
> Ok. can remove it.
> 
> One more concern, I have, it wont be thread safe. Is it acceptable?
> Though, an alternative is to use mkostemp, but it is only defined in
> __GNU_SOURCE, defining it for the whole library tst_test.c doesnt seem good.
> 
> instead using rand_r will require _thread static seed to make it
> thread safe. but doesn't seem to be worth the effort, if we can simply use,
> mkostemp or fcntl to set flags manually later.
> 
> What do you think?
> 
I missed it,
I could make use of mktemp. will replace all this. and update it in next
version.

> > >  	SAFE_UNLINK(template);
> > >  	return fd;
> > >  }
> > > diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c
> > > index bd0fb440a..3122d5b9d 100644
> > > --- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c
> > > +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c
> > > @@ -113,7 +113,7 @@ cleanup:
> > >  static void setup(void)
> > >  {
> > >  	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
> > > -	huge_fd = tst_creat_unlinked(MNTPOINT);
> > > +	huge_fd = tst_creat_unlinked(MNTPOINT, 0);
> > >  }
> > >  
> > >  static void cleanup(void)
> > > diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c
> > > index ce40e7b69..f66b331dc 100644
> > > --- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c
> > > +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c
> > > @@ -118,7 +118,7 @@ static void run_test(unsigned int test_type)
> > >  static void setup(void)
> > >  {
> > >  	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
> > > -	huge_fd = tst_creat_unlinked(MNTPOINT);
> > > +	huge_fd = tst_creat_unlinked(MNTPOINT, 0);
> > >  }
> > >  
> > >  static void cleanup(void)
> > > diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c
> > > index 1008395a4..ceb0f64a1 100644
> > > --- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c
> > > +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c
> > > @@ -60,7 +60,7 @@ static void run_test(void)
> > >  static void setup(void)
> > >  {
> > >  	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
> > > -	huge_fd = tst_creat_unlinked(MNTPOINT);
> > > +	huge_fd = tst_creat_unlinked(MNTPOINT, 0);
> > >  }
> > >  
> > >  static void cleanup(void)
> > > -- 
> > > 2.31.1
> > > 
> > 
> > -- 
> > Cyril Hrubis
> > chrubis@suse.cz
> > 
> > -- 
> > Mailing list info: https://lists.linux.it/listinfo/ltp
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

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

[LTP] [PATCH v4 2/7] Hugetlb: Migrating libhugetlbfs counters

[Tarun Sahu]

Migrating the libhugetlbfs/testcases/counters.c test

Test Description: This Test perform mmap and unmap and write operation on
hugetlb file based mapping. Mapping can be shared or private. and it
checks for Hugetlb counter (Total, Free, Reserve, Surplus) in /proc/meminfo
and compare them with expected (calculated) value. if all checks are
successful, the test passes.

Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com>
---
 runtest/hugetlb                               |   1 +
 testcases/kernel/mem/.gitignore               |   1 +
 .../kernel/mem/hugetlb/hugemmap/hugemmap10.c  | 462 ++++++++++++++++++
 3 files changed, 464 insertions(+)
 create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap10.c

diff --git a/runtest/hugetlb b/runtest/hugetlb
index e2ada7a97..8a56d52a3 100644
--- a/runtest/hugetlb
+++ b/runtest/hugetlb
@@ -6,6 +6,7 @@ hugemmap06 hugemmap06
 hugemmap07 hugemmap07
 hugemmap08 hugemmap08
 hugemmap09 hugemmap09
+hugemmap10 hugemmap10
 hugemmap05_1 hugemmap05 -m
 hugemmap05_2 hugemmap05 -s
 hugemmap05_3 hugemmap05 -s -m
diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore
index 1a242ffe0..e7def68cb 100644
--- a/testcases/kernel/mem/.gitignore
+++ b/testcases/kernel/mem/.gitignore
@@ -7,6 +7,7 @@
 /hugetlb/hugemmap/hugemmap07
 /hugetlb/hugemmap/hugemmap08
 /hugetlb/hugemmap/hugemmap09
+/hugetlb/hugemmap/hugemmap10
 /hugetlb/hugeshmat/hugeshmat01
 /hugetlb/hugeshmat/hugeshmat02
 /hugetlb/hugeshmat/hugeshmat03
diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap10.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap10.c
new file mode 100644
index 000000000..a77db660f
--- /dev/null
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap10.c
@@ -0,0 +1,462 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+/*
+ * Copyright (C) 2005-2007 IBM Corporation.
+ * Author: David Gibson & Adam Litke
+ */
+
+/*\
+ * [Description]
+ *
+ * This Test perform mmap and unmap and write operation on hugetlb file
+ * based mapping. Mapping can be shared or private. and it checks for
+ * Hugetlb counter (Total, Free, Reserve, Surplus) in /proc/meminfo and
+ * compare them with expected (calculated) value. if all checks are
+ * successful, the test passes.
+ *
+ */
+
+#define _GNU_SOURCE
+#include <unistd.h>
+#include <stdio.h>
+#include <sys/mount.h>
+#include <limits.h>
+#include <sys/param.h>
+#include <sys/types.h>
+
+#include "hugetlb.h"
+
+#define MNTPOINT "hugetlbfs/"
+
+#define CHECK_(fun)	\
+		{			\
+		if (fun)	\
+			break;	\
+	}
+
+static long hpage_size;
+static int private_resv;
+
+#define NR_SLOTS	2
+#define SL_SETUP	0
+#define SL_TEST		1
+static int map_fd[NR_SLOTS];
+static char *map_addr[NR_SLOTS];
+static unsigned long map_size[NR_SLOTS];
+static unsigned int touched[NR_SLOTS];
+
+static long prev_total;
+static long prev_free;
+static long prev_resv;
+static long prev_surp;
+
+static void read_meminfo_huge(long *total, long *free, long *resv, long *surp)
+{
+	*total = SAFE_READ_MEMINFO(MEMINFO_HPAGE_TOTAL);
+	*free = SAFE_READ_MEMINFO(MEMINFO_HPAGE_FREE);
+	*resv = SAFE_READ_MEMINFO(MEMINFO_HPAGE_RSVD);
+	*surp = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SURP);
+}
+
+static int kernel_has_private_reservations(void)
+{
+	int fd;
+	long t, f, r, s;
+	long nt, nf, nr, ns;
+	void *map;
+
+	read_meminfo_huge(&t, &f, &r, &s);
+	fd = tst_creat_unlinked(MNTPOINT, 0);
+
+	map = SAFE_MMAP(NULL, hpage_size, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0);
+
+	read_meminfo_huge(&nt, &nf, &nr, &ns);
+
+	SAFE_MUNMAP(map, hpage_size);
+	SAFE_CLOSE(fd);
+
+	/*
+	 * There are only three valid cases:
+	 * 1) If a surplus page was allocated to create a reservation, all
+	 *    four pool counters increment
+	 * 2) All counters remain the same except for Hugepages_Rsvd, then
+	 *    a reservation was created using an existing pool page.
+	 * 3) All counters remain the same, indicates that no reservation has
+	 *    been created
+	 */
+	if ((nt == t + 1) && (nf == f + 1) && (ns == s + 1) && (nr == r + 1))
+		return 1;
+	else if ((nt == t) && (nf == f) && (ns == s)) {
+		if (nr == r + 1)
+			return 1;
+		else if (nr == r)
+			return 0;
+	}
+	tst_brk(TCONF, "bad counter state - "
+	      "T:%li F:%li R:%li S:%li -> T:%li F:%li R:%li S:%li",
+		  t, f, r, s, nt, nf, nr, ns);
+	return -1;
+}
+
+static int verify_counters(int line, char *desc, long et, long ef, long er, long es)
+{
+	long t, f, r, s;
+	long fail = 0;
+
+	read_meminfo_huge(&t, &f, &r, &s);
+
+	if (t != et) {
+		tst_res(TFAIL, "At Line %i:While %s: Bad "MEMINFO_HPAGE_TOTAL
+				" expected %li, actual %li", line, desc, et, t);
+		fail++;
+	}
+	if (f != ef) {
+		tst_res(TFAIL, "At Line %i:While %s: Bad "MEMINFO_HPAGE_FREE
+				" expected %li, actual %li", line, desc, ef, f);
+		fail++;
+	}
+	if (r != er) {
+		tst_res(TFAIL, "At Line %i:While %s: Bad "MEMINFO_HPAGE_RSVD
+				" expected %li, actual %li", line, desc, er, r);
+		fail++;
+	}
+	if (s != es) {
+		tst_res(TFAIL, "At Line %i:While %s: Bad "MEMINFO_HPAGE_SURP
+				" expected %li, actual %li", line, desc, es, s);
+		fail++;
+	}
+
+	if (fail)
+		return -1;
+
+	prev_total = t;
+	prev_free = f;
+	prev_resv = r;
+	prev_surp = s;
+	return 0;
+}
+
+/* Memory operations:
+ * Each of these has a predefined effect on the counters
+ */
+static int set_nr_hugepages_(long count, char *desc, int line)
+{
+	long min_size;
+	long et, ef, er, es;
+
+	SAFE_FILE_PRINTF(PATH_NR_HPAGES, "%lu", count);
+
+	/* The code below is based on set_max_huge_pages in mm/hugetlb.c */
+	es = prev_surp;
+	et = prev_total;
+	ef = prev_free;
+	er = prev_resv;
+
+	/*
+	 * Increase the pool size
+	 * First take pages out of surplus state.  Then make up the
+	 * remaining difference by allocating fresh huge pages.
+	 */
+	while (es && count > et - es)
+		es--;
+	while (count > et - es) {
+		et++;
+		ef++;
+	}
+	if (count >= et - es)
+		goto out;
+
+	/*
+	 * Decrease the pool size
+	 * First return free pages to the buddy allocator (being careful
+	 * to keep enough around to satisfy reservations).  Then place
+	 * pages into surplus state as needed so the pool will shrink
+	 * to the desired size as pages become free.
+	 */
+	min_size = MAX(count, er + et - ef);
+	while (min_size < et - es) {
+		ef--;
+		et--;
+	}
+	while (count < et - es)
+		es++;
+
+out:
+	return verify_counters(line, desc, et, ef, er, es);
+}
+#define set_nr_hugepages(c, d) CHECK_(set_nr_hugepages_(c, d, __LINE__))
+
+static int map_(int s, int hpages, int flags, char *desc, int line)
+{
+	long et, ef, er, es;
+
+	map_fd[s] = tst_creat_unlinked(MNTPOINT, 0);
+	map_size[s] = hpages * hpage_size;
+	map_addr[s] = SAFE_MMAP(NULL, map_size[s], PROT_READ|PROT_WRITE, flags,
+				map_fd[s], 0);
+	touched[s] = 0;
+
+	et = prev_total;
+	ef = prev_free;
+	er = prev_resv;
+	es = prev_surp;
+	/*
+	 * When using MAP_SHARED, a reservation will be created to guarantee
+	 * pages to the process.  If not enough pages are available to
+	 * satisfy the reservation, surplus pages are added to the pool.
+	 * NOTE: This code assumes that the whole mapping needs to be
+	 * reserved and hence, will not work with partial reservations.
+	 *
+	 * If the kernel supports private reservations, then MAP_PRIVATE
+	 * mappings behave like MAP_SHARED at mmap time.  Otherwise,
+	 * no counter updates will occur.
+	 */
+	if ((flags & MAP_SHARED) || private_resv) {
+		unsigned long shortfall = 0;
+
+		if (hpages + prev_resv > prev_free)
+			shortfall = hpages - prev_free + prev_resv;
+		et += shortfall;
+		ef += shortfall;
+		er += hpages;
+		es += shortfall;
+	}
+
+	return verify_counters(line, desc, et, ef, er, es);
+}
+#define map(s, h, f, d) CHECK_(map_(s, h, f, d, __LINE__))
+
+static int unmap_(int s, int hpages, int flags, char *desc, int line)
+{
+	long et, ef, er, es;
+	unsigned long i;
+
+	SAFE_MUNMAP(map_addr[s], map_size[s]);
+	SAFE_CLOSE(map_fd[s]);
+	map_addr[s] = NULL;
+	map_size[s] = 0;
+
+	et = prev_total;
+	ef = prev_free;
+	er = prev_resv;
+	es = prev_surp;
+
+	/*
+	 * When a VMA is unmapped, the instantiated (touched) pages are
+	 * freed.  If the pool is in a surplus state, pages are freed to the
+	 * buddy allocator, otherwise they go back into the hugetlb pool.
+	 * NOTE: This code assumes touched pages have only one user.
+	 */
+	for (i = 0; i < touched[s]; i++) {
+		if (es) {
+			et--;
+			es--;
+		} else
+			ef++;
+	}
+
+	/*
+	 * mmap may have created some surplus pages to accommodate a
+	 * reservation.  If those pages were not touched, then they will
+	 * not have been freed by the code above.  Free them here.
+	 */
+	if ((flags & MAP_SHARED) || private_resv) {
+		int unused_surplus = MIN(hpages - touched[s], es);
+
+		et -= unused_surplus;
+		ef -= unused_surplus;
+		er -= hpages - touched[s];
+		es -= unused_surplus;
+	}
+
+	return verify_counters(line, desc, et, ef, er, es);
+}
+#define unmap(s, h, f, d) CHECK_(unmap_(s, h, f, d, __LINE__))
+
+static int touch_(int s, int hpages, int flags, char *desc, int line)
+{
+	long et, ef, er, es;
+	int nr;
+	char *c;
+
+	for (c = map_addr[s], nr = hpages;
+			hpages && c < map_addr[s] + map_size[s];
+			c += hpage_size, nr--)
+		*c = (char) (nr % 2);
+	/*
+	 * Keep track of how many pages were touched since we can't easily
+	 * detect that from user space.
+	 * NOTE: Calling this function more than once for a mmap may yield
+	 * results you don't expect.  Be careful :)
+	 */
+	touched[s] = MAX(touched[s], hpages);
+
+	/*
+	 * Shared (and private when supported) mappings and consume resv pages
+	 * that were previously allocated. Also deduct them from the free count.
+	 *
+	 * Unreserved private mappings may need to allocate surplus pages to
+	 * satisfy the fault.  The surplus pages become part of the pool
+	 * which could elevate total, free, and surplus counts.  resv is
+	 * unchanged but free must be decreased.
+	 */
+	if (flags & MAP_SHARED || private_resv) {
+		et = prev_total;
+		ef = prev_free - hpages;
+		er = prev_resv - hpages;
+		es = prev_surp;
+	} else {
+		if (hpages + prev_resv > prev_free)
+			et = prev_total + (hpages - prev_free + prev_resv);
+		else
+			et = prev_total;
+		er = prev_resv;
+		es = prev_surp + et - prev_total;
+		ef = prev_free - hpages + et - prev_total;
+	}
+	return verify_counters(line, desc, et, ef, er, es);
+}
+#define touch(s, h, f, d) CHECK_(touch_(s, h, f, d, __LINE__))
+
+static int test_counters_(char *desc, int base_nr)
+{
+	int fail = 1;
+
+	tst_res(TINFO, "%s...", desc);
+
+	do {
+		set_nr_hugepages(base_nr, "initializing hugepages pool");
+
+		/* untouched, shared mmap */
+		map(SL_TEST, 1, MAP_SHARED, "doing mmap shared with no touch");
+		unmap(SL_TEST, 1, MAP_SHARED, "doing munmap on shared with no touch");
+
+		/* untouched, private mmap */
+		map(SL_TEST, 1, MAP_PRIVATE, "doing mmap private with no touch");
+		unmap(SL_TEST, 1, MAP_PRIVATE, "doing munmap private with on touch");
+
+		/* touched, shared mmap */
+		map(SL_TEST, 1, MAP_SHARED, "doing mmap shared followed by touch");
+		touch(SL_TEST, 1, MAP_SHARED, "touching the addr after mmap shared");
+		unmap(SL_TEST, 1, MAP_SHARED, "doing munmap shared after touch");
+
+		/* touched, private mmap */
+		map(SL_TEST, 1, MAP_PRIVATE, "doing mmap private followed by touch");
+		touch(SL_TEST, 1, MAP_PRIVATE, "touching the addr after mmap private");
+		unmap(SL_TEST, 1, MAP_PRIVATE, "doing munmap private after touch");
+
+		/* Explicit resizing during outstanding surplus */
+		/* Consume surplus when growing pool */
+		map(SL_TEST, 2, MAP_SHARED, "doing mmap to consume surplus");
+		set_nr_hugepages(MAX(base_nr, 1), "setting hugepages pool to consume surplus");
+
+		/* Add pages once surplus is consumed */
+		set_nr_hugepages(MAX(base_nr, 3), "Adding more pages after consuming surplus");
+
+		/* Release free huge pages first */
+		set_nr_hugepages(MAX(base_nr, 2), "Releasing free huge pages");
+
+		/* When shrinking beyond committed level, increase surplus */
+		set_nr_hugepages(base_nr, "increasing surplus counts");
+
+		/* Upon releasing the reservation, reduce surplus counts */
+		unmap(SL_TEST, 2, MAP_SHARED, "reducing surplus counts");
+		fail = 0;
+	} while (0);
+
+	if (fail)
+		return -1;
+	tst_res(TINFO, "OK");
+	return 0;
+}
+
+#define test_counters(a, b) CHECK_(test_counters_(a, b))
+
+static void per_iteration_cleanup(void)
+{
+	int nr;
+
+	prev_total = 0;
+	prev_free = 0;
+	prev_resv = 0;
+	prev_surp = 0;
+	for (nr = 0; nr < NR_SLOTS; nr++) {
+		if (map_addr[nr])
+			SAFE_MUNMAP(map_addr[nr], map_size[nr]);
+		if (map_fd[nr] > 0)
+			SAFE_CLOSE(map_fd[nr]);
+	}
+}
+
+static void run_test(void)
+{
+	int base_nr;
+
+	for (base_nr = 0; base_nr <= 3; base_nr++) {
+		tst_res(TINFO, "Base pool size: %i", base_nr);
+		/* Run the tests with a clean slate */
+		test_counters("Clean", base_nr);
+
+		/* Now with a pre-existing untouched, shared mmap */
+		map(SL_SETUP, 1, MAP_SHARED,
+				"doing mmap for running pre-existing untouched shared mapping test");
+		test_counters("Untouched, shared", base_nr);
+		unmap(SL_SETUP, 1, MAP_SHARED,
+				"doing munmap after running pre-existing untouched shared mapping test");
+
+		/* Now with a pre-existing untouched, private mmap */
+		map(SL_SETUP, 1, MAP_PRIVATE,
+				"doing mmap for running pre-existing untouched private mapping test");
+		test_counters("Untouched, private", base_nr);
+		unmap(SL_SETUP, 1, MAP_PRIVATE,
+				"doing munmap after running pre-existing untouced private mapping test");
+
+		/* Now with a pre-existing touched, shared mmap */
+		map(SL_SETUP, 1, MAP_SHARED,
+				"doing mmap for running pre-existing touched shared mapping test");
+		touch(SL_SETUP, 1, MAP_SHARED,
+				"touching for running pre-existing touched shared mapping test");
+		test_counters("Touched, shared", base_nr);
+		unmap(SL_SETUP, 1, MAP_SHARED,
+				"doing munmap after running pre-existing touched shared mapping test");
+
+		/* Now with a pre-existing touched, private mmap */
+		map(SL_SETUP, 1, MAP_PRIVATE,
+				"doing mmap for running pre-existing touched private mapping test");
+		touch(SL_SETUP, 1, MAP_PRIVATE,
+				"touching for running pre-existing touched private mapping test");
+		test_counters("Touched, private", base_nr);
+		unmap(SL_SETUP, 1, MAP_PRIVATE,
+				"doing munmap after running pre-existing touched private mapping test");
+	}
+	if (base_nr > 3)
+		tst_res(TPASS, "Hugepages Counters works as expected.");
+	per_iteration_cleanup();
+}
+
+static void setup(void)
+{
+	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
+	SAFE_FILE_PRINTF(PATH_OC_HPAGES, "%lu", tst_hugepages);
+	private_resv = kernel_has_private_reservations();
+}
+
+static void cleanup(void)
+{
+	per_iteration_cleanup();
+}
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.mntpoint = MNTPOINT,
+	.needs_hugetlbfs = 1,
+	.save_restore = (const struct tst_path_val[]) {
+		{PATH_OC_HPAGES, NULL},
+		{PATH_NR_HPAGES, NULL},
+		{}
+	},
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run_test,
+	.hugepages = {3, TST_NEEDS},
+};
+
-- 
2.31.1


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

Re: [LTP] [PATCH v4 2/7] Hugetlb: Migrating libhugetlbfs counters

[Cyril Hrubis]

Hi!
> +#define CHECK_(fun)	\
> +		{			\
> +		if (fun)	\
> +			break;	\
> +	}

This is working around the tooling we have I would rather have a look
into the checkpatch script to see if we could tweak the rule rather than
to introduce this messy code.

Just submit the patch with sane code and I will check for what I can do
with the script to get rid of the error.

> +static long hpage_size;
> +static int private_resv;
> +
> +#define NR_SLOTS	2
> +#define SL_SETUP	0
> +#define SL_TEST		1
> +static int map_fd[NR_SLOTS];
> +static char *map_addr[NR_SLOTS];
> +static unsigned long map_size[NR_SLOTS];
> +static unsigned int touched[NR_SLOTS];
> +
> +static long prev_total;
> +static long prev_free;
> +static long prev_resv;
> +static long prev_surp;
> +
> +static void read_meminfo_huge(long *total, long *free, long *resv, long *surp)
> +{
> +	*total = SAFE_READ_MEMINFO(MEMINFO_HPAGE_TOTAL);
> +	*free = SAFE_READ_MEMINFO(MEMINFO_HPAGE_FREE);
> +	*resv = SAFE_READ_MEMINFO(MEMINFO_HPAGE_RSVD);
> +	*surp = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SURP);
> +}
> +
> +static int kernel_has_private_reservations(void)
> +{
> +	int fd;
> +	long t, f, r, s;
> +	long nt, nf, nr, ns;
> +	void *map;
> +
> +	read_meminfo_huge(&t, &f, &r, &s);
> +	fd = tst_creat_unlinked(MNTPOINT, 0);
> +
> +	map = SAFE_MMAP(NULL, hpage_size, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0);
> +
> +	read_meminfo_huge(&nt, &nf, &nr, &ns);
> +
> +	SAFE_MUNMAP(map, hpage_size);
> +	SAFE_CLOSE(fd);
> +
> +	/*
> +	 * There are only three valid cases:
> +	 * 1) If a surplus page was allocated to create a reservation, all
> +	 *    four pool counters increment
> +	 * 2) All counters remain the same except for Hugepages_Rsvd, then
> +	 *    a reservation was created using an existing pool page.
> +	 * 3) All counters remain the same, indicates that no reservation has
> +	 *    been created
> +	 */
> +	if ((nt == t + 1) && (nf == f + 1) && (ns == s + 1) && (nr == r + 1))
> +		return 1;
> +	else if ((nt == t) && (nf == f) && (ns == s)) {
> +		if (nr == r + 1)
> +			return 1;
> +		else if (nr == r)
> +			return 0;
> +	}
> +	tst_brk(TCONF, "bad counter state - "
> +	      "T:%li F:%li R:%li S:%li -> T:%li F:%li R:%li S:%li",
> +		  t, f, r, s, nt, nf, nr, ns);
> +	return -1;
> +}
> +
> +static int verify_counters(int line, char *desc, long et, long ef, long er, long es)
> +{
> +	long t, f, r, s;
> +	long fail = 0;
> +
> +	read_meminfo_huge(&t, &f, &r, &s);
> +
> +	if (t != et) {
> +		tst_res(TFAIL, "At Line %i:While %s: Bad "MEMINFO_HPAGE_TOTAL
> +				" expected %li, actual %li", line, desc, et, t);

We do have tst_res_() that can be called as:

	tst_res_(__FILE__, line,
                 "%s bad " MEMINFO_HPAGE_TOTAL " = %li expected %li",
		 desc, et, t);

> +		fail++;
> +	}
> +	if (f != ef) {
> +		tst_res(TFAIL, "At Line %i:While %s: Bad "MEMINFO_HPAGE_FREE
> +				" expected %li, actual %li", line, desc, ef, f);
> +		fail++;
> +	}
> +	if (r != er) {
> +		tst_res(TFAIL, "At Line %i:While %s: Bad "MEMINFO_HPAGE_RSVD
> +				" expected %li, actual %li", line, desc, er, r);
> +		fail++;
> +	}
> +	if (s != es) {
> +		tst_res(TFAIL, "At Line %i:While %s: Bad "MEMINFO_HPAGE_SURP
> +				" expected %li, actual %li", line, desc, es, s);
> +		fail++;
> +	}
> +
> +	if (fail)
> +		return -1;
> +
> +	prev_total = t;
> +	prev_free = f;
> +	prev_resv = r;
> +	prev_surp = s;
> +	return 0;
> +}
> +
> +/* Memory operations:
> + * Each of these has a predefined effect on the counters
> + */
> +static int set_nr_hugepages_(long count, char *desc, int line)
> +{
> +	long min_size;
> +	long et, ef, er, es;
> +
> +	SAFE_FILE_PRINTF(PATH_NR_HPAGES, "%lu", count);
> +
> +	/* The code below is based on set_max_huge_pages in mm/hugetlb.c */
> +	es = prev_surp;
> +	et = prev_total;
> +	ef = prev_free;
> +	er = prev_resv;
> +
> +	/*
> +	 * Increase the pool size
> +	 * First take pages out of surplus state.  Then make up the
> +	 * remaining difference by allocating fresh huge pages.
> +	 */
> +	while (es && count > et - es)
> +		es--;
> +	while (count > et - es) {
> +		et++;
> +		ef++;
> +	}
> +	if (count >= et - es)
> +		goto out;
> +
> +	/*
> +	 * Decrease the pool size
> +	 * First return free pages to the buddy allocator (being careful
> +	 * to keep enough around to satisfy reservations).  Then place
> +	 * pages into surplus state as needed so the pool will shrink
> +	 * to the desired size as pages become free.
> +	 */
> +	min_size = MAX(count, er + et - ef);
> +	while (min_size < et - es) {
> +		ef--;
> +		et--;
> +	}
> +	while (count < et - es)
> +		es++;
> +
> +out:
> +	return verify_counters(line, desc, et, ef, er, es);
> +}
> +#define set_nr_hugepages(c, d) CHECK_(set_nr_hugepages_(c, d, __LINE__))

The macro name should be uppercase so that it's clear that it's a macro
and not a simple function. With that we can also drop the underscore
from the actual function name too.

> +static int map_(int s, int hpages, int flags, char *desc, int line)
> +{
> +	long et, ef, er, es;
> +
> +	map_fd[s] = tst_creat_unlinked(MNTPOINT, 0);
> +	map_size[s] = hpages * hpage_size;
> +	map_addr[s] = SAFE_MMAP(NULL, map_size[s], PROT_READ|PROT_WRITE, flags,
> +				map_fd[s], 0);
> +	touched[s] = 0;
> +
> +	et = prev_total;
> +	ef = prev_free;
> +	er = prev_resv;
> +	es = prev_surp;
> +	/*
> +	 * When using MAP_SHARED, a reservation will be created to guarantee
> +	 * pages to the process.  If not enough pages are available to
> +	 * satisfy the reservation, surplus pages are added to the pool.
> +	 * NOTE: This code assumes that the whole mapping needs to be
> +	 * reserved and hence, will not work with partial reservations.
> +	 *
> +	 * If the kernel supports private reservations, then MAP_PRIVATE
> +	 * mappings behave like MAP_SHARED at mmap time.  Otherwise,
> +	 * no counter updates will occur.
> +	 */
> +	if ((flags & MAP_SHARED) || private_resv) {
> +		unsigned long shortfall = 0;
> +
> +		if (hpages + prev_resv > prev_free)
> +			shortfall = hpages - prev_free + prev_resv;
> +		et += shortfall;
> +		ef += shortfall;
> +		er += hpages;
> +		es += shortfall;
> +	}
> +
> +	return verify_counters(line, desc, et, ef, er, es);
> +}
> +#define map(s, h, f, d) CHECK_(map_(s, h, f, d, __LINE__))
> +
> +static int unmap_(int s, int hpages, int flags, char *desc, int line)
> +{
> +	long et, ef, er, es;
> +	unsigned long i;
> +
> +	SAFE_MUNMAP(map_addr[s], map_size[s]);
> +	SAFE_CLOSE(map_fd[s]);
> +	map_addr[s] = NULL;
> +	map_size[s] = 0;
> +
> +	et = prev_total;
> +	ef = prev_free;
> +	er = prev_resv;
> +	es = prev_surp;
> +
> +	/*
> +	 * When a VMA is unmapped, the instantiated (touched) pages are
> +	 * freed.  If the pool is in a surplus state, pages are freed to the
> +	 * buddy allocator, otherwise they go back into the hugetlb pool.
> +	 * NOTE: This code assumes touched pages have only one user.
> +	 */
> +	for (i = 0; i < touched[s]; i++) {
> +		if (es) {
> +			et--;
> +			es--;
> +		} else
> +			ef++;
> +	}
> +
> +	/*
> +	 * mmap may have created some surplus pages to accommodate a
> +	 * reservation.  If those pages were not touched, then they will
> +	 * not have been freed by the code above.  Free them here.
> +	 */
> +	if ((flags & MAP_SHARED) || private_resv) {
> +		int unused_surplus = MIN(hpages - touched[s], es);
> +
> +		et -= unused_surplus;
> +		ef -= unused_surplus;
> +		er -= hpages - touched[s];
> +		es -= unused_surplus;
> +	}
> +
> +	return verify_counters(line, desc, et, ef, er, es);
> +}
> +#define unmap(s, h, f, d) CHECK_(unmap_(s, h, f, d, __LINE__))
> +
> +static int touch_(int s, int hpages, int flags, char *desc, int line)
> +{
> +	long et, ef, er, es;
> +	int nr;
> +	char *c;
> +
> +	for (c = map_addr[s], nr = hpages;
> +			hpages && c < map_addr[s] + map_size[s];
> +			c += hpage_size, nr--)
> +		*c = (char) (nr % 2);
> +	/*
> +	 * Keep track of how many pages were touched since we can't easily
> +	 * detect that from user space.
> +	 * NOTE: Calling this function more than once for a mmap may yield
> +	 * results you don't expect.  Be careful :)
> +	 */
> +	touched[s] = MAX(touched[s], hpages);
> +
> +	/*
> +	 * Shared (and private when supported) mappings and consume resv pages
> +	 * that were previously allocated. Also deduct them from the free count.
> +	 *
> +	 * Unreserved private mappings may need to allocate surplus pages to
> +	 * satisfy the fault.  The surplus pages become part of the pool
> +	 * which could elevate total, free, and surplus counts.  resv is
> +	 * unchanged but free must be decreased.
> +	 */
> +	if (flags & MAP_SHARED || private_resv) {
> +		et = prev_total;
> +		ef = prev_free - hpages;
> +		er = prev_resv - hpages;
> +		es = prev_surp;
> +	} else {
> +		if (hpages + prev_resv > prev_free)
> +			et = prev_total + (hpages - prev_free + prev_resv);
> +		else
> +			et = prev_total;
> +		er = prev_resv;
> +		es = prev_surp + et - prev_total;
> +		ef = prev_free - hpages + et - prev_total;
> +	}
> +	return verify_counters(line, desc, et, ef, er, es);
> +}
> +#define touch(s, h, f, d) CHECK_(touch_(s, h, f, d, __LINE__))
> +
> +static int test_counters_(char *desc, int base_nr)
> +{
> +	int fail = 1;
> +
> +	tst_res(TINFO, "%s...", desc);
> +
> +	do {
> +		set_nr_hugepages(base_nr, "initializing hugepages pool");
> +
> +		/* untouched, shared mmap */
> +		map(SL_TEST, 1, MAP_SHARED, "doing mmap shared with no touch");
> +		unmap(SL_TEST, 1, MAP_SHARED, "doing munmap on shared with no touch");
> +
> +		/* untouched, private mmap */
> +		map(SL_TEST, 1, MAP_PRIVATE, "doing mmap private with no touch");
> +		unmap(SL_TEST, 1, MAP_PRIVATE, "doing munmap private with on touch");
> +
> +		/* touched, shared mmap */
> +		map(SL_TEST, 1, MAP_SHARED, "doing mmap shared followed by touch");
> +		touch(SL_TEST, 1, MAP_SHARED, "touching the addr after mmap shared");
> +		unmap(SL_TEST, 1, MAP_SHARED, "doing munmap shared after touch");
> +
> +		/* touched, private mmap */
> +		map(SL_TEST, 1, MAP_PRIVATE, "doing mmap private followed by touch");
> +		touch(SL_TEST, 1, MAP_PRIVATE, "touching the addr after mmap private");
> +		unmap(SL_TEST, 1, MAP_PRIVATE, "doing munmap private after touch");
> +
> +		/* Explicit resizing during outstanding surplus */
> +		/* Consume surplus when growing pool */
> +		map(SL_TEST, 2, MAP_SHARED, "doing mmap to consume surplus");
> +		set_nr_hugepages(MAX(base_nr, 1), "setting hugepages pool to consume surplus");
> +
> +		/* Add pages once surplus is consumed */
> +		set_nr_hugepages(MAX(base_nr, 3), "Adding more pages after consuming surplus");
> +
> +		/* Release free huge pages first */
> +		set_nr_hugepages(MAX(base_nr, 2), "Releasing free huge pages");
> +
> +		/* When shrinking beyond committed level, increase surplus */
> +		set_nr_hugepages(base_nr, "increasing surplus counts");
> +
> +		/* Upon releasing the reservation, reduce surplus counts */
> +		unmap(SL_TEST, 2, MAP_SHARED, "reducing surplus counts");
> +		fail = 0;
> +	} while (0);
> +
> +	if (fail)
> +		return -1;
> +	tst_res(TINFO, "OK");
> +	return 0;

Maybe it would be a bit nicer to have actually two different macros so
that we don't have to resort to this do {} while (0) trickery e.g.

#define CHECK_BREAK(cond) if (cond) break;
#define CHECK_RETURN(cond) if (cond) return -1;

#define MAP_BREAK(s, h, f, d) CHECK_BREAK(map(s, h, f, d, __LINE__))
#define MAP_RETURN(s, h, f, d) CHECK_RETURN(map(s, h, f, d, __LINE__))

Then the check counters would look much better.

Or we can just stick to the RETURN variants and put the body of the loop
in the runtest() function into do_interation() function.

> +}
> +
> +#define test_counters(a, b) CHECK_(test_counters_(a, b))
> +
> +static void per_iteration_cleanup(void)
> +{
> +	int nr;
> +
> +	prev_total = 0;
> +	prev_free = 0;
> +	prev_resv = 0;
> +	prev_surp = 0;
> +	for (nr = 0; nr < NR_SLOTS; nr++) {
> +		if (map_addr[nr])
> +			SAFE_MUNMAP(map_addr[nr], map_size[nr]);
> +		if (map_fd[nr] > 0)
> +			SAFE_CLOSE(map_fd[nr]);
> +	}
> +}
> +
> +static void run_test(void)
> +{
> +	int base_nr;
> +
> +	for (base_nr = 0; base_nr <= 3; base_nr++) {
> +		tst_res(TINFO, "Base pool size: %i", base_nr);
> +		/* Run the tests with a clean slate */
> +		test_counters("Clean", base_nr);
> +
> +		/* Now with a pre-existing untouched, shared mmap */
> +		map(SL_SETUP, 1, MAP_SHARED,
> +				"doing mmap for running pre-existing untouched shared mapping test");
> +		test_counters("Untouched, shared", base_nr);
> +		unmap(SL_SETUP, 1, MAP_SHARED,
> +				"doing munmap after running pre-existing untouched shared mapping test");
> +
> +		/* Now with a pre-existing untouched, private mmap */
> +		map(SL_SETUP, 1, MAP_PRIVATE,
> +				"doing mmap for running pre-existing untouched private mapping test");
> +		test_counters("Untouched, private", base_nr);
> +		unmap(SL_SETUP, 1, MAP_PRIVATE,
> +				"doing munmap after running pre-existing untouced private mapping test");
> +
> +		/* Now with a pre-existing touched, shared mmap */
> +		map(SL_SETUP, 1, MAP_SHARED,
> +				"doing mmap for running pre-existing touched shared mapping test");
> +		touch(SL_SETUP, 1, MAP_SHARED,
> +				"touching for running pre-existing touched shared mapping test");
> +		test_counters("Touched, shared", base_nr);
> +		unmap(SL_SETUP, 1, MAP_SHARED,
> +				"doing munmap after running pre-existing touched shared mapping test");
> +
> +		/* Now with a pre-existing touched, private mmap */
> +		map(SL_SETUP, 1, MAP_PRIVATE,
> +				"doing mmap for running pre-existing touched private mapping test");
> +		touch(SL_SETUP, 1, MAP_PRIVATE,
> +				"touching for running pre-existing touched private mapping test");
> +		test_counters("Touched, private", base_nr);
> +		unmap(SL_SETUP, 1, MAP_PRIVATE,
> +				"doing munmap after running pre-existing touched private mapping test");
> +	}
> +	if (base_nr > 3)
> +		tst_res(TPASS, "Hugepages Counters works as expected.");
> +	per_iteration_cleanup();
> +}
> +
> +static void setup(void)
> +{
> +	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
> +	SAFE_FILE_PRINTF(PATH_OC_HPAGES, "%lu", tst_hugepages);
> +	private_resv = kernel_has_private_reservations();
> +}
> +
> +static void cleanup(void)
> +{
> +	per_iteration_cleanup();
> +}
> +
> +static struct tst_test test = {
> +	.needs_root = 1,
> +	.mntpoint = MNTPOINT,
> +	.needs_hugetlbfs = 1,
> +	.save_restore = (const struct tst_path_val[]) {
> +		{PATH_OC_HPAGES, NULL},
> +		{PATH_NR_HPAGES, NULL},
> +		{}
> +	},
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_all = run_test,
> +	.hugepages = {3, TST_NEEDS},
> +};
> +
> -- 
> 2.31.1
> 

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v4 2/7] Hugetlb: Migrating libhugetlbfs counters

[Tarun Sahu]

Hi, 
Thanks for reviewing the patch, Cyril.
Please find my comments inline.
On Nov 16 2022, Cyril Hrubis wrote:
> Hi!
> > +#define CHECK_(fun)	\
> > +		{			\
> > +		if (fun)	\
> > +			break;	\
> > +	}
> 
> This is working around the tooling we have I would rather have a look
> into the checkpatch script to see if we could tweak the rule rather than
> to introduce this messy code.
> 
> Just submit the patch with sane code and I will check for what I can do
> with the script to get rid of the error.
Ok.

--Skip
> > +static long hpage_size;
> > +
> > +	if (t != et) {
> > +		tst_res(TFAIL, "At Line %i:While %s: Bad "MEMINFO_HPAGE_TOTAL
> > +				" expected %li, actual %li", line, desc, et, t);
> 
> We do have tst_res_() that can be called as:
> 
> 	tst_res_(__FILE__, line,
>                  "%s bad " MEMINFO_HPAGE_TOTAL " = %li expected %li",
> 		 desc, et, t);
Ok. Will update it.
> 
> > +out:
> > +	return verify_counters(line, desc, et, ef, er, es);
> > +}
> > +#define set_nr_hugepages(c, d) CHECK_(set_nr_hugepages_(c, d, __LINE__))
> 
> The macro name should be uppercase so that it's clear that it's a macro
> and not a simple function. With that we can also drop the underscore
> from the actual function name too.
> 
I avoided it deliberately knowing that these macros are being called too
often, keeping them in uppercase would have made the code look too messy.
Then, I looked that there was tst_res and similiar macros too which were
kept in smallcase too (which is wrapper to tst_res_ function), That is why
I kept them smallcase.

--skip
> > +static int test_counters_(char *desc, int base_nr)
> > +{
> > +	int fail = 1;
> > +
> > +	tst_res(TINFO, "%s...", desc);
> > +
> > +	do {
> > +		set_nr_hugepages(base_nr, "initializing hugepages pool");
> > +
> > +		/* untouched, shared mmap */
> > +		map(SL_TEST, 1, MAP_SHARED, "doing mmap shared with no touch");
> > +		unmap(SL_TEST, 1, MAP_SHARED, "doing munmap on shared with no touch");
> > +
> > +		/* untouched, private mmap */
> > +		map(SL_TEST, 1, MAP_PRIVATE, "doing mmap private with no touch");
> > +		unmap(SL_TEST, 1, MAP_PRIVATE, "doing munmap private with on touch");
> > +
> > +		/* touched, shared mmap */
> > +		map(SL_TEST, 1, MAP_SHARED, "doing mmap shared followed by touch");
> > +		touch(SL_TEST, 1, MAP_SHARED, "touching the addr after mmap shared");
> > +		unmap(SL_TEST, 1, MAP_SHARED, "doing munmap shared after touch");
> > +
> > +		/* touched, private mmap */
> > +		map(SL_TEST, 1, MAP_PRIVATE, "doing mmap private followed by touch");
> > +		touch(SL_TEST, 1, MAP_PRIVATE, "touching the addr after mmap private");
> > +		unmap(SL_TEST, 1, MAP_PRIVATE, "doing munmap private after touch");
> > +
> > +		/* Explicit resizing during outstanding surplus */
> > +		/* Consume surplus when growing pool */
> > +		map(SL_TEST, 2, MAP_SHARED, "doing mmap to consume surplus");
> > +		set_nr_hugepages(MAX(base_nr, 1), "setting hugepages pool to consume surplus");
> > +
> > +		/* Add pages once surplus is consumed */
> > +		set_nr_hugepages(MAX(base_nr, 3), "Adding more pages after consuming surplus");
> > +
> > +		/* Release free huge pages first */
> > +		set_nr_hugepages(MAX(base_nr, 2), "Releasing free huge pages");
> > +
> > +		/* When shrinking beyond committed level, increase surplus */
> > +		set_nr_hugepages(base_nr, "increasing surplus counts");
> > +
> > +		/* Upon releasing the reservation, reduce surplus counts */
> > +		unmap(SL_TEST, 2, MAP_SHARED, "reducing surplus counts");
> > +		fail = 0;
> > +	} while (0);
> > +
> > +	if (fail)
> > +		return -1;
> > +	tst_res(TINFO, "OK");
> > +	return 0;
> 
> Maybe it would be a bit nicer to have actually two different macros so
> that we don't have to resort to this do {} while (0) trickery e.g.
> 
> #define CHECK_BREAK(cond) if (cond) break;
> #define CHECK_RETURN(cond) if (cond) return -1;
> 
> #define MAP_BREAK(s, h, f, d) CHECK_BREAK(map(s, h, f, d, __LINE__))
> #define MAP_RETURN(s, h, f, d) CHECK_RETURN(map(s, h, f, d, __LINE__))
> 
> Then the check counters would look much better.
> 
> Or we can just stick to the RETURN variants and put the body of the loop
> in the runtest() function into do_interation() function.
> 
I like the second idea, as it will only have one macro. which will keep the
code less messy. But anyway, I had tried this already, if (cond) return; is
not allowed. "make check" throws warning saying, "Macros with flow control
statements should be avoided". Only way I could see is to use break.
> > +
> > -- 
> > 2.31.1
> > 
> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

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

Re: [LTP] [PATCH v4 2/7] Hugetlb: Migrating libhugetlbfs counters

[Cyril Hrubis]

Hi!
> > > +static long hpage_size;
> > > +
> > > +	if (t != et) {
> > > +		tst_res(TFAIL, "At Line %i:While %s: Bad "MEMINFO_HPAGE_TOTAL
> > > +				" expected %li, actual %li", line, desc, et, t);
> > 
> > We do have tst_res_() that can be called as:
> > 
> > 	tst_res_(__FILE__, line,
> >                  "%s bad " MEMINFO_HPAGE_TOTAL " = %li expected %li",
> > 		 desc, et, t);
> Ok. Will update it.
> > 
> > > +out:
> > > +	return verify_counters(line, desc, et, ef, er, es);
> > > +}
> > > +#define set_nr_hugepages(c, d) CHECK_(set_nr_hugepages_(c, d, __LINE__))
> > 
> > The macro name should be uppercase so that it's clear that it's a macro
> > and not a simple function. With that we can also drop the underscore
> > from the actual function name too.
> > 
> I avoided it deliberately knowing that these macros are being called too
> often, keeping them in uppercase would have made the code look too messy.
> Then, I looked that there was tst_res and similiar macros too which were
> kept in smallcase too (which is wrapper to tst_res_ function), That is why
> I kept them smallcase.

The whole point why we are going in circles around how to structure this
piece of code is code readibility. Source code is more often read than
written, which means that it's more important to invest into writing
code that is easily understandable since that will save effort in the
long term. That is true especially for kernel tests, where there is
enough complexity in the kernel itself and writing tests that are not
easy to understand does actually harm.

There is a key difference between tst_res() defined to tst_res_() that
adds a few parameters and macros that change the codeflow. If there are
macros that change code flow, and perhaps checkpatch is right here that
it's wiser to avoid them at all, they should at least scream in upper
case letters that this is not a regular function.

This all boils down to a principle of a least surprise.

Perhaps the best solution would be to get back to a drawing board and
figure out how to better structure the test so that the code flow is
easier to follow.

> > Maybe it would be a bit nicer to have actually two different macros so
> > that we don't have to resort to this do {} while (0) trickery e.g.
> > 
> > #define CHECK_BREAK(cond) if (cond) break;
> > #define CHECK_RETURN(cond) if (cond) return -1;
> > 
> > #define MAP_BREAK(s, h, f, d) CHECK_BREAK(map(s, h, f, d, __LINE__))
> > #define MAP_RETURN(s, h, f, d) CHECK_RETURN(map(s, h, f, d, __LINE__))
> > 
> > Then the check counters would look much better.
> > 
> > Or we can just stick to the RETURN variants and put the body of the loop
> > in the runtest() function into do_interation() function.
> > 
> I like the second idea, as it will only have one macro. which will keep the
> code less messy. But anyway, I had tried this already, if (cond) return; is
> not allowed. "make check" throws warning saying, "Macros with flow control
> statements should be avoided". Only way I could see is to use break.

Again, let's not work around the tooling we have, the tooling is
supposed to help, once you start bending the code so that the tooling is
happy you are on a wrong path.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v4 2/7] Hugetlb: Migrating libhugetlbfs counters

[Tarun Sahu]

Hi,
Thanks Cyril, for the such a good insights. I will rework on this test.

~Tarun

On Fri, 2022-11-18 at 15:48 +0100, Cyril Hrubis wrote:
> Hi!
> > > > +static long hpage_size;
> > > > +
> > > > +	if (t != et) {
> > > > +		tst_res(TFAIL, "At Line %i:While %s: Bad
> > > > "MEMINFO_HPAGE_TOTAL
> > > > +				" expected %li, actual %li",
> > > > line, desc, et, t);
> > > 
> > > We do have tst_res_() that can be called as:
> > > 
> > > 	tst_res_(__FILE__, line,
> > >                  "%s bad " MEMINFO_HPAGE_TOTAL " = %li expected
> > > %li",
> > > 		 desc, et, t);
> > Ok. Will update it.
> > > > +out:
> > > > +	return verify_counters(line, desc, et, ef, er, es);
> > > > +}
> > > > +#define set_nr_hugepages(c, d) CHECK_(set_nr_hugepages_(c, d,
> > > > __LINE__))
> > > 
> > > The macro name should be uppercase so that it's clear that it's a
> > > macro
> > > and not a simple function. With that we can also drop the
> > > underscore
> > > from the actual function name too.
> > > 
> > I avoided it deliberately knowing that these macros are being
> > called too
> > often, keeping them in uppercase would have made the code look too
> > messy.
> > Then, I looked that there was tst_res and similiar macros too which
> > were
> > kept in smallcase too (which is wrapper to tst_res_ function), That
> > is why
> > I kept them smallcase.
> 
> The whole point why we are going in circles around how to structure
> this
> piece of code is code readibility. Source code is more often read
> than
> written, which means that it's more important to invest into writing
> code that is easily understandable since that will save effort in the
> long term. That is true especially for kernel tests, where there is
> enough complexity in the kernel itself and writing tests that are not
> easy to understand does actually harm.
> 
> There is a key difference between tst_res() defined to tst_res_()
> that
> adds a few parameters and macros that change the codeflow. If there
> are
> macros that change code flow, and perhaps checkpatch is right here
> that
> it's wiser to avoid them at all, they should at least scream in upper
> case letters that this is not a regular function.
> 
> This all boils down to a principle of a least surprise.
> 
> Perhaps the best solution would be to get back to a drawing board and
> figure out how to better structure the test so that the code flow is
> easier to follow.
> 


> > > Maybe it would be a bit nicer to have actually two different
> > > macros so
> > > that we don't have to resort to this do {} while (0) trickery
> > > e.g.
> > > 
> > > #define CHECK_BREAK(cond) if (cond) break;
> > > #define CHECK_RETURN(cond) if (cond) return -1;
> > > 
> > > #define MAP_BREAK(s, h, f, d) CHECK_BREAK(map(s, h, f, d,
> > > __LINE__))
> > > #define MAP_RETURN(s, h, f, d) CHECK_RETURN(map(s, h, f, d,
> > > __LINE__))
> > > 
> > > Then the check counters would look much better.
> > > 
> > > Or we can just stick to the RETURN variants and put the body of
> > > the loop
> > > in the runtest() function into do_interation() function.
> > > 
> > I like the second idea, as it will only have one macro. which will
> > keep the
> > code less messy. But anyway, I had tried this already, if (cond)
> > return; is
> > not allowed. "make check" throws warning saying, "Macros with flow
> > control
> > statements should be avoided". Only way I could see is to use
> > break.
> 
> Again, let's not work around the tooling we have, the tooling is
> supposed to help, once you start bending the code so that the tooling
> is
> happy you are on a wrong path.
> 


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

[LTP] [PATCH v4 3/7] Hugetlb: Migrating libhugetlbfs directio

[Tarun Sahu]

Migrating the libhugetlbfs/testcases/direct.c test

Test Description: This Test perform Direct Write/Read from/To hugetlbfs
file which is mapped to process address space. The test is checking if it
succeeds and data written or read is not corrupted.

Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com>
---
 runtest/hugetlb                               |  1 +
 testcases/kernel/mem/.gitignore               |  1 +
 .../kernel/mem/hugetlb/hugemmap/hugemmap11.c  | 87 +++++++++++++++++++
 3 files changed, 89 insertions(+)
 create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap11.c

diff --git a/runtest/hugetlb b/runtest/hugetlb
index 8a56d52a3..b9ee7227d 100644
--- a/runtest/hugetlb
+++ b/runtest/hugetlb
@@ -7,6 +7,7 @@ hugemmap07 hugemmap07
 hugemmap08 hugemmap08
 hugemmap09 hugemmap09
 hugemmap10 hugemmap10
+hugemmap11 hugemmap11
 hugemmap05_1 hugemmap05 -m
 hugemmap05_2 hugemmap05 -s
 hugemmap05_3 hugemmap05 -s -m
diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore
index e7def68cb..3e64b67be 100644
--- a/testcases/kernel/mem/.gitignore
+++ b/testcases/kernel/mem/.gitignore
@@ -8,6 +8,7 @@
 /hugetlb/hugemmap/hugemmap08
 /hugetlb/hugemmap/hugemmap09
 /hugetlb/hugemmap/hugemmap10
+/hugetlb/hugemmap/hugemmap11
 /hugetlb/hugeshmat/hugeshmat01
 /hugetlb/hugeshmat/hugeshmat02
 /hugetlb/hugeshmat/hugeshmat03
diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap11.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap11.c
new file mode 100644
index 000000000..48ecfe9d8
--- /dev/null
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap11.c
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+/*
+ * Copyright (C) 2005-2006 IBM Corporation.
+ * Author: David Gibson & Adam Litke
+ */
+
+/*\
+ * [Description]
+ *
+ * This Test perform Direct Write/Read from/To hugetlbfs file
+ * which is mapped to process address space. The test is checking if it
+ * succeeds and data written or read is not corrupted.
+ *
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <sys/mount.h>
+#include <limits.h>
+#include <sys/param.h>
+#include <sys/types.h>
+
+#include "hugetlb.h"
+
+#define P0 "ffffffff"
+#define IOSZ 4096
+#define NORMAL_PATH ""
+#define MNTPOINT "hugetlbfs/"
+
+static long hpage_size;
+static int fd = -1, nfd = -1;
+
+static void run_test(void)
+{
+	void *p;
+	char buf[IOSZ] __attribute__((aligned(IOSZ)));
+
+	fd = tst_creat_unlinked(MNTPOINT, 0);
+	nfd = tst_creat_unlinked(NORMAL_PATH, O_DIRECT);
+	p = SAFE_MMAP(NULL, hpage_size, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0);
+	memcpy(p, P0, 8);
+
+	SAFE_WRITE(1, nfd, p, IOSZ);
+	SAFE_LSEEK(nfd, 0, SEEK_SET);
+
+	SAFE_READ(1, nfd, buf, IOSZ);
+	if (memcmp(P0, buf, 8)) {
+		tst_res(TFAIL, "Memory mismatch after Direct-IO write");
+		goto cleanup;
+	}
+	SAFE_LSEEK(nfd, 0, SEEK_SET);
+
+	memset(p, 0, IOSZ);
+	SAFE_READ(1, nfd, p, IOSZ);
+
+	if (memcmp(p, P0, 8))
+		tst_res(TFAIL, "Memory mismatch after Direct-IO read");
+	else
+		tst_res(TPASS, "Direct-IO write and read from/to hugepages is successful");
+cleanup:
+	SAFE_MUNMAP(p, hpage_size);
+	SAFE_CLOSE(fd);
+	SAFE_CLOSE(nfd);
+}
+
+static void setup(void)
+{
+	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
+}
+
+static void cleanup(void)
+{
+	if (fd > 0)
+		SAFE_CLOSE(fd);
+	if (nfd > 0)
+		SAFE_CLOSE(nfd);
+}
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.mntpoint = MNTPOINT,
+	.needs_hugetlbfs = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run_test,
+	.hugepages = {1, TST_NEEDS},
+};
-- 
2.31.1


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

[LTP] [PATCH v4 4/7] Hugetlb: Safe macro for posix_fadvise call

[Tarun Sahu]

This patch adds SAFE_POSIX_FADVISE for posix_fadvise call.

Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com>
---
 include/tst_safe_macros.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index 81c4b0844..4965e44d0 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -298,6 +298,23 @@ static inline int safe_ftruncate(const char *file, const int lineno,
 #define SAFE_FTRUNCATE(fd, length) \
 	safe_ftruncate(__FILE__, __LINE__, (fd), (length))
 
+static inline int safe_posix_fadvise(const char *file, const int lineno,
+                                int fd, off_t offset, off_t len, int advice)
+{
+	int rval;
+
+	rval = posix_fadvise(fd, offset, len, advice);
+
+	if (rval)
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"posix_fadvise(%d,%ld,%ld,%d) failed",
+			fd, (long)offset, (long)len, advice);
+
+	return rval;
+}
+#define SAFE_POSIX_FADVISE(fd, offset, len, advice) \
+	safe_posix_fadvise(__FILE__, __LINE__, (fd), (offset), (len), (advice))
+
 static inline int safe_truncate(const char *file, const int lineno,
                                 const char *path, off_t length)
 {
-- 
2.31.1


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

[LTP] [PATCH v4 5/7] Hugetlb: Migrating libhugetlbfs fadvise_reserve

[Tarun Sahu]

Migrating the libhugetlbfs/testcases/fadvise_reserve.c test

Test Description: fadvise() on some kernels can cause the reservation
counter to get corrupted. The problem is that the patches are allocated for
the reservation but not faulted in at the time of allocation. The counters
do not get updated and effectively "leak". This test identifies whether the
kernel is vulnerable to the problem or not. It's fixed in kernel by 'commit
f2deae9d4e70 ("Remove implementation of readpage from the hugetlbfs_aops")'

Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com>
---
 runtest/hugetlb                               |  1 +
 testcases/kernel/mem/.gitignore               |  1 +
 .../kernel/mem/hugetlb/hugemmap/hugemmap12.c  | 84 +++++++++++++++++++
 3 files changed, 86 insertions(+)
 create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap12.c

diff --git a/runtest/hugetlb b/runtest/hugetlb
index b9ee7227d..b019c4195 100644
--- a/runtest/hugetlb
+++ b/runtest/hugetlb
@@ -8,6 +8,7 @@ hugemmap08 hugemmap08
 hugemmap09 hugemmap09
 hugemmap10 hugemmap10
 hugemmap11 hugemmap11
+hugemmap12 hugemmap12
 hugemmap05_1 hugemmap05 -m
 hugemmap05_2 hugemmap05 -s
 hugemmap05_3 hugemmap05 -s -m
diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore
index 3e64b67be..ec250592d 100644
--- a/testcases/kernel/mem/.gitignore
+++ b/testcases/kernel/mem/.gitignore
@@ -9,6 +9,7 @@
 /hugetlb/hugemmap/hugemmap09
 /hugetlb/hugemmap/hugemmap10
 /hugetlb/hugemmap/hugemmap11
+/hugetlb/hugemmap/hugemmap12
 /hugetlb/hugeshmat/hugeshmat01
 /hugetlb/hugeshmat/hugeshmat02
 /hugetlb/hugeshmat/hugeshmat03
diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap12.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap12.c
new file mode 100644
index 000000000..ae132107c
--- /dev/null
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap12.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+/*
+ * Copyright (C) 2005-2006 IBM Corporation.
+ * Author: Mel Gorman
+ */
+
+/*\
+ * [Description]
+ *
+ * fadvise() on some kernels can cause the reservation counter to get
+ * corrupted. The problem is that the patches are allocated for the
+ * reservation but not faulted in at the time of allocation. The counters
+ * do not get updated and effectively "leak". This test identifies whether
+ * the kernel is vulnerable to the problem or not. It's fixed in kernel
+ * by commit f2deae9d4e70793568ef9e85d227abb7bef5b622.
+ *
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <sys/mount.h>
+#include <limits.h>
+#include <sys/param.h>
+#include <sys/types.h>
+
+#include "hugetlb.h"
+
+#define MNTPOINT "hugetlbfs/"
+static long hpage_size;
+static int  fd = -1;
+
+static void run_test(void)
+{
+	void *p;
+	unsigned long initial_rsvd, map_rsvd, fadvise_rsvd, end_rsvd;
+
+	fd = tst_creat_unlinked(MNTPOINT, 0);
+
+	initial_rsvd = SAFE_READ_MEMINFO(MEMINFO_HPAGE_RSVD);
+	tst_res(TINFO, "Reserve count before map: %lu", initial_rsvd);
+
+	p = SAFE_MMAP(NULL, hpage_size, PROT_READ|PROT_WRITE, MAP_SHARED,
+		 fd, 0);
+	map_rsvd = SAFE_READ_MEMINFO(MEMINFO_HPAGE_RSVD);
+	tst_res(TINFO, "Reserve count after map: %lu", map_rsvd);
+
+	SAFE_POSIX_FADVISE(fd, 0, hpage_size, POSIX_FADV_WILLNEED);
+	fadvise_rsvd = SAFE_READ_MEMINFO(MEMINFO_HPAGE_RSVD);
+	tst_res(TINFO, "Reserve count after fadvise: %lu", fadvise_rsvd);
+
+	memset(p, 1, hpage_size);
+
+	SAFE_MUNMAP(p, hpage_size);
+	SAFE_CLOSE(fd);
+	end_rsvd = SAFE_READ_MEMINFO(MEMINFO_HPAGE_RSVD);
+	tst_res(TINFO, "Reserve count after close: %lu", end_rsvd);
+
+	TST_EXP_EQ_LU(end_rsvd, initial_rsvd);
+}
+
+static void setup(void)
+{
+	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
+}
+
+static void cleanup(void)
+{
+	if (fd > 0)
+		SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.tags = (struct tst_tag[]) {
+		{"linux-git", "f2deae9d4e70"},
+		{}
+	},
+	.needs_root = 1,
+	.mntpoint = MNTPOINT,
+	.needs_hugetlbfs = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run_test,
+	.hugepages = {1, TST_NEEDS},
+};
-- 
2.31.1


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

[LTP] [PATCH v4 6/7] Hugetlb: Migrating libhugetlbfs fallocate_align

[Tarun Sahu]

Migrating the libhugetlbfs/testcases/fallocate_align.c test

Test Description: It tests alignment of fallocate arguments. fallocate will
take non-huge page aligned offsets and addresses.  However, operations are
only performed on huge pages.  This is different that than fallocate
behavior in "normal" filesystems.

Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com>
---
 runtest/hugetlb                               |   2 +
 testcases/kernel/mem/.gitignore               |   1 +
 .../kernel/mem/hugetlb/hugefallocate/Makefile |  10 ++
 .../hugetlb/hugefallocate/hugefallocate01.c   | 169 ++++++++++++++++++
 4 files changed, 182 insertions(+)
 create mode 100644 testcases/kernel/mem/hugetlb/hugefallocate/Makefile
 create mode 100644 testcases/kernel/mem/hugetlb/hugefallocate/hugefallocate01.c

diff --git a/runtest/hugetlb b/runtest/hugetlb
index b019c4195..ca92dfcff 100644
--- a/runtest/hugetlb
+++ b/runtest/hugetlb
@@ -1,3 +1,5 @@
+hugefallocate01 hugefallocate01
+
 hugemmap01 hugemmap01
 hugemmap02 hugemmap02
 hugemmap04 hugemmap04
diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore
index ec250592d..cafdb5259 100644
--- a/testcases/kernel/mem/.gitignore
+++ b/testcases/kernel/mem/.gitignore
@@ -1,4 +1,5 @@
 /cpuset/cpuset01
+/hugetlb/hugefallocate/hugefallocate01
 /hugetlb/hugemmap/hugemmap01
 /hugetlb/hugemmap/hugemmap02
 /hugetlb/hugemmap/hugemmap04
diff --git a/testcases/kernel/mem/hugetlb/hugefallocate/Makefile b/testcases/kernel/mem/hugetlb/hugefallocate/Makefile
new file mode 100644
index 000000000..77ebb0aef
--- /dev/null
+++ b/testcases/kernel/mem/hugetlb/hugefallocate/Makefile
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (C) 2009, Cisco Systems Inc.
+# Ngie Cooper, July 2009
+
+top_srcdir		?= ../../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+include $(abs_srcdir)/../Makefile.inc
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
+
diff --git a/testcases/kernel/mem/hugetlb/hugefallocate/hugefallocate01.c b/testcases/kernel/mem/hugetlb/hugefallocate/hugefallocate01.c
new file mode 100644
index 000000000..1ec0dd000
--- /dev/null
+++ b/testcases/kernel/mem/hugetlb/hugefallocate/hugefallocate01.c
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+/*
+ * Copyright (C) 2015 Oracle Corporation
+ * Author: Mike Kravetz
+ */
+
+/*\
+ * [Description]
+ *
+ * It tests alignment of fallocate arguments. fallocate will take non-huge
+ * page aligned offsets and addresses.  However, operations are only
+ * performed on huge pages.  This is different that than fallocate
+ * behavior in "normal" filesystems.
+ *
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <sys/mount.h>
+#include <limits.h>
+#include <sys/param.h>
+#include <sys/types.h>
+
+#include "hugetlb.h"
+
+#define MNTPOINT "hugetlbfs/"
+
+static int  fd = -1;
+static long hpage_size;
+
+static void run_test(void)
+{
+	int err;
+	unsigned long free_initial, free_after, free_after_delete;
+
+	fd = tst_creat_unlinked(MNTPOINT, 0);
+
+	free_initial = SAFE_READ_MEMINFO(MEMINFO_HPAGE_FREE);
+
+	/*
+	 * First preallocate file with just 1 byte.  Allocation sizes
+	 * are rounded up, so we should get an entire huge page.
+	 */
+	err = fallocate(fd, 0, 0, 1);
+	if (err) {
+		if (errno == EOPNOTSUPP)
+			tst_brk(TCONF, "Operation Not Supported");
+		tst_res(TFAIL|TERRNO, "fallocate()");
+		goto cleanup;
+	}
+
+	free_after = SAFE_READ_MEMINFO(MEMINFO_HPAGE_FREE);
+	if (free_initial - free_after != 1) {
+		tst_res(TFAIL, "fallocate 1 byte did not preallocate entire huge page");
+		goto cleanup;
+	}
+
+	/*
+	 * Now punch a hole with just 1 byte.  On hole punch, sizes are
+	 * rounded down. So, this operation should not create a hole.
+	 */
+	err = fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
+			0, 1);
+	if (err) {
+		tst_res(TFAIL|TERRNO, "fallocate(FALLOC_FL_PUNCH_HOLE)");
+		goto cleanup;
+	}
+
+	free_after = SAFE_READ_MEMINFO(MEMINFO_HPAGE_FREE);
+	if (free_after == free_initial) {
+		tst_res(TFAIL, "fallocate hole punch 1 byte free'ed a huge page");
+		goto cleanup;
+	}
+
+	/*
+	 * Now punch a hole with of 2 * hpage_size - 1 byte.  This size
+	 * should be rounded down to a single huge page and the hole created.
+	 */
+	err = fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
+			0, (2 * hpage_size) - 1);
+	if (err) {
+		tst_res(TFAIL|TERRNO, "fallocate(FALLOC_FL_PUNCH_HOLE)");
+		goto cleanup;
+	}
+
+	free_after = SAFE_READ_MEMINFO(MEMINFO_HPAGE_FREE);
+	if (free_after != free_initial) {
+		tst_res(TFAIL, "fallocate hole punch 2 * hpage_size - 1 byte did not"
+				" free huge page");
+		goto cleanup;
+	}
+
+	/*
+	 * Perform a preallocate operation with offset 1 and size of
+	 * hpage_size.  The offset should be rounded down and the
+	 * size rounded up to preallocate two huge pages.
+	 */
+	err = fallocate(fd, 0, 1, hpage_size);
+	if (err) {
+		tst_res(TFAIL, "fallocate()");
+		goto cleanup;
+	}
+
+	free_after = SAFE_READ_MEMINFO(MEMINFO_HPAGE_FREE);
+	if (free_initial - free_after != 2) {
+		tst_res(TFAIL, "fallocate 1 byte offset, huge page size did not"
+				" preallocate two huge pages");
+		goto cleanup;
+	}
+
+	/*
+	 * The hole punch code will only delete 'whole' huge pags that are
+	 * in the specified range.  The offset is rounded up, and (offset
+	 * + size) is rounded down to determine the huge pages to be deleted.
+	 * In this case, after rounding the range is (hpage_size, hpage_size).
+	 * So, no pages should be deleted.
+	 */
+	err = fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
+			1, hpage_size);
+	if (err) {
+		tst_res(TFAIL|TERRNO, "fallocate(FALLOC_FL_PUNCH_HOLE)");
+		goto cleanup;
+	}
+
+	free_after = SAFE_READ_MEMINFO(MEMINFO_HPAGE_FREE);
+	if (free_initial - free_after != 2) {
+		tst_res(TFAIL, "fallocate hole punch 1 byte offset, huge page size"
+				" incorrectly deleted a huge page");
+		goto cleanup;
+	}
+
+	/*
+	 * To delete both huge pages, the range passed to hole punch must
+	 * overlap the allocated pages
+	 */
+	err = fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
+			0, 2 * hpage_size);
+	if (err) {
+		tst_res(TFAIL|TERRNO, "fallocate(FALLOC_FL_PUNCH_HOLE)");
+		goto cleanup;
+	}
+
+	free_after_delete = SAFE_READ_MEMINFO(MEMINFO_HPAGE_FREE);
+	TST_EXP_EQ_LU(free_after_delete, free_initial);
+cleanup:
+	SAFE_CLOSE(fd);
+}
+
+static void setup(void)
+{
+	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
+}
+
+static void cleanup(void)
+{
+	if (fd > 0)
+		SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.mntpoint = MNTPOINT,
+	.needs_hugetlbfs = 1,
+	.needs_tmpdir = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run_test,
+	.hugepages = {2, TST_NEEDS},
+};
-- 
2.31.1


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

[LTP] [PATCH v4 7/7] Hugetlb: Migrating libhugetlbfs fallocate_basic

[Tarun Sahu]

Migrating the libhugetlbfs/testcases/fallocate_basic.c test

Test Description: It tests basic fallocate functionality in hugetlbfs.
Preallocate huge pages to a file in hugetlbfs, and then remove the pages
via hole punch.

Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com>
---
 runtest/hugetlb                               |  1 +
 testcases/kernel/mem/.gitignore               |  1 +
 .../hugetlb/hugefallocate/hugefallocate02.c   | 90 +++++++++++++++++++
 3 files changed, 92 insertions(+)
 create mode 100644 testcases/kernel/mem/hugetlb/hugefallocate/hugefallocate02.c

diff --git a/runtest/hugetlb b/runtest/hugetlb
index ca92dfcff..ec1fc2515 100644
--- a/runtest/hugetlb
+++ b/runtest/hugetlb
@@ -1,4 +1,5 @@
 hugefallocate01 hugefallocate01
+hugefallocate02 hugefallocate02
 
 hugemmap01 hugemmap01
 hugemmap02 hugemmap02
diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore
index cafdb5259..c0906f3d3 100644
--- a/testcases/kernel/mem/.gitignore
+++ b/testcases/kernel/mem/.gitignore
@@ -1,5 +1,6 @@
 /cpuset/cpuset01
 /hugetlb/hugefallocate/hugefallocate01
+/hugetlb/hugefallocate/hugefallocate02
 /hugetlb/hugemmap/hugemmap01
 /hugetlb/hugemmap/hugemmap02
 /hugetlb/hugemmap/hugemmap04
diff --git a/testcases/kernel/mem/hugetlb/hugefallocate/hugefallocate02.c b/testcases/kernel/mem/hugetlb/hugefallocate/hugefallocate02.c
new file mode 100644
index 000000000..d1e11dcbe
--- /dev/null
+++ b/testcases/kernel/mem/hugetlb/hugefallocate/hugefallocate02.c
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+/*
+ * Copyright (C) 2015 Oracle Corporation
+ * Author: Mike Kravetz
+ */
+
+/*\
+ * [Description]
+ *
+ * It tests basic fallocate functionality in hugetlbfs. Preallocate huge
+ * pages to a file in hugetlbfs, and then remove the pages via hole punch.
+ *
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <sys/mount.h>
+#include <limits.h>
+#include <sys/param.h>
+#include <sys/types.h>
+
+#include "hugetlb.h"
+
+#define MAX_PAGES_TO_USE 5
+#define MNTPOINT "hugetlbfs/"
+
+static int  fd = -1;
+static long hpage_size;
+
+static void run_test(void)
+{
+	int err;
+	unsigned long max_iterations;
+	unsigned long free_initial, free_after, free_end;
+
+	free_initial = SAFE_READ_MEMINFO(MEMINFO_HPAGE_FREE);
+	max_iterations = MIN(free_initial, MAX_PAGES_TO_USE);
+
+	fd = tst_creat_unlinked(MNTPOINT, 0);
+
+	/* First preallocate file with max_iterations pages */
+	err = fallocate(fd, 0, 0, hpage_size * max_iterations);
+	if (err) {
+		if (errno == EOPNOTSUPP)
+			tst_brk(TCONF, "fallocate() Operation is not supported");
+		tst_res(TFAIL|TERRNO, "fallocate()");
+		goto cleanup;
+	}
+
+	free_after = SAFE_READ_MEMINFO(MEMINFO_HPAGE_FREE);
+	if (free_initial - free_after != max_iterations) {
+		tst_res(TFAIL, "fallocate did not preallocate %lu huge pages",
+							max_iterations);
+		goto cleanup;
+	}
+
+	/* Now punch a hole of the same size */
+	err = fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
+			0, hpage_size * max_iterations);
+	if (err) {
+		tst_res(TFAIL|TERRNO, "fallocate(FALLOC_FL_PUNCH_HOLE)");
+		goto cleanup;
+	}
+
+	free_end = SAFE_READ_MEMINFO(MEMINFO_HPAGE_FREE);
+	TST_EXP_EQ_LU(free_end, free_initial);
+cleanup:
+	SAFE_CLOSE(fd);
+}
+
+static void setup(void)
+{
+	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
+}
+
+static void cleanup(void)
+{
+	if (fd > 0)
+		SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.mntpoint = MNTPOINT,
+	.needs_hugetlbfs = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run_test,
+	.hugepages = {3, TST_NEEDS},
+};
-- 
2.31.1


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