[LTP] [PATCH v2 00/13] Collection of fixes

[LTP] [PATCH v2 00/13] Collection of fixes

[Punit Agrawal]

Hi,

This is the second posting of that collects fixes for issues
encountered when running ltp on internal test platforms. Previous
version can be found at [0]. This update addresses all the feedback
received on the previous version.

Changes:

v2:
* removed check_hugepage() prototype from hugetlb.h (patch 1)
* hugeshmctl01 - migrate to using checkpoint synchronisation primitives
* sigwaitinfo - use child process to execute test instead of using
  longjmp based solution
* Dropped patches that are no longer applicable or already merged

If there aren't anymore comments, please consider them for merging.

Thanks,
Punit

[0] http://lists.linux.it/pipermail/ltp/2017-October/006225.html

James Morse (2):
  hotplug/cpu_hotplug: Repopulate cgroup:cpusets after testing hotplug
  hotplug/cpu_hotplug: Remove bashism disown from kill_pid()

Lorenzo Pieralisi (1):
  hugeshmget02: add missing SHM_HUGETLB flag on segment creation

Punit Agrawal (4):
  Move check_hugepage() helper to mem/lib
  hugeshmctl01: Convert to LTP synchronisation primitives
  hugeshmctl01: Fix warning about signed/unsigned comparison
  sigwaitinfo01: catch SEGV and report success for bad_address2 testcase

Suzuki K. Poulose (3):
  hugeshmctl02: Fix allocation size for odd number of hugepages
  getdtablesize01: Handle ENFILE errno
  perf_event_open: Handle absence of PMU gracefully

Will Deacon (3):
  thp: ensure THP/hugetlbfs is available
  sigwaitinfo01: fix race between sending and dequeueing RT signals
  syscalls/mount03: Copy setuid_test to execute instead of 'TEST FILE'

 .../cpu_hotplug/include/cpuhotplug_hotplug.sh      | 36 ++++++++++
 .../cpu_hotplug/include/cpuhotplug_testsuite.sh    |  1 -
 .../kernel/mem/hugetlb/hugeshmctl/hugeshmctl01.c   | 77 ++++------------------
 .../kernel/mem/hugetlb/hugeshmctl/hugeshmctl02.c   |  2 +-
 .../kernel/mem/hugetlb/hugeshmget/hugeshmget02.c   |  3 +-
 testcases/kernel/mem/hugetlb/lib/hugetlb.c         |  6 --
 testcases/kernel/mem/hugetlb/lib/hugetlb.h         |  3 -
 testcases/kernel/mem/include/mem.h                 |  2 +
 testcases/kernel/mem/lib/mem.c                     |  6 ++
 testcases/kernel/mem/thp/thp01.c                   |  3 +
 testcases/kernel/mem/thp/thp02.c                   |  2 +
 testcases/kernel/mem/thp/thp03.c                   |  2 +
 .../syscalls/getdtablesize/getdtablesize01.c       | 17 +++--
 testcases/kernel/syscalls/mount/mount03.c          |  4 +-
 .../syscalls/perf_event_open/perf_event_open01.c   |  3 +-
 .../syscalls/perf_event_open/perf_event_open02.c   |  4 +-
 .../kernel/syscalls/sigwaitinfo/sigwaitinfo01.c    | 34 +++++++++-
 17 files changed, 117 insertions(+), 88 deletions(-)

-- 
2.14.2

[LTP] [PATCH v2 01/13] Move check_hugepage() helper to mem/lib

[Punit Agrawal]

Move the check_hugepage() helper to mem/lib to be more widely available
to testcases. Specifically, it will be used in a subsequent commit to
check for the presence of hugepage support when running the thp tests.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 testcases/kernel/mem/hugetlb/lib/hugetlb.c | 6 ------
 testcases/kernel/mem/hugetlb/lib/hugetlb.h | 3 ---
 testcases/kernel/mem/include/mem.h         | 2 ++
 testcases/kernel/mem/lib/mem.c             | 6 ++++++
 4 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/testcases/kernel/mem/hugetlb/lib/hugetlb.c b/testcases/kernel/mem/hugetlb/lib/hugetlb.c
index 7afc7d4b4..2f86e3558 100644
--- a/testcases/kernel/mem/hugetlb/lib/hugetlb.c
+++ b/testcases/kernel/mem/hugetlb/lib/hugetlb.c
@@ -40,12 +40,6 @@
 #include <pwd.h>
 #include "hugetlb.h"
 
-void check_hugepage(void)
-{
-	if (access(PATH_HUGEPAGES, F_OK))
-		tst_brk(TCONF, "Huge page is not supported.");
-}
-
 /*
  * getipckey() - generates and returns a message key used by the "get"
  *		 calls to create an IPC resource.
diff --git a/testcases/kernel/mem/hugetlb/lib/hugetlb.h b/testcases/kernel/mem/hugetlb/lib/hugetlb.h
index c6d2016b7..76c1e8f30 100644
--- a/testcases/kernel/mem/hugetlb/lib/hugetlb.h
+++ b/testcases/kernel/mem/hugetlb/lib/hugetlb.h
@@ -45,12 +45,9 @@
  * from shmid_ds.ipc_perm.mode
  */
 #define MODE_MASK	0x01FF
-#define PATH_HUGEPAGES	"/sys/kernel/mm/hugepages/"
 
 key_t shmkey;			/* an IPC key generated by ftok() */
 
-void check_hugepage(void);
-
 int getipckey(void);
 int getuserid(char *user);
 void rm_shm(int shm_id);
diff --git a/testcases/kernel/mem/include/mem.h b/testcases/kernel/mem/include/mem.h
index 287f8b3f6..95d0bda72 100644
--- a/testcases/kernel/mem/include/mem.h
+++ b/testcases/kernel/mem/include/mem.h
@@ -56,8 +56,10 @@ void test_ksm_merge_across_nodes(unsigned long nr_pages);
 
 /* HUGETLB */
 
+#define PATH_HUGEPAGES		"/sys/kernel/mm/hugepages/"
 #define PATH_SHMMAX		"/proc/sys/kernel/shmmax"
 
+void check_hugepage(void);
 void write_memcg(void);
 
 /* cpuset/memcg */
diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c
index fc2f130f4..7f2099b9e 100644
--- a/testcases/kernel/mem/lib/mem.c
+++ b/testcases/kernel/mem/lib/mem.c
@@ -347,6 +347,12 @@ static void verify(char **memory, char value, int proc,
 	free(s);
 }
 
+void check_hugepage(void)
+{
+	if (access(PATH_HUGEPAGES, F_OK))
+		tst_brk(TCONF, "Huge page is not supported.");
+}
+
 void write_memcg(void)
 {
 	SAFE_FILE_PRINTF(MEMCG_LIMIT, "%ld", TESTMEM);
-- 
2.14.2

[LTP] [PATCH v2 02/13] thp: ensure THP/hugetlbfs is available

[Punit Agrawal]

From: Will Deacon <will.deacon@arm.com>

The THP tests can't pass if THP is unavailable or disabled. Add the
missing detection to thp01.c.

Also add tests for the presence of hugetlbfs for tests relying on this
kernel feature.

Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 testcases/kernel/mem/thp/thp01.c | 3 +++
 testcases/kernel/mem/thp/thp02.c | 2 ++
 testcases/kernel/mem/thp/thp03.c | 2 ++
 3 files changed, 7 insertions(+)

diff --git a/testcases/kernel/mem/thp/thp01.c b/testcases/kernel/mem/thp/thp01.c
index 2f2bb2bdb..939c01b98 100644
--- a/testcases/kernel/mem/thp/thp01.c
+++ b/testcases/kernel/mem/thp/thp01.c
@@ -116,6 +116,9 @@ static void setup(void)
 	int i;
 	long arg_len, arg_count;
 
+	if (access(PATH_THP, F_OK) == -1)
+		tst_brk(TCONF, "THP not enabled in kernel?");
+
 	bst = SAFE_MMAP(NULL, sizeof(*bst),
 			   PROT_READ | PROT_WRITE,
 			   MAP_SHARED | MAP_ANONYMOUS, -1, 0);
diff --git a/testcases/kernel/mem/thp/thp02.c b/testcases/kernel/mem/thp/thp02.c
index acc70e215..6502210e5 100644
--- a/testcases/kernel/mem/thp/thp02.c
+++ b/testcases/kernel/mem/thp/thp02.c
@@ -90,6 +90,8 @@ static void setup(void)
 	if (access(PATH_THP, F_OK) == -1)
 		tst_brk(TCONF, "THP not enabled in kernel?");
 
+	check_hugepage();
+
 	ps = sysconf(_SC_PAGESIZE);
 	hps = SAFE_READ_MEMINFO("Hugepagesize:") * 1024;
 	size = hps * 4;
diff --git a/testcases/kernel/mem/thp/thp03.c b/testcases/kernel/mem/thp/thp03.c
index 19db44944..c6062505f 100644
--- a/testcases/kernel/mem/thp/thp03.c
+++ b/testcases/kernel/mem/thp/thp03.c
@@ -82,6 +82,8 @@ static void setup(void)
 	if (access(PATH_THP, F_OK) == -1)
 		tst_brk(TCONF, "THP not enabled in kernel?");
 
+	check_hugepage();
+
 	hugepage_size = SAFE_READ_MEMINFO("Hugepagesize:") * KB;
 	unaligned_size = hugepage_size * 4 - 1;
 	page_size = SAFE_SYSCONF(_SC_PAGESIZE);
-- 
2.14.2

[LTP] [PATCH v2 03/13] hugeshmctl01: Convert to LTP synchronisation primitives

[Punit Agrawal]

hugeshmctl01 spawns child processes to perform shm operations and uses
signal (SIGUSR1) to communicate the completion of test 1, to the
children waiting at sigprocmask(). However, there is no guarantee that a
child has reached sigprocmask() before the parent issues a SIGUSR1 and
things go wrong.

Fix this by migrating hugeshmctl to LTP synchronisation primitives
TST_CHECKPOINT_{WAIT,WAKE,WAIT2} and removing the code that was used for
signals based synchronisation.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 .../kernel/mem/hugetlb/hugeshmctl/hugeshmctl01.c   | 75 ++++------------------
 1 file changed, 13 insertions(+), 62 deletions(-)

diff --git a/testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl01.c b/testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl01.c
index ad7a80d70..1808995ed 100644
--- a/testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl01.c
+++ b/testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl01.c
@@ -56,9 +56,7 @@ static struct shmid_ds buf;
 static time_t save_time;
 static int stat_time;
 static void *set_shared;
-static pid_t pid_arr[N_ATTACH];
 
-static void sighandler(int sig);
 static void stat_setup(void);
 static void stat_cleanup(void);
 static void set_setup(void);
@@ -141,19 +139,10 @@ void *set_shmat(void)
  */
 static void stat_setup(void)
 {
-	int i, rval;
+	int i;
 	void *test;
 	pid_t pid;
-	sigset_t newmask, oldmask;
-	struct sigaction sa;
-
-	memset (&sa, '\0', sizeof(sa));
-	sa.sa_handler = sighandler;
-	sa.sa_flags = 0;
-	TEST(sigaction(SIGUSR1, &sa, NULL));
-	if (TEST_RETURN == -1)
-		tst_brk(TBROK | TRERRNO,
-				"SIGSEGV signal setup failed");
+
 	/*
 	 * The first time through, let the children attach the memory.
 	 * The second time through, attach the memory first and let
@@ -168,21 +157,6 @@ static void stat_setup(void)
 		set_shared = set_shmat();
 	}
 
-	/*
-	 * Block SIGUSR1 before children pause for a signal
-	 * Doing so to avoid the risk that the parent cleans up
-	 * children by calling stat_cleanup() before children call
-	 * call pause() so that children sleep forever(this is a
-	 * side effect of the arbitrary usleep time below).
-	 * In FIRST, children call shmat. If children sleep forever,
-	 * those attached shm can't be released so some other shm
-	 * tests will fail a lot.
-	 */
-	sigemptyset(&newmask);
-	sigaddset(&newmask, SIGUSR1);
-	if (sigprocmask(SIG_BLOCK, &newmask, &oldmask) < 0)
-		tst_brk(TBROK | TERRNO, "block SIGUSR1 error");
-
 	for (i = 0; i < N_ATTACH; i++) {
 		switch (pid = SAFE_FORK()) {
 		case 0:
@@ -191,39 +165,24 @@ static void stat_setup(void)
 			/* do an assignement for fun */
 			*(int *)test = i;
 
-			/*
-			 * sigsuspend until we get a signal from stat_cleanup()
-			 * use sigsuspend instead of pause to avoid children
-			 * infinite sleep without getting SIGUSR1 from parent
-			 */
-			rval = sigsuspend(&oldmask);
-			if (rval != -1)
-				tst_brk(TBROK | TERRNO, "sigsuspend");
-
-			/*
-			 * don't have to block SIGUSR1 any more,
-			 * recover the mask
-			 */
-			if (sigprocmask(SIG_SETMASK, &oldmask, NULL) < 0)
-				tst_brk(TBROK | TERRNO,
-					 "child sigprocmask");
+			/* If last child then wake the parent */
+			if (i == N_ATTACH - 1)
+				TST_CHECKPOINT_WAKE(0);
+
+			TST_CHECKPOINT_WAIT(1);
 
 			/* now we're back - detach the memory and exit */
 			if (shmdt(test) == -1)
 				tst_brk(TBROK | TERRNO,
 					 "shmdt in stat_setup()");
+
+			/* Child process quits here */
 			exit(0);
-		default:
-			/* save the child's pid for cleanup later */
-			pid_arr[i] = pid;
 		}
 	}
 
-	/* parent doesn't have to block SIGUSR1, recover the mask */
-	if (sigprocmask(SIG_SETMASK, &oldmask, NULL) < 0)
-		tst_brk(TBROK, "parent sigprocmask");
-
-	usleep(250000);
+	/* Wait for all the children to attach to shared memory */
+	TST_CHECKPOINT_WAIT(0);
 }
 
 /*
@@ -283,11 +242,8 @@ fail:
  */
 static void stat_cleanup(void)
 {
-	int i;
-
 	/* wake up the childern so they can detach the memory and exit */
-	for (i = 0; i < N_ATTACH; i++)
-		SAFE_KILL(pid_arr[i], SIGUSR1);
+	TST_CHECKPOINT_WAKE2(1, N_ATTACH);
 
 	/* remove the parent's shared memory the second time through */
 	if (stat_time == SECOND)
@@ -296,12 +252,6 @@ static void stat_cleanup(void)
 	stat_time++;
 }
 
-static void sighandler(int sig)
-{
-	if (sig != SIGUSR1)
-		tst_res(TFAIL, "received unexpected signal %d", sig);
-}
-
 /*
  * set_setup() - set up for the IPC_SET command with shmctl()
  */
@@ -387,4 +337,5 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = test_hugeshmctl,
+	.needs_checkpoints = 1,
 };
