Skip to contentSkip to conversation

open_posix: add SAFE_PFUNC macro

PATCH New Jan Stancek 2 participants 13 messages Feb 19, 2016 – Feb 23, 2016

lore
Share & tools
lore

Jan Stancek

author
#0 lore raw

PATCH 1/5 open_posix: add SAFE_PFUNC macro

Simple macro to exit test and print error if a pthread_* function returned non-zero.

Signed-off-by Jan Stancek <[email protected]>
---
 testcases/open_posix_testsuite/include/safe_helpers.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 testcases/open_posix_testsuite/include/safe_helpers.h
testcases/open_posix_testsuite/include/safe_helpers.h new file +17 -0
new file mode 100644
index 000000000000..655e158f65d0
@@ -0,0 +1,17 @@
+#ifndef __SAFE_MACROS_H__
+#define __SAFE_MACROS_H__
+
+#include <string.h>
+
+#define SAFE_PFUNC(op) \
+do {\
+	int ret = (op); \
+	if (ret != 0) { \
+		printf("Test %s unresolved: got %i (%s) on line %i\n  %s\n", \
+			__FILE__, ret, strerror(ret), __LINE__, #op); \
+		fflush(stdout); \
+		exit(PTS_UNRESOLVED); \
+	} \
+} while (0)
+
+#endif
Signature · 1 line
1.8.3.1
author
#1 lore raw

PATCH 2/5 open_posix: condvar/schedule: use SAFE_PFUNC

Replace common error checking with SAFE_PFUNC macro. Also remove some obvious comments.

Signed-off-by Jan Stancek <[email protected]>
---
 .../threads/condvar/pthread_cond_wait_1.c          | 136 ++++-------------
 .../threads/condvar/pthread_cond_wait_2.c          | 135 +++-------------
 .../functional/threads/schedule/1-1.c              | 137 +++++------------
 .../functional/threads/schedule/1-2.c              | 169 ++++-----------------
 4 files changed, 115 insertions(+), 462 deletions(-)
testcases/open_posix_testsuite/functional/threads/condvar/pthread_cond_wait_1.c +26 -110
index 123d1e56d4cf..5b36fc24a3b8 100644
@@ -4,18 +4,17 @@
  * This file is licensed under the GPL license.  For the full content
  * of this license, see the COPYING file at the top level of this
  * source tree.
-
+ *
  * Test that pthread_cond_signal()
  *   shall wakeup a high priority thread even when a low priority thread
  *   is running
-
+ *
  * Steps:
  * 1. Create a condition variable
  * 2. Create a high priority thread and make it wait on the cond
  * 3. Create a low priority thread and let it busy-loop
  * 4. Signal the cond in a signal handler and check that high
  *    priority thread got woken up
- *
  */
 
 #include <pthread.h>

@@ -25,6 +24,7 @@
 #include <unistd.h>
 #include <time.h>
 #include "posixtest.h"
+#include "safe_helpers.h"
 
 #define TEST "5-1"
 #define AREA "scheduler"

@@ -35,27 +35,19 @@
 #define RUNTIME       5
 #define POLICY        SCHED_RR
 
-/* mutex required by the cond variable */
 pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
-/* condition variable that threads block on*/
 pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
 
 /* Flags that the threads use to indicate events */
 int woken_up = -1;
 int low_done = -1;
 
-/* Signal handler that handle the ALRM and wakes up
- * the high priority thread
- */
 void signal_handler(int sig)
 {
-	if (pthread_cond_signal(&cond) != 0) {
-		printf(ERROR_PREFIX "pthread_cond_signal\n");
-		exit(PTS_UNRESOLVED);
-	}
+	(void) sig;
+	SAFE_PFUNC(pthread_cond_signal(&cond));
 }
 
-/* Utility function to find difference between two time values */
 float timediff(struct timespec t2, struct timespec t1)
 {
 	float diff = t2.tv_sec - t1.tv_sec;

@@ -63,51 +55,32 @@ float timediff(struct timespec t2, struct timespec t1)
 	return diff;
 }
 
-void *hi_priority_thread(void *tmp)
+void *hi_prio_thread(void *tmp)
 {
 	struct sched_param param;
 	int policy;
-	int rc = 0;
 
+	(void) tmp;
 	param.sched_priority = HIGH_PRIORITY;
 
-	rc = pthread_setschedparam(pthread_self(), POLICY, &param);
-	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_setschedparam\n");
-		exit(PTS_UNRESOLVED);
-	}
-	rc = pthread_getschedparam(pthread_self(), &policy, &param);
-	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_getschedparam\n");
-		exit(PTS_UNRESOLVED);
-	}
+	SAFE_PFUNC(pthread_setschedparam(pthread_self(), POLICY, &param));
+	SAFE_PFUNC(pthread_getschedparam(pthread_self(), &policy, &param));
 	if ((policy != POLICY) || (param.sched_priority != HIGH_PRIORITY)) {
 		printf("Error: the policy or priority not correct\n");
 		exit(PTS_UNRESOLVED);
 	}
 
-	/* Install a signal handler for ALRM */
 	if (signal(SIGALRM, signal_handler) != 0) {
 		perror(ERROR_PREFIX "signal:");
 		exit(PTS_UNRESOLVED);
 	}
 