-- 
2.14.2

[LTP] [PATCH v2 04/13] hugeshmctl01: Fix warning about signed/unsigned comparison

[Punit Agrawal]

The macro N_ATTACH is assumed to be a signed intenger which results in
the below warning when compiling -

warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  if (buf.shm_nattch != N_ATTACH + stat_time) {
                     ^~

Fix this by explicitly marking N_ATTACH to be an unsigned integer and
fixing the resulting fallout.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl01.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl01.c b/testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl01.c
index 1808995ed..3e7f12691 100644
--- a/testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl01.c
+++ b/testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl01.c
@@ -47,7 +47,7 @@
 
 #define FIRST		0
 #define SECOND		1
-#define N_ATTACH	4
+#define N_ATTACH	4U
 #define NEWMODE		0066
 
 static size_t shm_size;
@@ -139,7 +139,7 @@ void *set_shmat(void)
  */
 static void stat_setup(void)
 {
-	int i;
+	unsigned int i;
 	void *test;
 	pid_t pid;
 
-- 
2.14.2

[LTP] [PATCH v2 05/13] hugeshmctl02: Fix allocation size for odd number of hugepages

[Punit Agrawal]

From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

The test tries to allocate 2 * (half the number of available hugepages).
If we have odd number hugepages, the calculated allocation size is not
page-aligned and allocation fails.

e.g, (use -s option to trigger it manually)

$ hugeshmctl02 -s 7 -i 5
hugeshmctl02    0  TINFO  :  set nr_hugepages to 7
hugeshmctl02    1  TBROK  :  hugeshmctl02.c:153: shmget #2: errno=ENOMEM(12): Cannot allocate memory
hugeshmctl02    2  TBROK  :  hugeshmctl02.c:153: Remaining cases broken
hugeshmctl02    0  TINFO  :  set nr_hugepages to 0

Align the size to hugepage_size.

Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl02.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl02.c b/testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl02.c
index 6079d9937..82964a327 100644
--- a/testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl02.c
+++ b/testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl02.c
@@ -101,7 +101,7 @@ static void setup(void)
 	set_sys_tune("nr_hugepages", hugepages, 1);
 	hpage_size = SAFE_READ_MEMINFO("Hugepagesize:") * 1024;
 
-	shm_size = hpage_size * hugepages / 2;
+	shm_size = hpage_size * (hugepages / 2);
 	update_shm_size(&shm_size);
 	shmkey = getipckey();
 
-- 
2.14.2

[LTP] [PATCH v2 06/13] hugeshmget02: add missing SHM_HUGETLB flag on segment creation

[Punit Agrawal]

From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

On test setup, hugeshmget02 detects the number of huge pages available
and uses that number to size a shared memory segment that is allocated
to carry out subsequent syscalls checks.

The setup() function detects the number of huge pages available so that
it can size the share memory segment to allocate with a reasonable size
value, but fails to pass the SHM_HUGETLB on shared memory segment
creation which defeats the whole purpose of detecting the available
number of huge pages before creating the segment.

This omission can result in test failures, eg:

hugeshmget02    0  TINFO  :  set nr_hugepages to 128
hugeshmget02    0  TINFO  :  Using 21 hugepages
hugeshmget02    1  TBROK  :  hugeshmget02.c:155: shmget #setup:
errno=ENOMEM(12): Cannot allocate memory
hugeshmget02    2  TBROK  :  hugeshmget02.c:155: Remaining cases broken
hugeshmget02    0  TINFO  :  set nr_hugepages to 0

This patch adds the missing SHM_HUGETLB flag to the shmget call in the
setup() function.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 testcases/kernel/mem/hugetlb/hugeshmget/hugeshmget02.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/mem/hugetlb/hugeshmget/hugeshmget02.c b/testcases/kernel/mem/hugetlb/hugeshmget/hugeshmget02.c
index 7db8b4ede..0abd1aaf8 100644
--- a/testcases/kernel/mem/hugetlb/hugeshmget/hugeshmget02.c
+++ b/testcases/kernel/mem/hugetlb/hugeshmget/hugeshmget02.c
@@ -99,7 +99,8 @@ void setup(void)
 
 	shmkey = getipckey();
 	shmkey2 = shmkey + 1;
-	shm_id_1 = shmget(shmkey, shm_size, IPC_CREAT | IPC_EXCL | SHM_RW);
+	shm_id_1 = shmget(shmkey, shm_size,
+			  SHM_HUGETLB | IPC_CREAT | IPC_EXCL | SHM_RW);
 	if (shm_id_1 == -1)
 		tst_brk(TBROK | TERRNO, "shmget #setup");
 }
-- 
2.14.2

[LTP] [PATCH v2 07/13] sigwaitinfo01: fix race between sending and dequeueing RT signals

[Punit Agrawal]

From: Will Deacon <will.deacon@arm.com>

Although RT signals are dequeued in order, there is a race where only
the higher signal may have been sent and will therefore be dequeued
first, leading to a false failure.

This patch waits on the signal sending tasks so that we can be sure both
of the signals have been sent before dequeuing them.

Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 testcases/kernel/syscalls/sigwaitinfo/sigwaitinfo01.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/testcases/kernel/syscalls/sigwaitinfo/sigwaitinfo01.c b/testcases/kernel/syscalls/sigwaitinfo/sigwaitinfo01.c
index 35acd67c4..16b5096b8 100644
--- a/testcases/kernel/syscalls/sigwaitinfo/sigwaitinfo01.c
+++ b/testcases/kernel/syscalls/sigwaitinfo/sigwaitinfo01.c
@@ -249,6 +249,7 @@ void test_masked_matching_rt(swi_func sigwaitinfo, int signo)
 	sigset_t sigs, oldmask;
 	siginfo_t si;
 	pid_t child[2];
+	int status;
 
 	signo = SIGRTMIN + 1;
 
@@ -268,6 +269,10 @@ void test_masked_matching_rt(swi_func sigwaitinfo, int signo)
 	child[0] = create_sig_proc(0, signo, 1);
 	child[1] = create_sig_proc(0, signo + 1, 1);
 
+	/* Ensure that the signals have been sent */
+	waitpid(child[0], &status, 0);
+	waitpid(child[1], &status, 0);
+
 	TEST(sigwaitinfo(&sigs, &si, NULL));
 	REPORT_SUCCESS_COND(signo, 0, si.si_pid == child[0]
 			    && si.si_code == SI_USER
-- 
2.14.2

[LTP] [PATCH v2 08/13] sigwaitinfo01: catch SEGV and report success for bad_address2 testcase

[Punit Agrawal]

The bad_address2 testcase passes (void *)1 as a sigset pointer to the
sigwaitinfo syscall. Unsurprisingly, this segfaults in libc rather than
returning -1 (errno = EFAULT) as LTP expects.

Instead, fork a child process which registers a SIGSEGV handler to catch
any SEGV's generated by the sigwaitinfo syscall. The signal handler
exits the child process with a return code which is checked by the
parent to evaluate test outcome.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 .../kernel/syscalls/sigwaitinfo/sigwaitinfo01.c    | 29 ++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/sigwaitinfo/sigwaitinfo01.c b/testcases/kernel/syscalls/sigwaitinfo/sigwaitinfo01.c
index 16b5096b8..8f517e918 100644
--- a/testcases/kernel/syscalls/sigwaitinfo/sigwaitinfo01.c
+++ b/testcases/kernel/syscalls/sigwaitinfo/sigwaitinfo01.c
@@ -361,10 +361,35 @@ void test_bad_address(swi_func sigwaitinfo, int signo)
 	kill(child, SIGTERM);
 }
 
+static void segv_handler(int signo)
+{
+	/* SIGSEGV is expected, exit with 0xff */
+	_exit(0xff);
+}
+
 void test_bad_address2(swi_func sigwaitinfo, int signo)
 {
-	TEST(sigwaitinfo((void *)1, NULL, NULL));
-	REPORT_SUCCESS(-1, EFAULT);
+	pid_t pid;
+	int status;
+
+	switch (pid = FORK_OR_VFORK()) {
+	case -1:
+		tst_brkm(TBROK | TERRNO, NULL, "fork() failed");
+	case 0:
+		signal(SIGSEGV, segv_handler);
+		TEST(sigwaitinfo((void *)1, NULL, NULL));
+
+		_exit(0);
+		break;
+	default:
+		break;
+	}
+
+	SUCCEED_OR_DIE(waitpid, "waitpid failed", pid, &status, 0);
+	if (WIFEXITED(status) && WEXITSTATUS(status) == 0xff)
+		tst_resm(TPASS, "Test passed");
+	else
+		tst_resm(TFAIL, "Unrecognised child exit code");
 }
 
 void test_bad_address3(swi_func sigwaitinfo, int signo)
-- 
2.14.2

[LTP] [PATCH v2 09/13] syscalls/mount03: Copy setuid_test to execute instead of 'TEST FILE'

[Punit Agrawal]

From: Will Deacon <will.deacon@arm.com>

When testing the MS_NOSUID mount flag, mount03 forgets to copy
setuid_test into the new filesystem. Instead it writes 'TEST FILE' into
a new file and attempts to execute it with S_ISUID. This fails, but not
for the reasons ltp expects.

Signed-off-by: Will Deacon <will.deacon@arm.com>
[Added commit message, changed to use tst_resource_copy()]
Signed-off-by: James Morse <james.morse@arm.com>
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 testcases/kernel/syscalls/mount/mount03.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/mount/mount03.c b/testcases/kernel/syscalls/mount/mount03.c
index bf78c797f..abdef80fd 100644
--- a/testcases/kernel/syscalls/mount/mount03.c
+++ b/testcases/kernel/syscalls/mount/mount03.c
@@ -261,8 +261,10 @@ int test_rwflag(int i, int cnt)
 	case 5:
 		/* Validate MS_NOSUID flag of mount call */
 
+		tst_resource_copy(__FILE__, __LINE__, cleanup, "setuid_test",
+				  path_name);
+
 		snprintf(file, PATH_MAX, "%ssetuid_test", path_name);
-		SAFE_FILE_PRINTF(cleanup, file, "TEST FILE");
 
 		SAFE_STAT(cleanup, file, &file_stat);
 
-- 
2.14.2

[LTP] [PATCH v2 10/13] getdtablesize01: Handle ENFILE errno

[Punit Agrawal]

From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

getdtablesize01 testcase attempts to open RLIMIT_NOFILE-1 (Maximum open
files for a process) file descriptors. However if we hit an
ENFILE (system wide limit of maximum number of open files) we should
break the test, rather than failing. This is more relevant for runs on
VMs where the rootfs could be a 9p fs or any other emulated fs.

Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 .../kernel/syscalls/getdtablesize/getdtablesize01.c     | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/testcases/kernel/syscalls/getdtablesize/getdtablesize01.c b/testcases/kernel/syscalls/getdtablesize/getdtablesize01.c
index 7ee231adf..eecf44b18 100644
--- a/testcases/kernel/syscalls/getdtablesize/getdtablesize01.c
+++ b/testcases/kernel/syscalls/getdtablesize/getdtablesize01.c
@@ -54,7 +54,7 @@ int TST_TOTAL = 1;
 
 int main(void)
 {
-	int table_size, loop, fd, count = 0;
+	int table_size, loop, fd = 0, count = 0;
 	int max_val_opfiles;
 	struct rlimit rlp;
 
@@ -82,24 +82,27 @@ int main(void)
 		 "Checking Max num of files that can be opened by a process.Should be: RLIMIT_NOFILE - 1");
 	for (loop = 1; loop <= max_val_opfiles; loop++) {
 		fd = open("/etc/hosts", O_RDONLY);
+
+		if (fd == -1)
+			break;
+		count = fd;
 #ifdef DEBUG
 		printf("Opened file num %d\n", fd);
 #endif
-		if (fd == -1)
-			break;
-		else
-			count = fd;
 	}
 
 //Now the max files opened should be RLIMIT_NOFILE - 1 , why ? read getdtablesize man page
 
-	if (count > 0)
-		close(count);
 	if (count == (max_val_opfiles - 1))
 		tst_resm(TPASS, "%d = %d", count, (max_val_opfiles - 1));
+	else if (fd < 0 && errno == ENFILE)
+		tst_brkm(TBROK, cleanup, "Reached maximum number of open files for the system");
 	else
 		tst_resm(TFAIL, "%d != %d", count, (max_val_opfiles - 1));
 
+	if (count > 0)
+		close(count);
+
 	cleanup();
 	tst_exit();
 }