-	/* acquire the mutex */
-	rc = pthread_mutex_lock(&mutex);
-	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_mutex_lock\n");
-		exit(PTS_UNRESOLVED);
-	}
+	SAFE_PFUNC(pthread_mutex_lock(&mutex));
 
-	/* Setup an alarm to go off in 2 seconds */
 	alarm(2);
 
 	/* Block, to be woken up by the signal handler */
-	rc = pthread_cond_wait(&cond, &mutex);
-	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_cond_wait\n");
-		exit(PTS_UNRESOLVED);
-	}
+	SAFE_PFUNC(pthread_cond_wait(&cond, &mutex));
 
 	/* This variable is unprotected because the scheduling removes
 	 * the contention

@@ -115,39 +88,26 @@ void *hi_priority_thread(void *tmp)
 	if (low_done != 1)
 		woken_up = 1;
 
-	rc = pthread_mutex_unlock(&mutex);
-	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_mutex_unlock\n");
-		exit(PTS_UNRESOLVED);
-	}
+	SAFE_PFUNC(pthread_mutex_unlock(&mutex));
 	return NULL;
 }
 
-void *low_priority_thread(void *tmp)
+void *low_prio_thread(void *tmp)
 {
 	struct timespec start_time, current_time;
 	struct sched_param param;
 	int policy;
-	int rc = 0;
 
+	(void) tmp;
 	param.sched_priority = LOW_PRIORITY;
 
-	rc = pthread_setschedparam(pthread_self(), POLICY, &param);
-	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_setschedparam\n");
-		exit(PTS_UNRESOLVED);
-	}
-	rc = pthread_getschedparam(pthread_self(), &policy, &param);
-	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_getschedparam\n");
-		exit(PTS_UNRESOLVED);
-	}
+	SAFE_PFUNC(pthread_setschedparam(pthread_self(), POLICY, &param));
+	SAFE_PFUNC(pthread_getschedparam(pthread_self(), &policy, &param));
 	if ((policy != POLICY) || (param.sched_priority != LOW_PRIORITY)) {
 		printf("Error: the policy or priority not correct\n");
 		exit(PTS_UNRESOLVED);
 	}
 
-	/* grab the start time and busy loop for 5 seconds */
 	clock_gettime(CLOCK_REALTIME, &start_time);
 	while (1) {
 		clock_gettime(CLOCK_REALTIME, &current_time);

@@ -163,69 +123,25 @@ int main()
 	pthread_t high_id, low_id;
 	pthread_attr_t high_attr, low_attr;
 	struct sched_param param;
-	int rc = 0;
 
 	/* Create the higher priority thread */
-	rc = pthread_attr_init(&high_attr);
-	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_attr_init\n");
-		exit(PTS_UNRESOLVED);
-	}
-
-	rc = pthread_attr_setschedpolicy(&high_attr, POLICY);
-	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_attr_setschedpolicy\n");
-		exit(PTS_UNRESOLVED);
-	}
+	SAFE_PFUNC(pthread_attr_init(&high_attr));
+	SAFE_PFUNC(pthread_attr_setschedpolicy(&high_attr, POLICY));
 	param.sched_priority = HIGH_PRIORITY;
-	rc = pthread_attr_setschedparam(&high_attr, &param);
-	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_attr_setschedparam\n");
-		exit(PTS_UNRESOLVED);
-	}
-	rc = pthread_create(&high_id, &high_attr, hi_priority_thread, NULL);
-	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_create\n");
-		exit(PTS_UNRESOLVED);
-	}
+	SAFE_PFUNC(pthread_attr_setschedparam(&high_attr, &param));
+	SAFE_PFUNC(pthread_create(&high_id, &high_attr, hi_prio_thread, NULL));
 
 	/* Create the low priority thread */
-	rc = pthread_attr_init(&low_attr);
-	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_attr_init\n");
-		exit(PTS_UNRESOLVED);
-	}
-	rc = pthread_attr_setschedpolicy(&low_attr, POLICY);
-	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_attr_setschedpolicy\n");
-		exit(PTS_UNRESOLVED);
-	}
+	SAFE_PFUNC(pthread_attr_init(&low_attr));
+	SAFE_PFUNC(pthread_attr_setschedpolicy(&low_attr, POLICY));
 	param.sched_priority = LOW_PRIORITY;
-	rc = pthread_attr_setschedparam(&low_attr, &param);
-	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_attr_setschedparam\n");
-		exit(PTS_UNRESOLVED);
-	}
-	rc = pthread_create(&low_id, &low_attr, low_priority_thread, NULL);
-	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_create\n");
-		exit(PTS_UNRESOLVED);
-	}
+	SAFE_PFUNC(pthread_attr_setschedparam(&low_attr, &param));
+	SAFE_PFUNC(pthread_create(&low_id, &low_attr, low_prio_thread, NULL));
 
 	/* Wait for the threads to exit */
-	rc = pthread_join(high_id, NULL);
-	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_join\n");
-		exit(PTS_UNRESOLVED);
-	}
-
-	rc = pthread_join(low_id, NULL);
-	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_join\n");
-		exit(PTS_UNRESOLVED);
-	}
+	SAFE_PFUNC(pthread_join(high_id, NULL));
+	SAFE_PFUNC(pthread_join(low_id, NULL));
 