-- 
2.14.2

[LTP] [PATCH v2 11/13] perf_event_open: Handle absence of PMU gracefully

[Punit Agrawal]

From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

The perf_event_open0{1,2} syscall tests fail for an absence of a
PMU (which returns ENODEV). Handle the return code gracefully as we
check for ENOENT.

Without the patch:

$ perf_event_open01
perf_event_open01    1  TFAIL  :  perf_event_open01.c:162: perf_event_open \
		failed unexpectedly: TEST_ERRNO=ENODEV(19): No such device

With the patch:

$ perf_event_open01
perf_event_open01    1  TCONF  :  perf_event_open01.c:159: perf_event_open for PERF_COUNT_HW_INSTRUCTIONS : No such device
perf_event_open01    2  TCONF  :  perf_event_open01.c:159: perf_event_open for PERF_COUNT_HW_CACHE_REFERENCES : No such device
perf_event_open01    3  TCONF  :  perf_event_open01.c:159: perf_event_open for PERF_COUNT_HW_CACHE_MISSES : No such device
perf_event_open01    4  TCONF  :  perf_event_open01.c:159: perf_event_open for PERF_COUNT_HW_BRANCH_INSTRUCTIONS : No such file or directory
perf_event_open01    5  TCONF  :  perf_event_open01.c:159: perf_event_open for PERF_COUNT_HW_BRANCH_MISSES : No such device
perf_event_open01    0  TINFO  :  read event counter succeeded, value: 833765520
perf_event_open01    6  TPASS  :  test PERF_TYPE_HARDWARE: PERF_COUNT_SW_CPU_CLOCK succeeded
perf_event_open01    0  TINFO  :  read event counter succeeded, value: 833967380
perf_event_open01    7  TPASS  :  test PERF_TYPE_HARDWARE: PERF_COUNT_SW_TASK_CLOCK succeeded

Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 testcases/kernel/syscalls/perf_event_open/perf_event_open01.c | 3 ++-
 testcases/kernel/syscalls/perf_event_open/perf_event_open02.c | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/syscalls/perf_event_open/perf_event_open01.c b/testcases/kernel/syscalls/perf_event_open/perf_event_open01.c
index 5c814b60c..5568035d9 100644
--- a/testcases/kernel/syscalls/perf_event_open/perf_event_open01.c
+++ b/testcases/kernel/syscalls/perf_event_open/perf_event_open01.c
@@ -149,7 +149,8 @@ static void verify(struct test_case_t *tc)
 
 	TEST(perf_event_open(&pe, 0, -1, -1, 0));
 	if (TEST_RETURN == -1) {
-		if (TEST_ERRNO == ENOENT || TEST_ERRNO == EOPNOTSUPP) {
+		if (TEST_ERRNO == ENOENT || TEST_ERRNO == EOPNOTSUPP ||
+		    TEST_ERRNO == ENODEV) {
 			tst_resm(TCONF | TTERRNO,
 			         "perf_event_open for %s not supported",
 			         tc->config_name);
diff --git a/testcases/kernel/syscalls/perf_event_open/perf_event_open02.c b/testcases/kernel/syscalls/perf_event_open/perf_event_open02.c
index 7d54cbd52..13a17948a 100644
--- a/testcases/kernel/syscalls/perf_event_open/perf_event_open02.c
+++ b/testcases/kernel/syscalls/perf_event_open/perf_event_open02.c
@@ -160,8 +160,8 @@ static int count_hardware_counters(void)
 	for (i = 0; i < MAX_CTRS; i++) {
 		fdarry[i] = perf_event_open(&hw_event, 0, -1, -1, 0);
 		if (fdarry[i] == -1) {
-			if (errno == ENOENT) {
-				tst_brkm(TCONF, cleanup,
+			if (errno == ENOENT || errno == ENODEV) {
+				tst_brkm(TCONF | TERRNO, cleanup,
 				         "PERF_COUNT_HW_INSTRUCTIONS not supported");
 			}
 			tst_brkm(TBROK | TERRNO, cleanup,
-- 
2.14.2

[LTP] [PATCH v2 12/13] hotplug/cpu_hotplug: Repopulate cgroup:cpusets after testing hotplug

[Punit Agrawal]

From: James Morse <james.morse@arm.com>

When LTP is started over ssh, systemd will use cgroups to group all the
tasks started by this session. LTPs cpuhotplug04 will remove all but one
CPU as part of its test.

In the kernel cpuset_hotplug_workfn() is documented thus: "Non-root
cpusets are only affected by offlining". This means cpuhotplug04 will
cause all future tasks to only have one available CPU. cpuhotplug04
effectively looses those CPUs for all users until the next reboot.

This problem has been reported, and was fixed for the unified hierarchy
by be4c9dd7aee5 ("cpuset: enable onlined cpu/node in effective
masks"). Since v3.17, this behaviour could be selected by passing '-o
__DEVEL__sane_behavior' to the cgroup mount.  Linux v4.4 grouped all the
ABI breakage together into a new filesystem: cgroup2, removing the
__DEVEL__sane_behavior flag.

With v226 Systemd can be forced to use cgroup2 by passing
'systemd.unified_cgroup_hierarchy=1' on the kernel command line.

Ubuntu 15.04 ships with systemd v219.

LTP tests that online CPUs must test for the legacy cgroup hierarchy and
update the cpusets for all users/sessions. Add this code the
cpu_online() helper.

Signed-off-by: James Morse <james.morse@arm.com>
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 .../cpu_hotplug/include/cpuhotplug_hotplug.sh      | 36 ++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/testcases/kernel/hotplug/cpu_hotplug/include/cpuhotplug_hotplug.sh b/testcases/kernel/hotplug/cpu_hotplug/include/cpuhotplug_hotplug.sh
index 9f856e554..35ec7c86b 100644
--- a/testcases/kernel/hotplug/cpu_hotplug/include/cpuhotplug_hotplug.sh
+++ b/testcases/kernel/hotplug/cpu_hotplug/include/cpuhotplug_hotplug.sh
@@ -58,6 +58,39 @@ set_affinity()
     return $?
 }
 
+# repopulate_cpusets()
+#
+# If the legacy cgroup hierarchy is in use, and cpuset is mounted, copy
+# the permitted cpus from the root group, which always has all available
+# cpus and memory, into the other hierarchy nodes. This prevents hotplug
+# tests from losing CPUs, and affecting the results of later tests.
+#
+CPUSET="/sys/fs/cgroup/cpuset"
+repopulate_cpusets()
+{
+	# Only if we have cpusets
+	if [ ! -d ${CPUSET} ] ; then
+		return;
+	fi
+
+	# And then only legacy cpusets
+	if grep --quiet "/sys/fs/cgroup/cpuset[ ]*cgroup2" /proc/mounts ; then
+		return;
+	fi
+
+	# Copy root cpus into system and user slices
+	cat ${CPUSET}/cpuset.cpus > ${CPUSET}/system.slice/cpuset.cpus
+	cat ${CPUSET}/cpuset.cpus > ${CPUSET}/user.slice/cpuset.cpus
+
+	# Fix each session of each user
+	for TUSER in ${CPUSET}/user.slice/user-*.slice ; do
+		cat ${CPUSET}/cpuset.cpus > ${TUSER}/cpuset.cpus
+		for SESSION in ${TUSER}/session-*.scope; do
+			cat ${CPUSET}/cpuset.cpus > ${SESSION}/cpuset.cpus
+		done
+	done
+}
+
 # online_cpu(CPU)
 #
 #  Onlines the given CPU.  Returns a true value if it was able
@@ -77,6 +110,9 @@ online_cpu()
 
     $TIME echo 1 > /sys/devices/system/cpu/cpu${CPU}/online
     RC=$?
+
+    repopulate_cpusets
+
     report_timing "Online cpu ${CPU}"
     return $RC
 }
-- 
2.14.2

[LTP] [PATCH v2 13/13] hotplug/cpu_hotplug: Remove bashism disown from kill_pid()

[Punit Agrawal]

From: James Morse <james.morse@arm.com>

disown is a bashism not present in dash. kill_pid() already sends the
process SIGKILL.

Remove the "disown" to allow running in shells that don't support
disown.

Signed-off-by: James Morse <james.morse@arm.com>
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 testcases/kernel/hotplug/cpu_hotplug/include/cpuhotplug_testsuite.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/testcases/kernel/hotplug/cpu_hotplug/include/cpuhotplug_testsuite.sh b/testcases/kernel/hotplug/cpu_hotplug/include/cpuhotplug_testsuite.sh
index 2d0166cc6..7dd0ebaae 100644
--- a/testcases/kernel/hotplug/cpu_hotplug/include/cpuhotplug_testsuite.sh
+++ b/testcases/kernel/hotplug/cpu_hotplug/include/cpuhotplug_testsuite.sh
@@ -74,7 +74,6 @@ pid_is_valid()
 kill_pid()
 {
     PID=$1
-    disown $PID
     kill -9 $PID > /dev/null 2>&1
 }
 
-- 
2.14.2

[LTP] [PATCH v2 00/13] Collection of fixes

[Punit Agrawal]

Hi,

Punit Agrawal <punit.agrawal@arm.com> writes:

> Hi,
>
> This is the second posting of that collects fixes for issues
> encountered when running ltp on internal test platforms. Previous
> version can be found at [0]. This update addresses all the feedback
> received on the previous version.
>
> Changes:
>
> v2:
> * removed check_hugepage() prototype from hugetlb.h (patch 1)
> * hugeshmctl01 - migrate to using checkpoint synchronisation primitives
> * sigwaitinfo - use child process to execute test instead of using
>   longjmp based solution
> * Dropped patches that are no longer applicable or already merged
>
> If there aren't anymore comments, please consider them for merging.

I'd appreciate any feedback on this patch set. This update addresses all
the comments received on the previous version.

If the patches look good, please consider merging them.

>
> Thanks,
> Punit
>
> [0] http://lists.linux.it/pipermail/ltp/2017-October/006225.html
>
> James Morse (2):
>   hotplug/cpu_hotplug: Repopulate cgroup:cpusets after testing hotplug
>   hotplug/cpu_hotplug: Remove bashism disown from kill_pid()
>
> Lorenzo Pieralisi (1):
>   hugeshmget02: add missing SHM_HUGETLB flag on segment creation
>
> Punit Agrawal (4):
>   Move check_hugepage() helper to mem/lib
>   hugeshmctl01: Convert to LTP synchronisation primitives
>   hugeshmctl01: Fix warning about signed/unsigned comparison
>   sigwaitinfo01: catch SEGV and report success for bad_address2 testcase
>
> Suzuki K. Poulose (3):
>   hugeshmctl02: Fix allocation size for odd number of hugepages
>   getdtablesize01: Handle ENFILE errno
>   perf_event_open: Handle absence of PMU gracefully
>
> Will Deacon (3):
>   thp: ensure THP/hugetlbfs is available
>   sigwaitinfo01: fix race between sending and dequeueing RT signals
>   syscalls/mount03: Copy setuid_test to execute instead of 'TEST FILE'
>
>  .../cpu_hotplug/include/cpuhotplug_hotplug.sh      | 36 ++++++++++
>  .../cpu_hotplug/include/cpuhotplug_testsuite.sh    |  1 -
>  .../kernel/mem/hugetlb/hugeshmctl/hugeshmctl01.c   | 77 ++++------------------
>  .../kernel/mem/hugetlb/hugeshmctl/hugeshmctl02.c   |  2 +-
>  .../kernel/mem/hugetlb/hugeshmget/hugeshmget02.c   |  3 +-
>  testcases/kernel/mem/hugetlb/lib/hugetlb.c         |  6 --
>  testcases/kernel/mem/hugetlb/lib/hugetlb.h         |  3 -
>  testcases/kernel/mem/include/mem.h                 |  2 +
>  testcases/kernel/mem/lib/mem.c                     |  6 ++
>  testcases/kernel/mem/thp/thp01.c                   |  3 +
>  testcases/kernel/mem/thp/thp02.c                   |  2 +
>  testcases/kernel/mem/thp/thp03.c                   |  2 +
>  .../syscalls/getdtablesize/getdtablesize01.c       | 17 +++--
>  testcases/kernel/syscalls/mount/mount03.c          |  4 +-
>  .../syscalls/perf_event_open/perf_event_open01.c   |  3 +-
>  .../syscalls/perf_event_open/perf_event_open02.c   |  4 +-
>  .../kernel/syscalls/sigwaitinfo/sigwaitinfo01.c    | 34 +++++++++-
>  17 files changed, 117 insertions(+), 88 deletions(-)

[LTP] [PATCH v2 02/13] thp: ensure THP/hugetlbfs is available

[Cyril Hrubis]

Hi!
> diff --git a/testcases/kernel/mem/thp/thp01.c b/testcases/kernel/mem/thp/thp01.c
> index 2f2bb2bdb..939c01b98 100644
> --- a/testcases/kernel/mem/thp/thp01.c
> +++ b/testcases/kernel/mem/thp/thp01.c
> @@ -116,6 +116,9 @@ static void setup(void)
>  	int i;
>  	long arg_len, arg_count;
>  
> +	if (access(PATH_THP, F_OK) == -1)
> +		tst_brk(TCONF, "THP not enabled in kernel?");
> +
>  	bst = SAFE_MMAP(NULL, sizeof(*bst),
>  			   PROT_READ | PROT_WRITE,
>  			   MAP_SHARED | MAP_ANONYMOUS, -1, 0);

I do not get what is the problem with this test on a machine without
hugepages or transparent hugepages. The test just tries to execute true
with excessively large array of command line arguments, if that fails
without hugepages it's a kernel bug.

> diff --git a/testcases/kernel/mem/thp/thp02.c b/testcases/kernel/mem/thp/thp02.c
> index acc70e215..6502210e5 100644
> --- a/testcases/kernel/mem/thp/thp02.c
> +++ b/testcases/kernel/mem/thp/thp02.c
> @@ -90,6 +90,8 @@ static void setup(void)
>  	if (access(PATH_THP, F_OK) == -1)
>  		tst_brk(TCONF, "THP not enabled in kernel?");
>  
> +	check_hugepage();
> +
>  	ps = sysconf(_SC_PAGESIZE);
>  	hps = SAFE_READ_MEMINFO("Hugepagesize:") * 1024;
>  	size = hps * 4;
> diff --git a/testcases/kernel/mem/thp/thp03.c b/testcases/kernel/mem/thp/thp03.c
> index 19db44944..c6062505f 100644
> --- a/testcases/kernel/mem/thp/thp03.c
> +++ b/testcases/kernel/mem/thp/thp03.c
> @@ -82,6 +82,8 @@ static void setup(void)
>  	if (access(PATH_THP, F_OK) == -1)
>  		tst_brk(TCONF, "THP not enabled in kernel?");
>  
> +	check_hugepage();
> +
>  	hugepage_size = SAFE_READ_MEMINFO("Hugepagesize:") * KB;
>  	unaligned_size = hugepage_size * 4 - 1;
>  	page_size = SAFE_SYSCONF(_SC_PAGESIZE);

Does this two fail on an attempt to find "Hugepagesize:" in meminfo
file? Otherwise I do not see how they can fail.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 03/13] hugeshmctl01: Convert to LTP synchronisation primitives

[Cyril Hrubis]

Hi!
>  static void stat_setup(void);
>  static void stat_cleanup(void);
>  static void set_setup(void);
> @@ -141,19 +139,10 @@ void *set_shmat(void)
>   */
>  static void stat_setup(void)
>  {
> -	int i, rval;
> +	int i;
>  	void *test;
>  	pid_t pid;
> -	sigset_t newmask, oldmask;
> -	struct sigaction sa;
> -
> -	memset (&sa, '\0', sizeof(sa));
> -	sa.sa_handler = sighandler;
> -	sa.sa_flags = 0;
> -	TEST(sigaction(SIGUSR1, &sa, NULL));
> -	if (TEST_RETURN == -1)
> -		tst_brk(TBROK | TRERRNO,
> -				"SIGSEGV signal setup failed");
> +
>  	/*
>  	 * The first time through, let the children attach the memory.
>  	 * The second time through, attach the memory first and let
> @@ -168,21 +157,6 @@ static void stat_setup(void)
>  		set_shared = set_shmat();
>  	}
>  
> -	/*
> -	 * Block SIGUSR1 before children pause for a signal
> -	 * Doing so to avoid the risk that the parent cleans up
> -	 * children by calling stat_cleanup() before children call
> -	 * call pause() so that children sleep forever(this is a
> -	 * side effect of the arbitrary usleep time below).
> -	 * In FIRST, children call shmat. If children sleep forever,
> -	 * those attached shm can't be released so some other shm
> -	 * tests will fail a lot.
> -	 */
> -	sigemptyset(&newmask);
> -	sigaddset(&newmask, SIGUSR1);
> -	if (sigprocmask(SIG_BLOCK, &newmask, &oldmask) < 0)
> -		tst_brk(TBROK | TERRNO, "block SIGUSR1 error");
> -
>  	for (i = 0; i < N_ATTACH; i++) {
>  		switch (pid = SAFE_FORK()) {
>  		case 0:
> @@ -191,39 +165,24 @@ static void stat_setup(void)
>  			/* do an assignement for fun */
>  			*(int *)test = i;
>  
> -			/*
> -			 * sigsuspend until we get a signal from stat_cleanup()
> -			 * use sigsuspend instead of pause to avoid children
> -			 * infinite sleep without getting SIGUSR1 from parent
> -			 */
> -			rval = sigsuspend(&oldmask);
> -			if (rval != -1)
> -				tst_brk(TBROK | TERRNO, "sigsuspend");
> -
> -			/*
> -			 * don't have to block SIGUSR1 any more,
> -			 * recover the mask
> -			 */
> -			if (sigprocmask(SIG_SETMASK, &oldmask, NULL) < 0)
> -				tst_brk(TBROK | TERRNO,
> -					 "child sigprocmask");
> +			/* If last child then wake the parent */
> +			if (i == N_ATTACH - 1)
> +				TST_CHECKPOINT_WAKE(0);

As far as I can tell the assumption that the children are executed in
any order does not hold at all. So the parent should wait for _each_ of
them separately.

I guess that we have to suspend the parent process with
TST_CHECKPOINT_WAIT() after each fork() and wait till the children wakes
up the parent before we attempt to fork next child.

> +			TST_CHECKPOINT_WAIT(1);
>  
>  			/* now we're back - detach the memory and exit */
>  			if (shmdt(test) == -1)
>  				tst_brk(TBROK | TERRNO,
>  					 "shmdt in stat_setup()");
> +
> +			/* Child process quits here */

Please avoid adding obvious comments like this one. The original code is
not a good example and should be cleaned up.

>  			exit(0);
> -		default:
> -			/* save the child's pid for cleanup later */
> -			pid_arr[i] = pid;
>  		}
>  	}
>  
> -	/* parent doesn't have to block SIGUSR1, recover the mask */
> -	if (sigprocmask(SIG_SETMASK, &oldmask, NULL) < 0)
> -		tst_brk(TBROK, "parent sigprocmask");
> -
> -	usleep(250000);
> +	/* Wait for all the children to attach to shared memory */
> +	TST_CHECKPOINT_WAIT(0);
>  }
>  
>  /*
> @@ -283,11 +242,8 @@ fail:
>   */
>  static void stat_cleanup(void)
>  {
> -	int i;
> -
>  	/* wake up the childern so they can detach the memory and exit */
> -	for (i = 0; i < N_ATTACH; i++)
> -		SAFE_KILL(pid_arr[i], SIGUSR1);
> +	TST_CHECKPOINT_WAKE2(1, N_ATTACH);

We should still SAFE_WAIT() each of the processses here otherwise we
cannot guarantee that the memory was detached and that the child really
exitted.

>  	/* remove the parent's shared memory the second time through */
>  	if (stat_time == SECOND)
> @@ -296,12 +252,6 @@ static void stat_cleanup(void)
>  	stat_time++;
>  }
>  
> -static void sighandler(int sig)
> -{
> -	if (sig != SIGUSR1)
> -		tst_res(TFAIL, "received unexpected signal %d", sig);
> -}
> -
>  /*
>   * set_setup() - set up for the IPC_SET command with shmctl()
>   */
> @@ -387,4 +337,5 @@ static struct tst_test test = {
>  	.setup = setup,
>  	.cleanup = cleanup,
>  	.test_all = test_hugeshmctl,
> +	.needs_checkpoints = 1,
>  };
> -- 
> 2.14.2
> 

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 02/13] thp: ensure THP/hugetlbfs is available

[Punit Agrawal]

Hi Cyril,

Thanks for taking a look.

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> diff --git a/testcases/kernel/mem/thp/thp01.c b/testcases/kernel/mem/thp/thp01.c
>> index 2f2bb2bdb..939c01b98 100644
>> --- a/testcases/kernel/mem/thp/thp01.c
>> +++ b/testcases/kernel/mem/thp/thp01.c
>> @@ -116,6 +116,9 @@ static void setup(void)
>>  	int i;
>>  	long arg_len, arg_count;
>>  
>> +	if (access(PATH_THP, F_OK) == -1)
>> +		tst_brk(TCONF, "THP not enabled in kernel?");
>> +
>>  	bst = SAFE_MMAP(NULL, sizeof(*bst),
>>  			   PROT_READ | PROT_WRITE,
>>  			   MAP_SHARED | MAP_ANONYMOUS, -1, 0);
>
> I do not get what is the problem with this test on a machine without
> hugepages or transparent hugepages. The test just tries to execute true
> with excessively large array of command line arguments, if that fails
> without hugepages it's a kernel bug.

The test passes even if THP is not enabled on the test kernel. This
gives the false impression that THP is working as expected.

The explicit check is making sure the functionality under test is is
enabled in the test kernel.

>
>> diff --git a/testcases/kernel/mem/thp/thp02.c b/testcases/kernel/mem/thp/thp02.c
>> index acc70e215..6502210e5 100644
>> --- a/testcases/kernel/mem/thp/thp02.c
>> +++ b/testcases/kernel/mem/thp/thp02.c
>> @@ -90,6 +90,8 @@ static void setup(void)
>>  	if (access(PATH_THP, F_OK) == -1)
>>  		tst_brk(TCONF, "THP not enabled in kernel?");
>>  
>> +	check_hugepage();
>> +
>>  	ps = sysconf(_SC_PAGESIZE);
>>  	hps = SAFE_READ_MEMINFO("Hugepagesize:") * 1024;
>>  	size = hps * 4;
>> diff --git a/testcases/kernel/mem/thp/thp03.c b/testcases/kernel/mem/thp/thp03.c
>> index 19db44944..c6062505f 100644
>> --- a/testcases/kernel/mem/thp/thp03.c
>> +++ b/testcases/kernel/mem/thp/thp03.c
>> @@ -82,6 +82,8 @@ static void setup(void)
>>  	if (access(PATH_THP, F_OK) == -1)
>>  		tst_brk(TCONF, "THP not enabled in kernel?");
>>  
>> +	check_hugepage();
>> +
>>  	hugepage_size = SAFE_READ_MEMINFO("Hugepagesize:") * KB;
>>  	unaligned_size = hugepage_size * 4 - 1;
>>  	page_size = SAFE_SYSCONF(_SC_PAGESIZE);
>
> Does this two fail on an attempt to find "Hugepagesize:" in meminfo
> file? Otherwise I do not see how they can fail.

Yes, "Hugepagesize:" goes missing in /proc/meminfo. When the kernel
doesn't have hugepage support enabled, the tests fail with message like
-

safe_file_ops.c:220: BROK: Expected 1 conversions got 0 at thp03.c:85

With the patch, the message helps identify the cause of the failure.

Thanks,
Punit

[LTP] [PATCH v2 03/13] hugeshmctl01: Convert to LTP synchronisation primitives

[Punit Agrawal]

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>>  static void stat_setup(void);
>>  static void stat_cleanup(void);
>>  static void set_setup(void);
>> @@ -141,19 +139,10 @@ void *set_shmat(void)
>>   */
>>  static void stat_setup(void)
>>  {
>> -	int i, rval;
>> +	int i;
>>  	void *test;
>>  	pid_t pid;
>> -	sigset_t newmask, oldmask;
>> -	struct sigaction sa;
>> -
>> -	memset (&sa, '\0', sizeof(sa));
>> -	sa.sa_handler = sighandler;
>> -	sa.sa_flags = 0;
>> -	TEST(sigaction(SIGUSR1, &sa, NULL));
>> -	if (TEST_RETURN == -1)
>> -		tst_brk(TBROK | TRERRNO,
>> -				"SIGSEGV signal setup failed");
>> +
>>  	/*
>>  	 * The first time through, let the children attach the memory.
>>  	 * The second time through, attach the memory first and let
>> @@ -168,21 +157,6 @@ static void stat_setup(void)
>>  		set_shared = set_shmat();
>>  	}
>>  
>> -	/*
>> -	 * Block SIGUSR1 before children pause for a signal
>> -	 * Doing so to avoid the risk that the parent cleans up
>> -	 * children by calling stat_cleanup() before children call
>> -	 * call pause() so that children sleep forever(this is a
>> -	 * side effect of the arbitrary usleep time below).
>> -	 * In FIRST, children call shmat. If children sleep forever,
>> -	 * those attached shm can't be released so some other shm
>> -	 * tests will fail a lot.
>> -	 */
>> -	sigemptyset(&newmask);
>> -	sigaddset(&newmask, SIGUSR1);
>> -	if (sigprocmask(SIG_BLOCK, &newmask, &oldmask) < 0)
>> -		tst_brk(TBROK | TERRNO, "block SIGUSR1 error");
>> -
>>  	for (i = 0; i < N_ATTACH; i++) {
>>  		switch (pid = SAFE_FORK()) {
>>  		case 0:
>> @@ -191,39 +165,24 @@ static void stat_setup(void)
>>  			/* do an assignement for fun */
>>  			*(int *)test = i;
>>  
>> -			/*
>> -			 * sigsuspend until we get a signal from stat_cleanup()
>> -			 * use sigsuspend instead of pause to avoid children
>> -			 * infinite sleep without getting SIGUSR1 from parent
>> -			 */
>> -			rval = sigsuspend(&oldmask);
>> -			if (rval != -1)
>> -				tst_brk(TBROK | TERRNO, "sigsuspend");
>> -
>> -			/*
>> -			 * don't have to block SIGUSR1 any more,
>> -			 * recover the mask
>> -			 */
>> -			if (sigprocmask(SIG_SETMASK, &oldmask, NULL) < 0)
>> -				tst_brk(TBROK | TERRNO,
>> -					 "child sigprocmask");
>> +			/* If last child then wake the parent */
>> +			if (i == N_ATTACH - 1)
>> +				TST_CHECKPOINT_WAKE(0);
>
> As far as I can tell the assumption that the children are executed in
> any order does not hold at all. So the parent should wait for _each_ of
> them separately.

Good catch. I see the problem with the patch in the scenario where the
N-1 th child executes TST_CHECKPOINT_WAKE(0) before the other children
have progressed past the write to "test".

I'll fix it as per your suggestion below.

>
> I guess that we have to suspend the parent process with
> TST_CHECKPOINT_WAIT() after each fork() and wait till the children wakes
> up the parent before we attempt to fork next child.
>

>> +			TST_CHECKPOINT_WAIT(1);
>>  
>>  			/* now we're back - detach the memory and exit */
>>  			if (shmdt(test) == -1)
>>  				tst_brk(TBROK | TERRNO,
>>  					 "shmdt in stat_setup()");
>> +
>> +			/* Child process quits here */
>
> Please avoid adding obvious comments like this one. The original code is
> not a good example and should be cleaned up.

Dropped :)

>
>>  			exit(0);
>> -		default:
>> -			/* save the child's pid for cleanup later */
>> -			pid_arr[i] = pid;
>>  		}
>>  	}
>>  
>> -	/* parent doesn't have to block SIGUSR1, recover the mask */
>> -	if (sigprocmask(SIG_SETMASK, &oldmask, NULL) < 0)
>> -		tst_brk(TBROK, "parent sigprocmask");
>> -
>> -	usleep(250000);
>> +	/* Wait for all the children to attach to shared memory */
>> +	TST_CHECKPOINT_WAIT(0);
>>  }
>>  
>>  /*
>> @@ -283,11 +242,8 @@ fail:
>>   */
>>  static void stat_cleanup(void)
>>  {
>> -	int i;
>> -
>>  	/* wake up the childern so they can detach the memory and exit */
>> -	for (i = 0; i < N_ATTACH; i++)
>> -		SAFE_KILL(pid_arr[i], SIGUSR1);
>> +	TST_CHECKPOINT_WAKE2(1, N_ATTACH);
>
> We should still SAFE_WAIT() each of the processses here otherwise we
> cannot guarantee that the memory was detached and that the child really
> exitted.

Makes sense. I've added a SAFE_WAITPID() after the
TST_CHECKPOINT_WAKE2().

I'll wait for you to go through the remaining patches before sending an
update.

Thanks for the review.

Punit

>
>>  	/* remove the parent's shared memory the second time through */
>>  	if (stat_time == SECOND)
>> @@ -296,12 +252,6 @@ static void stat_cleanup(void)
>>  	stat_time++;
>>  }
>>  
>> -static void sighandler(int sig)
>> -{
>> -	if (sig != SIGUSR1)
>> -		tst_res(TFAIL, "received unexpected signal %d", sig);
>> -}
>> -
>>  /*
>>   * set_setup() - set up for the IPC_SET command with shmctl()
>>   */
>> @@ -387,4 +337,5 @@ static struct tst_test test = {
>>  	.setup = setup,
>>  	.cleanup = cleanup,
>>  	.test_all = test_hugeshmctl,
>> +	.needs_checkpoints = 1,
>>  };
>> -- 
>> 2.14.2
>> 

[LTP] [PATCH v2 08/13] sigwaitinfo01: catch SEGV and report success for bad_address2 testcase

[Cyril Hrubis]

Hi!
>  void test_bad_address2(swi_func sigwaitinfo, int signo)
>  {
> -	TEST(sigwaitinfo((void *)1, NULL, NULL));
> -	REPORT_SUCCESS(-1, EFAULT);
> +	pid_t pid;
> +	int status;
> +
> +	switch (pid = FORK_OR_VFORK()) {
> +	case -1:
> +		tst_brkm(TBROK | TERRNO, NULL, "fork() failed");
> +	case 0:
> +		signal(SIGSEGV, segv_handler);
> +		TEST(sigwaitinfo((void *)1, NULL, NULL));
> +
> +		_exit(0);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	SUCCEED_OR_DIE(waitpid, "waitpid failed", pid, &status, 0);
> +	if (WIFEXITED(status) && WEXITSTATUS(status) == 0xff)
> +		tst_resm(TPASS, "Test passed");

Why don't we just check for WIFSIGNALED() and WTERMSIG() instead?

We do not have to install any handler that way...

> +	else
> +		tst_resm(TFAIL, "Unrecognised child exit code");
>  }
>  
>  void test_bad_address3(swi_func sigwaitinfo, int signo)
> -- 
> 2.14.2
> 

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 02/13] thp: ensure THP/hugetlbfs is available

[Cyril Hrubis]

Hi!
> > I do not get what is the problem with this test on a machine without
> > hugepages or transparent hugepages. The test just tries to execute true
> > with excessively large array of command line arguments, if that fails
> > without hugepages it's a kernel bug.
> 
> The test passes even if THP is not enabled on the test kernel. This
> gives the false impression that THP is working as expected.
> 
> The explicit check is making sure the functionality under test is is
> enabled in the test kernel.

I see where that came from, but I do not think that the test name is a
reason to disable this stress test. However I'm not sure if there is a
better name for the test. I guess that we may possibly move it to the
cve directory.

> >> diff --git a/testcases/kernel/mem/thp/thp02.c b/testcases/kernel/mem/thp/thp02.c
> >> index acc70e215..6502210e5 100644
> >> --- a/testcases/kernel/mem/thp/thp02.c
> >> +++ b/testcases/kernel/mem/thp/thp02.c
> >> @@ -90,6 +90,8 @@ static void setup(void)
> >>  	if (access(PATH_THP, F_OK) == -1)
> >>  		tst_brk(TCONF, "THP not enabled in kernel?");
> >>  
> >> +	check_hugepage();
> >> +
> >>  	ps = sysconf(_SC_PAGESIZE);
> >>  	hps = SAFE_READ_MEMINFO("Hugepagesize:") * 1024;
> >>  	size = hps * 4;
> >> diff --git a/testcases/kernel/mem/thp/thp03.c b/testcases/kernel/mem/thp/thp03.c
> >> index 19db44944..c6062505f 100644
> >> --- a/testcases/kernel/mem/thp/thp03.c
> >> +++ b/testcases/kernel/mem/thp/thp03.c
> >> @@ -82,6 +82,8 @@ static void setup(void)
> >>  	if (access(PATH_THP, F_OK) == -1)
> >>  		tst_brk(TCONF, "THP not enabled in kernel?");
> >>  
> >> +	check_hugepage();
> >> +
> >>  	hugepage_size = SAFE_READ_MEMINFO("Hugepagesize:") * KB;
> >>  	unaligned_size = hugepage_size * 4 - 1;
> >>  	page_size = SAFE_SYSCONF(_SC_PAGESIZE);
> >
> > Does this two fail on an attempt to find "Hugepagesize:" in meminfo
> > file? Otherwise I do not see how they can fail.
> 
> Yes, "Hugepagesize:" goes missing in /proc/meminfo. When the kernel
> doesn't have hugepage support enabled, the tests fail with message like
> -
> 
> safe_file_ops.c:220: BROK: Expected 1 conversions got 0 at thp03.c:85
> 
> With the patch, the message helps identify the cause of the failure.

That's fine. Can you plese include this description in the commit
message as well?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 09/13] syscalls/mount03: Copy setuid_test to execute instead of 'TEST FILE'

[Cyril Hrubis]

Hi!
> When testing the MS_NOSUID mount flag, mount03 forgets to copy
> setuid_test into the new filesystem. Instead it writes 'TEST FILE' into
> a new file and attempts to execute it with S_ISUID. This fails, but not
> for the reasons ltp expects.

What is the exact error here? I guess that the file does not have
execute bit set and that check kicks in first?

Anyway we really should check the errno from the exec() is EPERM which
should have caught this problem.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 08/13] sigwaitinfo01: catch SEGV and report success for bad_address2 testcase

[Punit Agrawal]

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>>  void test_bad_address2(swi_func sigwaitinfo, int signo)
>>  {
>> -	TEST(sigwaitinfo((void *)1, NULL, NULL));
>> -	REPORT_SUCCESS(-1, EFAULT);
>> +	pid_t pid;
>> +	int status;
>> +
>> +	switch (pid = FORK_OR_VFORK()) {
>> +	case -1:
>> +		tst_brkm(TBROK | TERRNO, NULL, "fork() failed");
>> +	case 0:
>> +		signal(SIGSEGV, segv_handler);
>> +		TEST(sigwaitinfo((void *)1, NULL, NULL));
>> +
>> +		_exit(0);
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +
>> +	SUCCEED_OR_DIE(waitpid, "waitpid failed", pid, &status, 0);
>> +	if (WIFEXITED(status) && WEXITSTATUS(status) == 0xff)
>> +		tst_resm(TPASS, "Test passed");
>
> Why don't we just check for WIFSIGNALED() and WTERMSIG() instead?
>
> We do not have to install any handler that way...

I think that is what I tried initially but for the tst_sig() call in
setup() that installs a default handler and catches the SIGSEGV.

>
>> +	else
>> +		tst_resm(TFAIL, "Unrecognised child exit code");
>>  }
>>  
>>  void test_bad_address3(swi_func sigwaitinfo, int signo)
>> -- 
>> 2.14.2
>> 

[LTP] [PATCH v2 02/13] thp: ensure THP/hugetlbfs is available

[Punit Agrawal]

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> > I do not get what is the problem with this test on a machine without
>> > hugepages or transparent hugepages. The test just tries to execute true
>> > with excessively large array of command line arguments, if that fails
>> > without hugepages it's a kernel bug.
>> 
>> The test passes even if THP is not enabled on the test kernel. This
>> gives the false impression that THP is working as expected.
>> 
>> The explicit check is making sure the functionality under test is is
>> enabled in the test kernel.
>
> I see where that came from, but I do not think that the test name is a
> reason to disable this stress test. However I'm not sure if there is a
> better name for the test. I guess that we may possibly move it to the
> cve directory.
>

So you're saying the test should run irrespective of whether THP is
enabled or not in the kernel. I'll drop the thp01.c hunk from this patch
then.

>> >> diff --git a/testcases/kernel/mem/thp/thp02.c b/testcases/kernel/mem/thp/thp02.c
>> >> index acc70e215..6502210e5 100644
>> >> --- a/testcases/kernel/mem/thp/thp02.c
>> >> +++ b/testcases/kernel/mem/thp/thp02.c
>> >> @@ -90,6 +90,8 @@ static void setup(void)
>> >>  	if (access(PATH_THP, F_OK) == -1)
>> >>  		tst_brk(TCONF, "THP not enabled in kernel?");
>> >>  
>> >> +	check_hugepage();
>> >> +
>> >>  	ps = sysconf(_SC_PAGESIZE);
>> >>  	hps = SAFE_READ_MEMINFO("Hugepagesize:") * 1024;
>> >>  	size = hps * 4;
>> >> diff --git a/testcases/kernel/mem/thp/thp03.c b/testcases/kernel/mem/thp/thp03.c
>> >> index 19db44944..c6062505f 100644
>> >> --- a/testcases/kernel/mem/thp/thp03.c
>> >> +++ b/testcases/kernel/mem/thp/thp03.c
>> >> @@ -82,6 +82,8 @@ static void setup(void)
>> >>  	if (access(PATH_THP, F_OK) == -1)
>> >>  		tst_brk(TCONF, "THP not enabled in kernel?");
>> >>  
>> >> +	check_hugepage();
>> >> +
>> >>  	hugepage_size = SAFE_READ_MEMINFO("Hugepagesize:") * KB;
>> >>  	unaligned_size = hugepage_size * 4 - 1;
>> >>  	page_size = SAFE_SYSCONF(_SC_PAGESIZE);
>> >
>> > Does this two fail on an attempt to find "Hugepagesize:" in meminfo
>> > file? Otherwise I do not see how they can fail.
>> 
>> Yes, "Hugepagesize:" goes missing in /proc/meminfo. When the kernel
>> doesn't have hugepage support enabled, the tests fail with message like
>> -
>> 
>> safe_file_ops.c:220: BROK: Expected 1 conversions got 0 at thp03.c:85
>> 
>> With the patch, the message helps identify the cause of the failure.
>
> That's fine. Can you plese include this description in the commit
> message as well?

Done for the next version.

Thanks for the review.

[LTP] [PATCH v2 02/13] thp: ensure THP/hugetlbfs is available

[Cyril Hrubis]

Hi!
> >> The test passes even if THP is not enabled on the test kernel. This
> >> gives the false impression that THP is working as expected.
> >> 
> >> The explicit check is making sure the functionality under test is is
> >> enabled in the test kernel.
> >
> > I see where that came from, but I do not think that the test name is a
> > reason to disable this stress test. However I'm not sure if there is a
> > better name for the test. I guess that we may possibly move it to the
> > cve directory.
> >
> 
> So you're saying the test should run irrespective of whether THP is
> enabled or not in the kernel. I'll drop the thp01.c hunk from this patch
> then.

Exactly, it's basically test that tries to pass as large argument list
to exec() as possible, the fact that this has been broken with THP
enabled does not make it irrelevant for kernel without it.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 08/13] sigwaitinfo01: catch SEGV and report success for bad_address2 testcase

[Cyril Hrubis]

Hi!
> > Why don't we just check for WIFSIGNALED() and WTERMSIG() instead?
> >
> > We do not have to install any handler that way...
> 
> I think that is what I tried initially but for the tst_sig() call in
> setup() that installs a default handler and catches the SIGSEGV.

Then just do signal(SIGSEGV, SIGDFL) in the child just before do do the
test. Also it does not make sense to use FORK_OR_VFORK() there because
the thest will not work on uClinux anyway, just use plain old fork()
instead.

> >
> >> +	else
> >> +		tst_resm(TFAIL, "Unrecognised child exit code");
> >>  }
> >>  
> >>  void test_bad_address3(swi_func sigwaitinfo, int signo)
> >> -- 
> >> 2.14.2
> >> 

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 09/13] syscalls/mount03: Copy setuid_test to execute instead of 'TEST FILE'