-	/* Check the result */
 	if (woken_up == -1) {
 		printf("Test FAILED: high priority was not woken up\\n");
 		exit(PTS_FAIL);
testcases/open_posix_testsuite/functional/threads/condvar/pthread_cond_wait_2.c +25 -110

232 lines not on this page · raw message

testcases/open_posix_testsuite/functional/threads/schedule/1-1.c +34 -103

218 lines not on this page · raw message

testcases/open_posix_testsuite/functional/threads/schedule/1-2.c +30 -139

270 lines not on this page · raw message

Signature · 1 line
1.8.3.1
author
#3 lore raw

PATCH 3/5 open_posix: condvar/schedule: remove useless waiting

In reply to Jan Stancek

If high prio thread wakes up, test is done, there's no point waiting another 3 seconds for low prio thread to end its busy loop.

If test PASS-es, we save 3 seconds per test.

Signed-off-by Jan Stancek <[email protected]>
---
 .../functional/threads/condvar/pthread_cond_wait_1.c         | 12 ++++++------
 .../functional/threads/condvar/pthread_cond_wait_2.c         | 12 ++++++------
 .../open_posix_testsuite/functional/threads/schedule/1-1.c   | 10 +++++-----
 .../open_posix_testsuite/functional/threads/schedule/1-2.c   | 10 +++++-----
 4 files changed, 22 insertions(+), 22 deletions(-)
testcases/open_posix_testsuite/functional/threads/condvar/pthread_cond_wait_1.c +6 -6
index 5b36fc24a3b8..dfbf44bdefd7 100644
@@ -39,8 +39,8 @@ pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
 pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
 
 /* Flags that the threads use to indicate events */
-int woken_up = -1;
-int low_done = -1;
+static volatile int woken_up;
+static volatile int low_done;
 
 void signal_handler(int sig)
 {

@@ -85,7 +85,7 @@ void *hi_prio_thread(void *tmp)
 	/* This variable is unprotected because the scheduling removes
 	 * the contention
 	 */
-	if (low_done != 1)
+	if (!low_done)
 		woken_up = 1;
 
 	SAFE_PFUNC(pthread_mutex_unlock(&mutex));

@@ -109,7 +109,7 @@ void *low_prio_thread(void *tmp)
 	}
 
 	clock_gettime(CLOCK_REALTIME, &start_time);
-	while (1) {
+	while (!woken_up) {
 		clock_gettime(CLOCK_REALTIME, &current_time);
 		if (timediff(current_time, start_time) > RUNTIME)
 			break;

@@ -142,8 +142,8 @@ int main()
 	SAFE_PFUNC(pthread_join(high_id, NULL));
 	SAFE_PFUNC(pthread_join(low_id, NULL));
 
-	if (woken_up == -1) {
-		printf("Test FAILED: high priority was not woken up\\n");
+	if (!woken_up) {
+		printf("Test FAILED: high priority was not woken up\n");
 		exit(PTS_FAIL);
 	}
testcases/open_posix_testsuite/functional/threads/condvar/pthread_cond_wait_2.c +6 -6
index 7308bd6b98d1..c5670f8e26fd 100644
@@ -39,8 +39,8 @@ pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
 pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
 
 /* Flags that the threads use to indicate events */
-int woken_up = -1;
-int low_done = -1;
+static volatile int woken_up;
+static volatile int low_done;
 
 void signal_handler(int sig)
 {

@@ -85,7 +85,7 @@ void *hi_prio_thread(void *tmp)
 	/* This variable is unprotected because the scheduling removes
 	 * the contention
 	 */
-	if (low_done != 1)
+	if (!low_done)
 		woken_up = 1;
 
 	SAFE_PFUNC(pthread_mutex_unlock(&mutex));

@@ -109,7 +109,7 @@ void *low_prio_thread(void *tmp)
 	}
 
 	clock_gettime(CLOCK_REALTIME, &start_time);
-	while (1) {
+	while (!woken_up) {
 		clock_gettime(CLOCK_REALTIME, &current_time);
 		if (timediff(current_time, start_time) > RUNTIME)
 			break;

@@ -142,8 +142,8 @@ int main()
 	SAFE_PFUNC(pthread_join(high_id, NULL));
 	SAFE_PFUNC(pthread_join(low_id, NULL));
 
-	if (woken_up == -1) {
-		printf("Test FAILED: high priority was not woken up\\n");
+	if (!woken_up) {
+		printf("Test FAILED: high priority was not woken up\n");
 		exit(PTS_FAIL);
 	}
testcases/open_posix_testsuite/functional/threads/schedule/1-1.c +5 -5
index f1bf37292b53..a6b634c7c59d 100644
@@ -37,8 +37,8 @@
 #define RUNTIME 5
 
 pthread_barrier_t barrier;
-volatile int woken_up = -1;
-volatile int low_done = -1;
+static volatile int woken_up;
+static volatile int low_done;
 
 float timediff(struct timespec t2, struct timespec t1)
 {

@@ -91,7 +91,7 @@ void *hi_prio_thread(void *tmp)
 	/* This variable is unprotected because the scheduling removes
 	 * the contention
 	 */
-	if (low_done != 1)
+	if (!low_done)
 		woken_up = 1;
 
 	pthread_exit(NULL);

@@ -113,7 +113,7 @@ void *low_prio_thread(void *tmp)
 	}
 
 	clock_gettime(CLOCK_REALTIME, &start_timespec);
-	while (1) {
+	while (!woken_up) {
 		clock_gettime(CLOCK_REALTIME, &current_timespec);
 		if (timediff(current_timespec, start_timespec) > RUNTIME)
 			break;

@@ -149,7 +149,7 @@ int main()
 	SAFE_PFUNC(pthread_join(high_id, NULL));
 	SAFE_PFUNC(pthread_join(low_id, NULL));
 
-	if (woken_up == -1) {
+	if (!woken_up) {
 		printf("High priority was not woken up. Test FAILED\n");
 		exit(PTS_FAIL);
 	}
testcases/open_posix_testsuite/functional/threads/schedule/1-2.c +5 -5
index 3d3427b8ec4b..f7b0bd89e081 100644
@@ -39,8 +39,8 @@ pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
 pthread_mutex_t cond_mutex = PTHREAD_MUTEX_INITIALIZER;
 pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
 
-volatile int woken_up = -1;
-volatile int low_done = -1;
+static volatile int woken_up;
+static volatile int low_done;
 
 float timediff(struct timespec t2, struct timespec t1)
 {

@@ -78,7 +78,7 @@ void *hi_prio_thread(void *tmp)
 	/* This variable is unprotected because the scheduling removes
 	 * the contention
 	 */
-	if (low_done != 1)
+	if (!low_done)
 		woken_up = 1;
 
 	SAFE_PFUNC(pthread_mutex_unlock(&mutex));

@@ -105,7 +105,7 @@ void *low_prio_thread(void *tmp)
 	}
 
 	clock_gettime(CLOCK_REALTIME, &start_time);
-	while (1) {
+	while (!woken_up) {
 		clock_gettime(CLOCK_REALTIME, &current_time);
 		if (timediff(current_time, start_time) > RUNTIME)
 			break;

@@ -168,7 +168,7 @@ int main()
 	SAFE_PFUNC(pthread_join(high_id, NULL));
 	SAFE_PFUNC(pthread_join(low_id, NULL));
 
-	if (woken_up == -1) {
+	if (!woken_up) {
 		printf("High priority was not woken up. Test FAILED.\n");
 		exit(PTS_FAIL);
 	}
Signature · 1 line
1.8.3.1
#4 lore raw

PATCH 3/5 open_posix: condvar/schedule: remove useless waiting

Hi!

Quoted 4 lines

If high prio thread wakes up, test is done, there's no point waiting another 3 seconds for low prio thread to end its busy loop.

If test PASS-es, we save 3 seconds per test.

Looks good, acked.

Signature · 2 lines
Cyril Hrubis
[email protected]
author
#5 lore raw

PATCH 4/5 open_posix: condvar/schedule: remove duplicit setsched calls

In reply to Jan Stancek

pthread_attr_setschedparam's man page says:
In order for the parameter setting made by pthread_attr_setschedparam() to have effect when calling pthread_create(3), the caller must use pthread_attr_setinheritsched(3) to set the inherit-scheduler attribute of the attributes object attr to PTHREAD_EXPLICIT_SCHED.

So add PTHREAD_EXPLICIT_SCHED to attr used during pthread_create and remove duplicit pthread_setsched calls in each thread.

Signed-off-by Jan Stancek <[email protected]>
---
 .../functional/threads/condvar/pthread_cond_wait_1.c             | 6 ++----
 .../functional/threads/condvar/pthread_cond_wait_2.c             | 6 ++----
 testcases/open_posix_testsuite/functional/threads/schedule/1-1.c | 8 ++++----
 testcases/open_posix_testsuite/functional/threads/schedule/1-2.c | 9 ++++-----
 4 files changed, 12 insertions(+), 17 deletions(-)
testcases/open_posix_testsuite/functional/threads/condvar/pthread_cond_wait_1.c +2 -4
index dfbf44bdefd7..a674efc406ef 100644
@@ -61,9 +61,7 @@ void *hi_prio_thread(void *tmp)
 	int policy;
 
 	(void) tmp;
-	param.sched_priority = HIGH_PRIORITY;
 
-	SAFE_PFUNC(pthread_setschedparam(pthread_self(), POLICY, &param));
 	SAFE_PFUNC(pthread_getschedparam(pthread_self(), &policy, &param));
 	if ((policy != POLICY) || (param.sched_priority != HIGH_PRIORITY)) {
 		printf("Error: the policy or priority not correct\n");

@@ -99,9 +97,7 @@ void *low_prio_thread(void *tmp)
 	int policy;
 
 	(void) tmp;
-	param.sched_priority = LOW_PRIORITY;
 
-	SAFE_PFUNC(pthread_setschedparam(pthread_self(), POLICY, &param));
 	SAFE_PFUNC(pthread_getschedparam(pthread_self(), &policy, &param));
 	if ((policy != POLICY) || (param.sched_priority != LOW_PRIORITY)) {
 		printf("Error: the policy or priority not correct\n");

@@ -126,6 +122,7 @@ int main()
 
 	/* Create the higher priority thread */
 	SAFE_PFUNC(pthread_attr_init(&high_attr));
+	SAFE_PFUNC(pthread_attr_setinheritsched(&high_attr, PTHREAD_EXPLICIT_SCHED));
 	SAFE_PFUNC(pthread_attr_setschedpolicy(&high_attr, POLICY));
 	param.sched_priority = HIGH_PRIORITY;
 	SAFE_PFUNC(pthread_attr_setschedparam(&high_attr, &param));

@@ -133,6 +130,7 @@ int main()
 
 	/* Create the low priority thread */
 	SAFE_PFUNC(pthread_attr_init(&low_attr));
+	SAFE_PFUNC(pthread_attr_setinheritsched(&low_attr, PTHREAD_EXPLICIT_SCHED));
 	SAFE_PFUNC(pthread_attr_setschedpolicy(&low_attr, POLICY));
 	param.sched_priority = LOW_PRIORITY;
 	SAFE_PFUNC(pthread_attr_setschedparam(&low_attr, &param));
testcases/open_posix_testsuite/functional/threads/condvar/pthread_cond_wait_2.c +2 -4
index c5670f8e26fd..398b8db5448e 100644
@@ -61,9 +61,7 @@ void *hi_prio_thread(void *tmp)
 	int policy;
 
 	(void) tmp;
-	param.sched_priority = HIGH_PRIORITY;
 
-	SAFE_PFUNC(pthread_setschedparam(pthread_self(), POLICY, &param));
 	SAFE_PFUNC(pthread_getschedparam(pthread_self(), &policy, &param));
 	if ((policy != POLICY) || (param.sched_priority != HIGH_PRIORITY)) {
 		printf("Error: the policy or priority not correct\n");

@@ -99,9 +97,7 @@ void *low_prio_thread(void *tmp)
 	int policy;
 
 	(void) tmp;
-	param.sched_priority = LOW_PRIORITY;
 
-	SAFE_PFUNC(pthread_setschedparam(pthread_self(), POLICY, &param));
 	SAFE_PFUNC(pthread_getschedparam(pthread_self(), &policy, &param));
 	if ((policy != POLICY) || (param.sched_priority != LOW_PRIORITY)) {
 		printf("Error: the policy or priority not correct\n");

@@ -126,6 +122,7 @@ int main()
 
 	/* Create the higher priority thread */
 	SAFE_PFUNC(pthread_attr_init(&high_attr));
+	SAFE_PFUNC(pthread_attr_setinheritsched(&high_attr, PTHREAD_EXPLICIT_SCHED));
 	SAFE_PFUNC(pthread_attr_setschedpolicy(&high_attr, POLICY));
 	param.sched_priority = HIGH_PRIORITY;
 	SAFE_PFUNC(pthread_attr_setschedparam(&high_attr, &param));

@@ -133,6 +130,7 @@ int main()
 
 	/* Create the low priority thread */
 	SAFE_PFUNC(pthread_attr_init(&low_attr));
+	SAFE_PFUNC(pthread_attr_setinheritsched(&low_attr, PTHREAD_EXPLICIT_SCHED));
 	SAFE_PFUNC(pthread_attr_setschedpolicy(&low_attr, POLICY));
 	param.sched_priority = LOW_PRIORITY;
 	SAFE_PFUNC(pthread_attr_setschedparam(&low_attr, &param));
testcases/open_posix_testsuite/functional/threads/schedule/1-1.c +4 -4
index a6b634c7c59d..c65e127bdf5b 100644
@@ -70,8 +70,7 @@ void *hi_prio_thread(void *tmp)
 	void *previous_signal;
 
 	(void) tmp;
-	param.sched_priority = HIGH_PRIORITY;
-	SAFE_PFUNC(pthread_setschedparam(pthread_self(), SCHED_RR, &param));
+
 	SAFE_PFUNC(pthread_getschedparam(pthread_self(), &policy, &param));
 	if (policy != SCHED_RR || param.sched_priority != HIGH_PRIORITY) {
 		printf("Error: the policy or priority not correct\n");

@@ -104,8 +103,7 @@ void *low_prio_thread(void *tmp)
 	int policy;
 
 	(void) tmp;
-	param.sched_priority = LOW_PRIORITY;
-	SAFE_PFUNC(pthread_setschedparam(pthread_self(), SCHED_RR, &param));
+
 	SAFE_PFUNC(pthread_getschedparam(pthread_self(), &policy, &param));
 	if (policy != SCHED_RR || param.sched_priority != LOW_PRIORITY) {
 		printf("Error: the policy or priority not correct\n");

@@ -133,6 +131,7 @@ int main()
 
 	/* Create the higher priority */
 	SAFE_PFUNC(pthread_attr_init(&high_attr));
+	SAFE_PFUNC(pthread_attr_setinheritsched(&high_attr, PTHREAD_EXPLICIT_SCHED));
 	SAFE_PFUNC(pthread_attr_setschedpolicy(&high_attr, SCHED_RR));
 	param.sched_priority = HIGH_PRIORITY;
 	SAFE_PFUNC(pthread_attr_setschedparam(&high_attr, &param));

@@ -140,6 +139,7 @@ int main()
 
 	/* Create the low priority thread */
 	SAFE_PFUNC(pthread_attr_init(&low_attr));
+	SAFE_PFUNC(pthread_attr_setinheritsched(&low_attr, PTHREAD_EXPLICIT_SCHED));
 	SAFE_PFUNC(pthread_attr_setschedpolicy(&low_attr, SCHED_RR));
 	param.sched_priority = LOW_PRIORITY;
 	SAFE_PFUNC(pthread_attr_setschedparam(&low_attr, &param));
testcases/open_posix_testsuite/functional/threads/schedule/1-2.c +4 -5
index f7b0bd89e081..c85460e7cf89 100644
@@ -61,8 +61,7 @@ void *hi_prio_thread(void *tmp)
 	int policy;
 
 	(void) tmp;
-	param.sched_priority = HIGH_PRIORITY;
-	SAFE_PFUNC(pthread_setschedparam(pthread_self(), SCHED_RR, &param));
+
 	SAFE_PFUNC(pthread_getschedparam(pthread_self(), &policy, &param));
 	if (policy != SCHED_RR) {
 		printf(ERROR_PREFIX "The policy is not correct\n");

@@ -92,8 +91,7 @@ void *low_prio_thread(void *tmp)
 	int policy;
 
 	(void) tmp;
-	param.sched_priority = LOW_PRIORITY;
-	SAFE_PFUNC(pthread_setschedparam(pthread_self(), SCHED_RR, &param));
+
 	SAFE_PFUNC(pthread_getschedparam(pthread_self(), &policy, &param));
 	if (policy != SCHED_RR) {
 		printf(ERROR_PREFIX "Policy not correct\n");

@@ -137,6 +135,7 @@ int main()
 
 	/* create the higher priority */
 	SAFE_PFUNC(pthread_attr_init(&high_attr));
+	SAFE_PFUNC(pthread_attr_setinheritsched(&high_attr, PTHREAD_EXPLICIT_SCHED));
 	SAFE_PFUNC(pthread_attr_setschedpolicy(&high_attr, SCHED_RR));
 	param.sched_priority = HIGH_PRIORITY;
 	SAFE_PFUNC(pthread_attr_setschedparam(&high_attr, &param));

@@ -144,6 +143,7 @@ int main()
 
 	/* Create the low priority thread */
 	SAFE_PFUNC(pthread_attr_init(&low_attr));
+	SAFE_PFUNC(pthread_attr_setinheritsched(&low_attr, PTHREAD_EXPLICIT_SCHED));
 	SAFE_PFUNC(pthread_attr_setschedpolicy(&low_attr, SCHED_RR));
 	param.sched_priority = LOW_PRIORITY;
 	SAFE_PFUNC(pthread_attr_setschedparam(&low_attr, &param));

@@ -156,7 +156,6 @@ int main()
 	}
 
 	SAFE_PFUNC(pthread_mutex_lock(&cond_mutex));
-
 	alarm(2);
 	SAFE_PFUNC(pthread_cond_wait(&cond, &cond_mutex));
 	SAFE_PFUNC(pthread_mutex_unlock(&cond_mutex));
Signature · 1 line
1.8.3.1
author
#7 lore raw

PATCH 5/5 open_posix: condvar/schedule: mask SIGALRM in SCHED_OTHER threads

In reply to Jan Stancek

Kernel can deliver signal to any thread (usually it's the main). If these tests run on single CPU and signal gets delivered to main thread with SCHED_OTHER scheduling class then signal handler won't run until RT thread occupying CPU completes. Which happens after 5 seconds and test fails.

Mask SIGLARM from SCHED_OTHER (main) thread. Kernel will deliver signal to any of RT threads that can handle it.

Before patch:

  # time taskset -c 0 ./condvar_pthread_cond_wait_1.run-test
  Test FAILED: high priority was not woken up
  real    0m5.003s
  user    0m4.046s
  sys     0m0.007s

After patch:

  # time taskset -c 0 ./condvar_pthread_cond_wait_1.run-test
  Test PASSED
  real    0m2.002s
  user    0m1.967s
  sys     0m0.004s
Signed-off-by Jan Stancek <[email protected]>
---
 .../functional/threads/condvar/pthread_cond_wait_1.c         |  4 ++++
 .../functional/threads/condvar/pthread_cond_wait_2.c         |  4 ++++
 .../open_posix_testsuite/functional/threads/schedule/1-1.c   |  4 ++++
 testcases/open_posix_testsuite/include/safe_helpers.h        | 12 ++++++++++++
 4 files changed, 24 insertions(+)
testcases/open_posix_testsuite/functional/threads/condvar/pthread_cond_wait_1.c +4 -0
index a674efc406ef..9c92bf79eb7a 100644
@@ -61,6 +61,7 @@ void *hi_prio_thread(void *tmp)
 	int policy;
 
 	(void) tmp;
+	safe_adjust_sigmask(SIGALRM, 0);
 
 	SAFE_PFUNC(pthread_getschedparam(pthread_self(), &policy, &param));
 	if ((policy != POLICY) || (param.sched_priority != HIGH_PRIORITY)) {

@@ -97,6 +98,7 @@ void *low_prio_thread(void *tmp)
 	int policy;
 
 	(void) tmp;
+	safe_adjust_sigmask(SIGALRM, 0);
 
 	SAFE_PFUNC(pthread_getschedparam(pthread_self(), &policy, &param));
 	if ((policy != POLICY) || (param.sched_priority != LOW_PRIORITY)) {

@@ -120,6 +122,8 @@ int main()
 	pthread_attr_t high_attr, low_attr;
 	struct sched_param param;
 
+	safe_adjust_sigmask(SIGALRM, 1);
+
 	/* Create the higher priority thread */
 	SAFE_PFUNC(pthread_attr_init(&high_attr));
 	SAFE_PFUNC(pthread_attr_setinheritsched(&high_attr, PTHREAD_EXPLICIT_SCHED));
testcases/open_posix_testsuite/functional/threads/condvar/pthread_cond_wait_2.c +4 -0
index 398b8db5448e..009e51eeb41b 100644
@@ -61,6 +61,7 @@ void *hi_prio_thread(void *tmp)
 	int policy;
 
 	(void) tmp;
+	safe_adjust_sigmask(SIGALRM, 0);
 
 	SAFE_PFUNC(pthread_getschedparam(pthread_self(), &policy, &param));
 	if ((policy != POLICY) || (param.sched_priority != HIGH_PRIORITY)) {

@@ -97,6 +98,7 @@ void *low_prio_thread(void *tmp)
 	int policy;
 
 	(void) tmp;
+	safe_adjust_sigmask(SIGALRM, 0);
 
 	SAFE_PFUNC(pthread_getschedparam(pthread_self(), &policy, &param));
 	if ((policy != POLICY) || (param.sched_priority != LOW_PRIORITY)) {

@@ -120,6 +122,8 @@ int main()
 	pthread_attr_t high_attr, low_attr;
 	struct sched_param param;
 
+	safe_adjust_sigmask(SIGALRM, 1);
+
 	/* Create the higher priority thread */
 	SAFE_PFUNC(pthread_attr_init(&high_attr));
 	SAFE_PFUNC(pthread_attr_setinheritsched(&high_attr, PTHREAD_EXPLICIT_SCHED));
testcases/open_posix_testsuite/functional/threads/schedule/1-1.c +4 -0
index c65e127bdf5b..00ce577f5c16 100644
@@ -70,6 +70,7 @@ void *hi_prio_thread(void *tmp)
 	void *previous_signal;
 
 	(void) tmp;
+	safe_adjust_sigmask(SIGALRM, 0);
 
 	SAFE_PFUNC(pthread_getschedparam(pthread_self(), &policy, &param));
 	if (policy != SCHED_RR || param.sched_priority != HIGH_PRIORITY) {

@@ -103,6 +104,7 @@ void *low_prio_thread(void *tmp)
 	int policy;
 
 	(void) tmp;
+	safe_adjust_sigmask(SIGALRM, 0);
 
 	SAFE_PFUNC(pthread_getschedparam(pthread_self(), &policy, &param));
 	if (policy != SCHED_RR || param.sched_priority != LOW_PRIORITY) {

@@ -127,6 +129,8 @@ int main()
 	pthread_attr_t low_attr, high_attr;
 	struct sched_param param;
 
+	safe_adjust_sigmask(SIGALRM, 1);
+
 	SAFE_PFUNC(pthread_barrier_init(&barrier, NULL, 2));
 
 	/* Create the higher priority */
testcases/open_posix_testsuite/include/safe_helpers.h +12 -0
index 655e158f65d0..78ca208ad23e 100644
@@ -14,4 +14,16 @@ do {\
 	} \
 } while (0)
 
+void safe_adjust_sigmask(int signal, int block)
+{
+	sigset_t set;
+
+	SAFE_PFUNC(pthread_sigmask(SIG_SETMASK, NULL, &set));
+	if (block)
+		sigaddset(&set, signal);
+	else
+		sigdelset(&set, signal);
+	SAFE_PFUNC(pthread_sigmask(SIG_SETMASK, &set, NULL));
+}
+
 #endif
Signature · 1 line
1.8.3.1
#8 lore raw

PATCH 5/5 open_posix: condvar/schedule: mask SIGALRM in SCHED_OTHER threads

Hi!

Quoted 8 lines

Kernel can deliver signal to any thread (usually it's the main). If these tests run on single CPU and signal gets delivered to main thread with SCHED_OTHER scheduling class then signal handler won't run until RT thread occupying CPU completes. Which happens after 5 seconds and test fails.

Mask SIGLARM from SCHED_OTHER (main) thread. Kernel will deliver signal to any of RT threads that can handle it.

POSIX explicitly states:

It is not safe to use the pthread_cond_signal() function in a signal handler that is invoked asynchronously.

I guess that the test could be changed to:

* Lock one high priority thread on cond variable
* Run one low priority thread that does sleep(1) then calls pthread_cond_signal()
* Run one bussy loop low priority thread

That way we would avoid the signal handlers entirely. What do you think?

Signature · 2 lines
Cyril Hrubis
[email protected]
author
#9 lore raw

PATCH 5/5 open_posix: condvar/schedule: mask SIGALRM in SCHED_OTHER threads

----- Original Message -----

Quoted 13 lines

From: "Cyril Hrubis" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: [email protected]
Sent: Monday, 22 February, 2016 5:09:03 PM
Subject: Re: [LTP] [PATCH 5/5] open_posix: condvar/schedule: mask SIGALRM in SCHED_OTHER threads

Hi!

Quoted 6 lines

Kernel can deliver signal to any thread (usually it's the main). If these tests run on single CPU and signal gets delivered to main thread with SCHED_OTHER scheduling class then signal handler won't run until RT thread occupying CPU completes. Which happens after 5 seconds and test fails.

Quoted 7 lines
Quoted 2 lines

Mask SIGLARM from SCHED_OTHER (main) thread. Kernel will deliver signal to any of RT threads that can handle it.

POSIX explicitly states:

It is not safe to use the pthread_cond_signal() function in a signal handler that is invoked asynchronously.

Good catch.

Quoted 10 lines

I guess that the test could be changed to:

* Lock one high priority thread on cond variable
* Run one low priority thread that does sleep(1) then calls pthread_cond_signal()
* Run one bussy loop low priority thread

That way we would avoid the signal handlers entirely. What do you think?

Would it be enough to:
- call set_affinity_single() to have it all run on single CPU
- spawn high prio thread and wait for it to block on condition
- spawn low prio thread and singal condition, then busy loop for couple seconds

And we can be also sure that high prio thread preempted low prio thread regardless of number of CPUs on system.

Also, any objections if we split the series? If I fix 1/5 and skip 5/5 for now, those 4 could go in now and then next series would be series that removes signal handlers from all.

Regards,
Jan

Quoted 5 lines

--
Cyril Hrubis
[email protected]

#10 lore raw

PATCH 5/5 open_posix: condvar/schedule: mask SIGALRM in SCHED_OTHER threads

Hi!

Quoted 9 lines
Quoted 8 lines

I guess that the test could be changed to:

* Lock one high priority thread on cond variable
* Run one low priority thread that does sleep(1) then calls pthread_cond_signal()
* Run one bussy loop low priority thread

That way we would avoid the signal handlers entirely. What do you think?

Quoted 8 lines

Would it be enough to:
- call set_affinity_single() to have it all run on single CPU
- spawn high prio thread and wait for it to block on condition
- spawn low prio thread and singal condition, then busy loop for couple seconds

And we can be also sure that high prio thread preempted low prio thread regardless of number of CPUs on system.

Good idea. I would use affinity on the bussy thread and the high priority thread and let the low priority thread that calls the signal function without any affinity.

That way the condition signaling would be trully asynchronous on SMP system (and the test will work on single CPU as well since SCHED_RR will wake up the signaling thread sooner or later since it has the same priority as the bussy loop one).

Quoted 3 lines

Also, any objections if we split the series? If I fix 1/5 and skip 5/5 for now, those 4 could go in now and then next series would be series that removes signal handlers from all.

Sure. I've acked these patches allready :)

Signature · 2 lines
Cyril Hrubis
[email protected]
author
#11 lore raw

PATCH 5/5 open_posix: condvar/schedule: mask SIGALRM in SCHED_OTHER threads

----- Original Message -----

Quoted 35 lines

From: "Cyril Hrubis" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: [email protected]
Sent: Tuesday, 23 February, 2016 11:00:27 AM
Subject: Re: [LTP] [PATCH 5/5] open_posix: condvar/schedule: mask SIGALRM in SCHED_OTHER threads

Hi!

Quoted 9 lines
Quoted 8 lines

I guess that the test could be changed to:

* Lock one high priority thread on cond variable
* Run one low priority thread that does sleep(1) then calls pthread_cond_signal()
* Run one bussy loop low priority thread

That way we would avoid the signal handlers entirely. What do you think?

Quoted 9 lines

Would it be enough to:
- call set_affinity_single() to have it all run on single CPU
- spawn high prio thread and wait for it to block on condition
- spawn low prio thread and singal condition, then busy loop for couple seconds

And we can be also sure that high prio thread preempted low prio thread regardless of number of CPUs on system.

Good idea. I would use affinity on the bussy thread and the high priority thread and let the low priority thread that calls the signal function without any affinity.

That way the condition signaling would be trully asynchronous on SMP system (and the test will work on single CPU as well since SCHED_RR will wake up the signaling thread sooner or later since it has the same priority as the bussy loop one).

Quoted 5 lines
Quoted 3 lines

Also, any objections if we split the series? If I fix 1/5 and skip 5/5 for now, those 4 could go in now and then next series would be series that removes signal handlers from all.

Sure. I've acked these patches allready :)

1/5 through 4/5 are pushed. I posted new version of 5/5 separately.

Regards,
Jan

Quoted 5 lines

--
Cyril Hrubis
[email protected]

#12 lore raw

PATCH 1/5 open_posix: add SAFE_PFUNC macro

In reply to Jan Stancek

Hi!

testcases/open_posix_testsuite/include/safe_helpers.h line 2

Quoted 2 lines
testcases/open_posix_testsuite/include/safe_helpers.h new file +2 -0
+#ifndef __SAFE_MACROS_H__
+#define __SAFE_MACROS_H__

The file lacks GNU header and the guards should be SAFE_HELPERS rather than SAFE_MACROS.

Quoted 1 line

+#include <string.h>

Technically we should incude stdio.h for fflush() and printf() as well.

testcases/open_posix_testsuite/include/safe_helpers.h line 17

Quoted 12 lines
testcases/open_posix_testsuite/include/safe_helpers.h new file +12 -0
+#define SAFE_PFUNC(op) \
+do {\
+	int ret = (op); \
+	if (ret != 0) { \
+		printf("Test %s unresolved: got %i (%s) on line %i\n  %s\n", \
+			__FILE__, ret, strerror(ret), __LINE__, #op); \
+		fflush(stdout); \
+		exit(PTS_UNRESOLVED); \
+	} \
+} while (0)
+
+#endif

Otherwise it looks good.

Signature · 2 lines
Cyril Hrubis
[email protected]