[Punit Agrawal]

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> When testing the MS_NOSUID mount flag, mount03 forgets to copy
>> setuid_test into the new filesystem. Instead it writes 'TEST FILE' into
>> a new file and attempts to execute it with S_ISUID. This fails, but not
>> for the reasons ltp expects.
>
> What is the exact error here? I guess that the file does not have
> execute bit set and that check kicks in first?

I have a feeling that we might be cross-talking on this one.

IIUC, the intention for the test is to try and execute the binary
setuid_test (built from setuid_test.c). Instead, the test creates a file
named "setuid_test" at the mount location and writes "TEST FILE" into
it.

       snprintf(file, PATH_MAX, "%ssetuid_test", path_name);
       SAFE_FILE_PRINTF(cleanup, file, "TEST FILE");

It then tries to execute this file containing the text string which
fails with the message -

/bin/sh: 0: Can't open /tmp/mouc0EQuB/mntpoint/setuid_test
mount03     6  TPASS  :  mount(2) passed with rwflag = 2

The patch copies the file setuid_test into the mounted filesystem at
which point we no longer get the "/bin/sh:" message.

>
> Anyway we really should check the errno from the exec() is EPERM which
> should have caught this problem.

I would argue that this is another problem with the test and should be
fixed in a separate patch on top.

Let me now if that makes sense. Or have I missed something?

Thanks,
Punit

[LTP] [PATCH v2 09/13] syscalls/mount03: Copy setuid_test to execute instead of 'TEST FILE'

[Cyril Hrubis]

Hi!
> > What is the exact error here? I guess that the file does not have
> > execute bit set and that check kicks in first?
> 
> I have a feeling that we might be cross-talking on this one.
> 
> IIUC, the intention for the test is to try and execute the binary
> setuid_test (built from setuid_test.c). Instead, the test creates a file
> named "setuid_test" at the mount location and writes "TEST FILE" into
> it.
> 
>        snprintf(file, PATH_MAX, "%ssetuid_test", path_name);
>        SAFE_FILE_PRINTF(cleanup, file, "TEST FILE");
> 
> It then tries to execute this file containing the text string which
> fails with the message -
> 
> /bin/sh: 0: Can't open /tmp/mouc0EQuB/mntpoint/setuid_test
> mount03     6  TPASS  :  mount(2) passed with rwflag = 2
> 
> The patch copies the file setuid_test into the mounted filesystem at
> which point we no longer get the "/bin/sh:" message.

Ah, right, I was under an impression that suid programs cannot be
executed on MS_NOSUID mounted FS. But after consulting mount() manual
page that was true for ancient kernels before 2.4 and the suid bits are
simply ignored in this case on newer kernels.

So yes we need to execute a helper that tries to set privileged uid.

The idea of this patch is correct then, but we need to clean up a bit.

First of all all binary helpers should start with the prefix of the test
that uses them, so in this case the helper should be renamed to
mount03_setuid.c, secondly you are supposed to use the
TST_RESOURCE_COPY() macro instead of the function itself and it should
be done once in the test setup as well as the code that sets the actual
setuid bit.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 09/13] syscalls/mount03: Copy setuid_test to execute instead of 'TEST FILE'

[Punit Agrawal]

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> > What is the exact error here? I guess that the file does not have
>> > execute bit set and that check kicks in first?
>> 
>> I have a feeling that we might be cross-talking on this one.
>> 
>> IIUC, the intention for the test is to try and execute the binary
>> setuid_test (built from setuid_test.c). Instead, the test creates a file
>> named "setuid_test" at the mount location and writes "TEST FILE" into
>> it.
>> 
>>        snprintf(file, PATH_MAX, "%ssetuid_test", path_name);
>>        SAFE_FILE_PRINTF(cleanup, file, "TEST FILE");
>> 
>> It then tries to execute this file containing the text string which
>> fails with the message -
>> 
>> /bin/sh: 0: Can't open /tmp/mouc0EQuB/mntpoint/setuid_test
>> mount03     6  TPASS  :  mount(2) passed with rwflag = 2
>> 
>> The patch copies the file setuid_test into the mounted filesystem at
>> which point we no longer get the "/bin/sh:" message.
>
> Ah, right, I was under an impression that suid programs cannot be
> executed on MS_NOSUID mounted FS. But after consulting mount() manual
> page that was true for ancient kernels before 2.4 and the suid bits are
> simply ignored in this case on newer kernels.
>
> So yes we need to execute a helper that tries to set privileged uid.
>
> The idea of this patch is correct then, but we need to clean up a bit.
>
> First of all all binary helpers should start with the prefix of the test
> that uses them, so in this case the helper should be renamed to
> mount03_setuid.c, secondly you are supposed to use the
> TST_RESOURCE_COPY() macro instead of the function itself and it should
> be done once in the test setup as well as the code that sets the actual
> setuid bit.

The copying can't be done in setup() as the test filesystem hasn't been
mounted yet.

I've folded the rest of your suggestions into the patch.

Let me know if you're done reviewing the rest of the series - I can send
out an update with the changes we've discussed in the next day or so
after a bit of testing and tidy-up.

Thanks,
Punit

[LTP] [PATCH v2 10/13] getdtablesize01: Handle ENFILE errno

[Cyril Hrubis]

Hi!
>  //Now the max files opened should be RLIMIT_NOFILE - 1 , why ? read getdtablesize man page
>  
> -	if (count > 0)
> -		close(count);
>  	if (count == (max_val_opfiles - 1))
>  		tst_resm(TPASS, "%d = %d", count, (max_val_opfiles - 1));

We may also check for EMFILE here.

> +	else if (fd < 0 && errno == ENFILE)
> +		tst_brkm(TBROK, cleanup, "Reached maximum number of open files for the system");
                           ^
			   This should rather be TCONF i.e. skipped.

>  	else
>  		tst_resm(TFAIL, "%d != %d", count, (max_val_opfiles - 1));
>  
> +	if (count > 0)
> +		close(count);
> +
>  	cleanup();
>  	tst_exit();
>  }
> -- 
> 2.14.2
> 

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 09/13] syscalls/mount03: Copy setuid_test to execute instead of 'TEST FILE'

[Cyril Hrubis]

Hi!
> The copying can't be done in setup() as the test filesystem hasn't been
> mounted yet.

Well we can always mount it in the setup, prepare it for the testing,
then umount it again...

> Let me know if you're done reviewing the rest of the series - I can send
> out an update with the changes we've discussed in the next day or so
> after a bit of testing and tidy-up.

I will look closely on the cpu_hotplug patches then I'm done. The rest
of the patches I haven't commeted are OK.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 12/13] hotplug/cpu_hotplug: Repopulate cgroup:cpusets after testing hotplug

[Cyril Hrubis]

Hi!
> LTP tests that online CPUs must test for the legacy cgroup hierarchy and
> update the cpusets for all users/sessions. Add this code the
> cpu_online() helper.

I'm not sure if we want to work around kernel/systemd bugs like this,
but even if we do we probably want to inform user about the bug.  Maybe
we should just check if the cpus are disabled after the test, if so,
restore them and fail the test.

> Signed-off-by: James Morse <james.morse@arm.com>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> ---
>  .../cpu_hotplug/include/cpuhotplug_hotplug.sh      | 36 ++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/testcases/kernel/hotplug/cpu_hotplug/include/cpuhotplug_hotplug.sh b/testcases/kernel/hotplug/cpu_hotplug/include/cpuhotplug_hotplug.sh
> index 9f856e554..35ec7c86b 100644
> --- a/testcases/kernel/hotplug/cpu_hotplug/include/cpuhotplug_hotplug.sh
> +++ b/testcases/kernel/hotplug/cpu_hotplug/include/cpuhotplug_hotplug.sh
> @@ -58,6 +58,39 @@ set_affinity()
>      return $?
>  }
>  
> +# repopulate_cpusets()
> +#
> +# If the legacy cgroup hierarchy is in use, and cpuset is mounted, copy
> +# the permitted cpus from the root group, which always has all available
> +# cpus and memory, into the other hierarchy nodes. This prevents hotplug
> +# tests from losing CPUs, and affecting the results of later tests.
> +#
> +CPUSET="/sys/fs/cgroup/cpuset"
> +repopulate_cpusets()
> +{
> +	# Only if we have cpusets
> +	if [ ! -d ${CPUSET} ] ; then
> +		return;
> +	fi
> +
> +	# And then only legacy cpusets
> +	if grep --quiet "/sys/fs/cgroup/cpuset[ ]*cgroup2" /proc/mounts ; then
> +		return;
> +	fi
> +
> +	# Copy root cpus into system and user slices
> +	cat ${CPUSET}/cpuset.cpus > ${CPUSET}/system.slice/cpuset.cpus
> +	cat ${CPUSET}/cpuset.cpus > ${CPUSET}/user.slice/cpuset.cpus

This will fail on distros that do not use systemd, we should check first
if system.slice and user.slice directories are present.

> +	# Fix each session of each user
> +	for TUSER in ${CPUSET}/user.slice/user-*.slice ; do
> +		cat ${CPUSET}/cpuset.cpus > ${TUSER}/cpuset.cpus
> +		for SESSION in ${TUSER}/session-*.scope; do
> +			cat ${CPUSET}/cpuset.cpus > ${SESSION}/cpuset.cpus
> +		done
> +	done
> +
> +}
> +
>  # online_cpu(CPU)
>  #
>  #  Onlines the given CPU.  Returns a true value if it was able
> @@ -77,6 +110,9 @@ online_cpu()
>  
>      $TIME echo 1 > /sys/devices/system/cpu/cpu${CPU}/online
>      RC=$?
> +
> +    repopulate_cpusets
> +
>      report_timing "Online cpu ${CPU}"
>      return $RC
>  }
> -- 
> 2.14.2
> 

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 09/13] syscalls/mount03: Copy setuid_test to execute instead of 'TEST FILE'

[Punit Agrawal]

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> The copying can't be done in setup() as the test filesystem hasn't been
>> mounted yet.
>
> Well we can always mount it in the setup, prepare it for the testing,
> then umount it again...

Ok, done that now.

>
>> Let me know if you're done reviewing the rest of the series - I can send
>> out an update with the changes we've discussed in the next day or so
>> after a bit of testing and tidy-up.
>
> I will look closely on the cpu_hotplug patches then I'm done. The rest
> of the patches I haven't commeted are OK.

Cool, thanks!

[LTP] [PATCH v2 10/13] getdtablesize01: Handle ENFILE errno

[Punit Agrawal]

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>>  //Now the max files opened should be RLIMIT_NOFILE - 1 , why ? read getdtablesize man page
>>  
>> -	if (count > 0)
>> -		close(count);
>>  	if (count == (max_val_opfiles - 1))
>>  		tst_resm(TPASS, "%d = %d", count, (max_val_opfiles - 1));
>
> We may also check for EMFILE here.

I don't understand what you're suggesting here.

>
>> +	else if (fd < 0 && errno == ENFILE)
>> +		tst_brkm(TBROK, cleanup, "Reached maximum number of open files for the system");
>                            ^
> 			   This should rather be TCONF i.e. skipped.

I've changed TBROK to TCONF now.

Thanks.

[...]

[LTP] [PATCH v2 10/13] getdtablesize01: Handle ENFILE errno

[Cyril Hrubis]

Hi!
> >>  //Now the max files opened should be RLIMIT_NOFILE - 1 , why ? read getdtablesize man page
> >>  
> >> -	if (count > 0)
> >> -		close(count);
> >>  	if (count == (max_val_opfiles - 1))
> >>  		tst_resm(TPASS, "%d = %d", count, (max_val_opfiles - 1));
> >
> > We may also check for EMFILE here.
> 
> I don't understand what you're suggesting here.

When process has exhausted the maximal number of file descriptors it can
open we are supposted to fail with EMFILE, when that happened to the
whole system before we hit the per process limit we get ENFILE. So I was
suggesting that we may as well check that we were able to open expected
number of file descriptors and that the last open() failed with EMFILE.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 10/13] getdtablesize01: Handle ENFILE errno

[Suzuki K Poulose]

On 30/11/17 16:20, Cyril Hrubis wrote:
> Hi!
>>>>   //Now the max files opened should be RLIMIT_NOFILE - 1 , why ? read getdtablesize man page
>>>>   
>>>> -	if (count > 0)
>>>> -		close(count);
>>>>   	if (count == (max_val_opfiles - 1))
>>>>   		tst_resm(TPASS, "%d = %d", count, (max_val_opfiles - 1));
>>>
>>> We may also check for EMFILE here.
>>
>> I don't understand what you're suggesting here.
> 
> When process has exhausted the maximal number of file descriptors it can
> open we are supposted to fail with EMFILE, when that happened to the
> whole system before we hit the per process limit we get ENFILE. So I was
> suggesting that we may as well check that we were able to open expected
> number of file descriptors and that the last open() failed with EMFILE.

But isn't that we check already ? I mean, the test already gets the limit
on number of open files for this process and tries to open one less fd.
So I don't think we should check that error. If we get that error, something
else is wrong in the accounting and should be treated as an error.

Suzuki



> 

[LTP] [PATCH v2 10/13] getdtablesize01: Handle ENFILE errno

[Cyril Hrubis]

Hi!
> > When process has exhausted the maximal number of file descriptors it can
> > open we are supposted to fail with EMFILE, when that happened to the
> > whole system before we hit the per process limit we get ENFILE. So I was
> > suggesting that we may as well check that we were able to open expected
> > number of file descriptors and that the last open() failed with EMFILE.
> 
> But isn't that we check already ? I mean, the test already gets the limit
> on number of open files for this process and tries to open one less fd.

As far as I can tell it tries to open as much fds as is the limit, the
loop is kind of silly since it starts at 1 and the condition is <= but
that should call open the open number of limit times.

> So I don't think we should check that error. If we get that error, something
> else is wrong in the accounting and should be treated as an error.

But the loop never finishes and the open() always fails with EMFILE
because process has at least three fds opened when it starts (the
stdin/stdout/stderr are inherited from the parent). Hence the loop is
always broken on fd == -1.

I guess that we may as well change the loop to infinite one, which would
be less confusing in this case.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 10/13] getdtablesize01: Handle ENFILE errno

[Punit Agrawal]

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> > When process has exhausted the maximal number of file descriptors it can
>> > open we are supposted to fail with EMFILE, when that happened to the
>> > whole system before we hit the per process limit we get ENFILE. So I was
>> > suggesting that we may as well check that we were able to open expected
>> > number of file descriptors and that the last open() failed with EMFILE.
>> 
>> But isn't that we check already ? I mean, the test already gets the limit
>> on number of open files for this process and tries to open one less fd.
>
> As far as I can tell it tries to open as much fds as is the limit, the
> loop is kind of silly since it starts at 1 and the condition is <= but
> that should call open the open number of limit times.
>
>> So I don't think we should check that error. If we get that error, something
>> else is wrong in the accounting and should be treated as an error.
>
> But the loop never finishes and the open() always fails with EMFILE
> because process has at least three fds opened when it starts (the
> stdin/stdout/stderr are inherited from the parent). Hence the loop is
> always broken on fd == -1.
>
> I guess that we may as well change the loop to infinite one, which would
> be less confusing in this case.

Ok, I've changed the for loop to start from 0 and dropped the stopping
condition.

Thanks,
Punit

[LTP] [PATCH v2 12/13] hotplug/cpu_hotplug: Repopulate cgroup:cpusets after testing hotplug

[Punit Agrawal]

Hi Cyril,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> LTP tests that online CPUs must test for the legacy cgroup hierarchy and
>> update the cpusets for all users/sessions. Add this code the
>> cpu_online() helper.
>
> I'm not sure if we want to work around kernel/systemd bugs like this,
> but even if we do we probably want to inform user about the bug.  Maybe
> we should just check if the cpus are disabled after the test, if so,
> restore them and fail the test.

It turns out that the problem behaviour is specific to the filesystem
(Ubuntu 14.04).

On a recent debian filesystem I tested with, systemd doesn't even seem
to use cpusets by default. I imagine other distros make similarly
different choices. :)

If anybody faces the same (or similar) problem with reduced performance
after runnnig ltp, they can find their way to this commit now that it's
on the list (and you're aware of the problem).

I am going to drop this patch from the next version.

Thanks,
Punit

>
>> Signed-off-by: James Morse <james.morse@arm.com>
>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>> ---
>>  .../cpu_hotplug/include/cpuhotplug_hotplug.sh      | 36 ++++++++++++++++++++++
>>  1 file changed, 36 insertions(+)
>> 
>> diff --git a/testcases/kernel/hotplug/cpu_hotplug/include/cpuhotplug_hotplug.sh b/testcases/kernel/hotplug/cpu_hotplug/include/cpuhotplug_hotplug.sh
>> index 9f856e554..35ec7c86b 100644
>> --- a/testcases/kernel/hotplug/cpu_hotplug/include/cpuhotplug_hotplug.sh
>> +++ b/testcases/kernel/hotplug/cpu_hotplug/include/cpuhotplug_hotplug.sh
>> @@ -58,6 +58,39 @@ set_affinity()
>>      return $?
>>  }
>>  
>> +# repopulate_cpusets()
>> +#
>> +# If the legacy cgroup hierarchy is in use, and cpuset is mounted, copy
>> +# the permitted cpus from the root group, which always has all available
>> +# cpus and memory, into the other hierarchy nodes. This prevents hotplug
>> +# tests from losing CPUs, and affecting the results of later tests.
>> +#
>> +CPUSET="/sys/fs/cgroup/cpuset"
>> +repopulate_cpusets()
>> +{
>> +	# Only if we have cpusets
>> +	if [ ! -d ${CPUSET} ] ; then
>> +		return;
>> +	fi
>> +
>> +	# And then only legacy cpusets
>> +	if grep --quiet "/sys/fs/cgroup/cpuset[ ]*cgroup2" /proc/mounts ; then
>> +		return;
>> +	fi
>> +
>> +	# Copy root cpus into system and user slices
>> +	cat ${CPUSET}/cpuset.cpus > ${CPUSET}/system.slice/cpuset.cpus
>> +	cat ${CPUSET}/cpuset.cpus > ${CPUSET}/user.slice/cpuset.cpus
>
> This will fail on distros that do not use systemd, we should check first
> if system.slice and user.slice directories are present.
>
>> +	# Fix each session of each user
>> +	for TUSER in ${CPUSET}/user.slice/user-*.slice ; do
>> +		cat ${CPUSET}/cpuset.cpus > ${TUSER}/cpuset.cpus
>> +		for SESSION in ${TUSER}/session-*.scope; do
>> +			cat ${CPUSET}/cpuset.cpus > ${SESSION}/cpuset.cpus
>> +		done
>> +	done
>> +
>> +}
>> +
>>  # online_cpu(CPU)
>>  #
>>  #  Onlines the given CPU.  Returns a true value if it was able
>> @@ -77,6 +110,9 @@ online_cpu()
>>  
>>      $TIME echo 1 > /sys/devices/system/cpu/cpu${CPU}/online
>>      RC=$?
>> +
>> +    repopulate_cpusets
>> +
>>      report_timing "Online cpu ${CPU}"
>>      return $RC
>>  }
>> -- 
>> 2.14.2
>>