* [PATCH 1/4] Properly test for return-values from read() and write()
@ 2015-10-06 9:08 Henrik Austad
2015-10-06 9:08 ` [PATCH 2/4] signaltest: properly test return value from system() Henrik Austad
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Henrik Austad @ 2015-10-06 9:08 UTC (permalink / raw)
To: linux-rt-users; +Cc: Henrik Austad, Clark Williams, John Kacur
It is conceivable that read() and write() will fail, and when they do,
it is useful to catch these errors. This also has the benefit of
removing gcc-warnings like :
warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result]
write(tracing_enabled, "0", 1);
^
warning: ignoring return value of ‘ftruncate’, declared with attribute warn_unused_result [-Wunused-result]
ftruncate(shmem, totalsize);
^
warning: ignoring return value of ‘read’, declared with attribute warn_unused_result [-Wunused-result]
read(path, timestamp, sizeof(timestamp));
^
Signed-off-by: Henrik Austad <haustad@cisco.com>
Cc: Clark Williams <williams@redhat.com>
Cc: John Kacur <jkacur@redhat.com>
---
src/backfire/sendme.c | 8 ++++++--
src/cyclictest/cyclictest.c | 9 ++++++---
src/pmqtest/pmqtest.c | 3 ++-
src/ptsematest/ptsematest.c | 3 ++-
src/rt-migrate-test/rt-migrate-test.c | 5 +++--
src/sigwaittest/sigwaittest.c | 8 ++++++--
src/svsematest/svsematest.c | 9 +++++++--
7 files changed, 32 insertions(+), 13 deletions(-)
diff --git a/src/backfire/sendme.c b/src/backfire/sendme.c
index c1854d9..34504b7 100644
--- a/src/backfire/sendme.c
+++ b/src/backfire/sendme.c
@@ -256,9 +256,13 @@ int main(int argc, char *argv[])
ts.tv_nsec = (interval % USEC_PER_SEC) * 1000;
gettimeofday(&before, NULL);
- write(path, sigtest, strlen(sigtest));
+ if (write(path, sigtest, strlen(sigtest)) < 0)
+ fprintf(stderr, "Could not write sigtest to backfire-path\n");
+
while (after.tv_sec == 0);
- read(path, timestamp, sizeof(timestamp));
+ if (read(path, timestamp, sizeof(timestamp)) <= 0)
+ fprintf(stderr, "Trouble reading file backfire-path\n");
+
if (sscanf(timestamp, "%lu,%lu\n", &sendtime.tv_sec,
&sendtime.tv_usec) != 2)
break;
diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 58f1983..22932e9 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -450,7 +450,8 @@ static void tracemark(char *fmt, ...)
va_start(ap, fmt);
len = vsnprintf(tracebuf, TRACEBUFSIZ, fmt, ap);
va_end(ap);
- write(tracemark_fd, tracebuf, len);
+ if (write(tracemark_fd, tracebuf, len) < 0)
+ err_msg_n(errno, "WARN: could not write to tracebuf");
}
@@ -463,7 +464,8 @@ static void tracing(int on)
case KV_26_LT24: prctl(0, 1); break;
case KV_26_33:
case KV_30:
- write(trace_fd, "1", 1);
+ if (write(trace_fd, "1", 1) < 0)
+ err_msg_n(errno, "Could not set trace_fd");
break;
default: break;
}
@@ -473,7 +475,8 @@ static void tracing(int on)
case KV_26_LT24: prctl(0, 0); break;
case KV_26_33:
case KV_30:
- write(trace_fd, "0", 1);
+ if (write(trace_fd, "0", 1) < 0)
+ err_msg_n(errno, "Could not set trace_fd");
break;
default: break;
}
diff --git a/src/pmqtest/pmqtest.c b/src/pmqtest/pmqtest.c
index 75d5ee8..78eaa9c 100644
--- a/src/pmqtest/pmqtest.c
+++ b/src/pmqtest/pmqtest.c
@@ -210,7 +210,8 @@ void *pmqthread(void *param)
int tracing_enabled =
open(tracing_enabled_file, O_WRONLY);
if (tracing_enabled >= 0) {
- write(tracing_enabled, "0", 1);
+ if (write(tracing_enabled, "0", 1) < 0)
+ err_msg_n(errno, "WARN: could write to tracing_enabled");
close(tracing_enabled);
} else
snprintf(par->error, sizeof(par->error),
diff --git a/src/ptsematest/ptsematest.c b/src/ptsematest/ptsematest.c
index a31c745..f777b38 100644
--- a/src/ptsematest/ptsematest.c
+++ b/src/ptsematest/ptsematest.c
@@ -137,7 +137,8 @@ void *semathread(void *param)
int tracing_enabled =
open(tracing_enabled_file, O_WRONLY);
if (tracing_enabled >= 0) {
- write(tracing_enabled, "0", 1);
+ if (write(tracing_enabled, "0", 1) < 0)
+ err_msg_n(errno, "WARN: Could not enable tracing.");
close(tracing_enabled);
} else
snprintf(par->error, sizeof(par->error),
diff --git a/src/rt-migrate-test/rt-migrate-test.c b/src/rt-migrate-test/rt-migrate-test.c
index d7b68dd..352a331 100644
--- a/src/rt-migrate-test/rt-migrate-test.c
+++ b/src/rt-migrate-test/rt-migrate-test.c
@@ -44,7 +44,7 @@
#include <errno.h>
#include <sched.h>
#include <pthread.h>
-
+#include "error.h"
#define gettid() syscall(__NR_gettid)
int nr_tasks;
@@ -87,7 +87,8 @@ static void ftrace_write(const char *fmt, ...)
n = vsnprintf(buff, BUFSIZ, fmt, ap);
va_end(ap);
- write(mark_fd, buff, n);
+ if (write(mark_fd, buff, n) < 0)
+ err_msg_n(errno, "WARN: Could not write to trace_marker.");
}
#define nano2sec(nan) (nan / 1000000000ULL)
diff --git a/src/sigwaittest/sigwaittest.c b/src/sigwaittest/sigwaittest.c
index 91fcdaa..abeaa35 100644
--- a/src/sigwaittest/sigwaittest.c
+++ b/src/sigwaittest/sigwaittest.c
@@ -36,6 +36,7 @@
#include <sys/time.h>
#include <linux/unistd.h>
#include <utmpx.h>
+#include "error.h"
#include "rt-utils.h"
#include "rt-get_cpu.h"
@@ -185,7 +186,8 @@ void *semathread(void *param)
int tracing_enabled =
open(tracing_enabled_file, O_WRONLY);
if (tracing_enabled >= 0) {
- write(tracing_enabled, "0", 1);
+ if (write(tracing_enabled, "0", 1) < 0)
+ err_msg_n(errno, "WARN: Could not disable tracing.");
close(tracing_enabled);
} else
snprintf(par->error, sizeof(par->error),
@@ -378,7 +380,9 @@ int main(int argc, char *argv[])
fprintf(stderr, "Could not create shared memory\n");
return 1;
}
- ftruncate(shmem, totalsize);
+ if (ftruncate(shmem, totalsize) < 0)
+ err_msg_n(errno, "WARN: Could not truncate file to %d bytes.", totalsize);
+
param = mmap(0, totalsize, PROT_READ|PROT_WRITE, MAP_SHARED,
shmem, 0);
if (param == MAP_FAILED) {
diff --git a/src/svsematest/svsematest.c b/src/svsematest/svsematest.c
index eeb8285..9d4d2b9 100644
--- a/src/svsematest/svsematest.c
+++ b/src/svsematest/svsematest.c
@@ -191,7 +191,8 @@ void *semathread(void *param)
int tracing_enabled =
open(tracing_enabled_file, O_WRONLY);
if (tracing_enabled >= 0) {
- write(tracing_enabled, "0", 1);
+ if (write(tracing_enabled, "0", 1))
+ err_msg_n(errno, "WARN: Could not write to tracing_enabled!");
close(tracing_enabled);
} else
snprintf(par->error, sizeof(par->error),
@@ -431,7 +432,11 @@ int main(int argc, char *argv[])
fprintf(stderr, "Could not create shared memory\n");
return 1;
}
- ftruncate(shmem, totalsize);
+ if (ftruncate(shmem, totalsize) < 0) {
+ fprintf(stderr, "Could not truncate file to specified size (%d)\n", totalsize);
+ return 1;
+
+ }
param = mmap(0, totalsize, PROT_READ|PROT_WRITE, MAP_SHARED,
shmem, 0);
if (param == MAP_FAILED) {
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/4] signaltest: properly test return value from system()
2015-10-06 9:08 [PATCH 1/4] Properly test for return-values from read() and write() Henrik Austad
@ 2015-10-06 9:08 ` Henrik Austad
2015-10-07 10:44 ` John Kacur
2015-10-06 9:08 ` [PATCH 3/4] Android: clean up the bypass ifdeffery Henrik Austad
2015-10-06 9:08 ` [PATCH 4/4] cyclictest: move redefine of CPUSET back to uclib Henrik Austad
2 siblings, 1 reply; 9+ messages in thread
From: Henrik Austad @ 2015-10-06 9:08 UTC (permalink / raw)
To: linux-rt-users; +Cc: Henrik Austad, Clark Williams, John Kacur
signaltest writes to several /proc/sys/kernel attributes without testing
if the system() call succeeds or not. We expect the writes from echo to
suceed (i.e. yield '0' in return), so we test explicitly for that.
Signed-off-by: Henrik Austad <haustad@cisco.com>
Cc: Clark Williams <williams@redhat.com>
Cc: John Kacur <jkacur@redhat.com>
---
src/signaltest/signaltest.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/src/signaltest/signaltest.c b/src/signaltest/signaltest.c
index b80969b..8074b68 100644
--- a/src/signaltest/signaltest.c
+++ b/src/signaltest/signaltest.c
@@ -65,6 +65,11 @@ struct thread_stat {
int threadstarted;
int tid;
};
+#define SYSTEM_W(x) \
+ if (system((x)) != 0) { \
+ fprintf(stderr, "Trouble running %s\n", (x)); \
+ return NULL; \
+ } \
static int shutdown;
static int tracelimit = 0;
@@ -102,16 +107,16 @@ void *signalthread(void *param)
int first = 1;
if (tracelimit) {
- system("echo 1 > /proc/sys/kernel/trace_all_cpus");
- system("echo 1 > /proc/sys/kernel/trace_enabled");
- system("echo 1 > /proc/sys/kernel/trace_freerunning");
- system("echo 0 > /proc/sys/kernel/trace_print_at_crash");
- system("echo 1 > /proc/sys/kernel/trace_user_triggered");
- system("echo -1 > /proc/sys/kernel/trace_user_trigger_irq");
- system("echo 0 > /proc/sys/kernel/trace_verbose");
- system("echo 0 > /proc/sys/kernel/preempt_thresh");
- system("echo 0 > /proc/sys/kernel/wakeup_timing");
- system("echo 0 > /proc/sys/kernel/preempt_max_latency");
+ SYSTEM_W("echo 1 > /proc/sys/kernel/trace_all_cpus");
+ SYSTEM_W("echo 1 > /proc/sys/kernel/trace_enabled");
+ SYSTEM_W("echo 1 > /proc/sys/kernel/trace_freerunning");
+ SYSTEM_W("echo 0 > /proc/sys/kernel/trace_print_at_crash");
+ SYSTEM_W("echo 1 > /proc/sys/kernel/trace_user_triggered");
+ SYSTEM_W("echo -1 > /proc/sys/kernel/trace_user_trigger_irq");
+ SYSTEM_W("echo 0 > /proc/sys/kernel/trace_verbose");
+ SYSTEM_W("echo 0 > /proc/sys/kernel/preempt_thresh");
+ SYSTEM_W("echo 0 > /proc/sys/kernel/wakeup_timing");
+ SYSTEM_W("echo 0 > /proc/sys/kernel/preempt_max_latency");
}
stat->tid = gettid();
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/4] signaltest: properly test return value from system()
2015-10-06 9:08 ` [PATCH 2/4] signaltest: properly test return value from system() Henrik Austad
@ 2015-10-07 10:44 ` John Kacur
0 siblings, 0 replies; 9+ messages in thread
From: John Kacur @ 2015-10-07 10:44 UTC (permalink / raw)
To: Henrik Austad; +Cc: linux-rt-users, Clark Williams
On Tue, 6 Oct 2015, Henrik Austad wrote:
> signaltest writes to several /proc/sys/kernel attributes without testing
> if the system() call succeeds or not. We expect the writes from echo to
> suceed (i.e. yield '0' in return), so we test explicitly for that.
>
> Signed-off-by: Henrik Austad <haustad@cisco.com>
> Cc: Clark Williams <williams@redhat.com>
> Cc: John Kacur <jkacur@redhat.com>
> ---
> src/signaltest/signaltest.c | 25 +++++++++++++++----------
> 1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/src/signaltest/signaltest.c b/src/signaltest/signaltest.c
> index b80969b..8074b68 100644
> --- a/src/signaltest/signaltest.c
> +++ b/src/signaltest/signaltest.c
> @@ -65,6 +65,11 @@ struct thread_stat {
> int threadstarted;
> int tid;
> };
> +#define SYSTEM_W(x) \
> + if (system((x)) != 0) { \
> + fprintf(stderr, "Trouble running %s\n", (x)); \
> + return NULL; \
> + } \
>
> static int shutdown;
> static int tracelimit = 0;
> @@ -102,16 +107,16 @@ void *signalthread(void *param)
> int first = 1;
>
> if (tracelimit) {
> - system("echo 1 > /proc/sys/kernel/trace_all_cpus");
> - system("echo 1 > /proc/sys/kernel/trace_enabled");
> - system("echo 1 > /proc/sys/kernel/trace_freerunning");
> - system("echo 0 > /proc/sys/kernel/trace_print_at_crash");
> - system("echo 1 > /proc/sys/kernel/trace_user_triggered");
> - system("echo -1 > /proc/sys/kernel/trace_user_trigger_irq");
> - system("echo 0 > /proc/sys/kernel/trace_verbose");
> - system("echo 0 > /proc/sys/kernel/preempt_thresh");
> - system("echo 0 > /proc/sys/kernel/wakeup_timing");
> - system("echo 0 > /proc/sys/kernel/preempt_max_latency");
> + SYSTEM_W("echo 1 > /proc/sys/kernel/trace_all_cpus");
> + SYSTEM_W("echo 1 > /proc/sys/kernel/trace_enabled");
> + SYSTEM_W("echo 1 > /proc/sys/kernel/trace_freerunning");
> + SYSTEM_W("echo 0 > /proc/sys/kernel/trace_print_at_crash");
> + SYSTEM_W("echo 1 > /proc/sys/kernel/trace_user_triggered");
> + SYSTEM_W("echo -1 > /proc/sys/kernel/trace_user_trigger_irq");
> + SYSTEM_W("echo 0 > /proc/sys/kernel/trace_verbose");
> + SYSTEM_W("echo 0 > /proc/sys/kernel/preempt_thresh");
> + SYSTEM_W("echo 0 > /proc/sys/kernel/wakeup_timing");
> + SYSTEM_W("echo 0 > /proc/sys/kernel/preempt_max_latency");
> }
>
> stat->tid = gettid();
> --
> 1.9.1
>
I really don't like patches like this that just exist to shut-up the
compiler's overzealous warnings. Are you actually having a problem with
this program that you need to debug? If you are, is the message "Trouble
running echo num to some /proc" file really useful? Have you carefully
read the system man page to see what kind of errors can be returned and
why? I'm going to pass on this one for now.
Sorry
John
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/4] Android: clean up the bypass ifdeffery
2015-10-06 9:08 [PATCH 1/4] Properly test for return-values from read() and write() Henrik Austad
2015-10-06 9:08 ` [PATCH 2/4] signaltest: properly test return value from system() Henrik Austad
@ 2015-10-06 9:08 ` Henrik Austad
2015-10-07 10:34 ` John Kacur
2015-10-06 9:08 ` [PATCH 4/4] cyclictest: move redefine of CPUSET back to uclib Henrik Austad
2 siblings, 1 reply; 9+ messages in thread
From: Henrik Austad @ 2015-10-06 9:08 UTC (permalink / raw)
To: linux-rt-users; +Cc: Henrik Austad, Clark Williams, John Kacur
88af643971b9 (android: adjust target for android) introduced some really
ugly ifdefs to avoid calling into pthread_barrier_wait and
pthread_barrier_init.
This patch attempts to coalesce this into a single place and let the
compiler handle the linking so that cyclictest.c is untouched by evil
ifdefs.
Compiled and tested on:
- x86_64 (v3.13 kernel)
- tilegx (v3.10 kernel)
- arm64 android (v3.10 kernel)
Signed-off-by: Henrik Austad <haustad@cisco.com>
Cc: Clark Williams <williams@redhat.com>
Cc: John Kacur <jkacur@redhat.com>
---
Makefile | 2 +-
include/bionic.h | 32 ++++++++++++++++++++++++++++++++
src/arch/bionic/Makefile | 4 +---
src/cyclictest/cyclictest.c | 25 +++----------------------
4 files changed, 37 insertions(+), 26 deletions(-)
create mode 100644 include/bionic.h
diff --git a/Makefile b/Makefile
index 2d20ea4..24ef8fe 100644
--- a/Makefile
+++ b/Makefile
@@ -22,7 +22,7 @@ bindir ?= $(prefix)/bin
mandir ?= $(prefix)/share/man
srcdir ?= $(prefix)/src
-CFLAGS ?= -Wall -Wno-nonnull
+CFLAGS ?= -Wall -Wno-nonnull -Iinclude/
CPPFLAGS += -D_GNU_SOURCE -Isrc/include
LDFLAGS ?=
diff --git a/include/bionic.h b/include/bionic.h
new file mode 100644
index 0000000..8526299
--- /dev/null
+++ b/include/bionic.h
@@ -0,0 +1,32 @@
+#ifndef BIONIC_H
+#define BIONIC_H
+
+/* we do not have pthread barriers, add as small an emt */
+#ifdef NO_PTHREAD_BARRIER
+#warning Program is being compiled without PTHREAD_BARRIER support, some options may behave erratic.
+typedef int pthread_barrier_t;
+typedef int pthread_barrierattr_t;
+
+#ifndef PTHREAD_BARRIER_SERIAL_THREAD
+#define PTHREAD_BARRIER_SERIAL_THREAD 0
+#endif
+
+static inline int pthread_barrier_wait(pthread_barrier_t *barrier)
+{
+ return PTHREAD_BARRIER_SERIAL_THREAD;
+}
+
+static inline int pthread_barrier_destroy(pthread_barrier_t *barrier)
+{
+ return 0;
+}
+static inline int pthread_barrier_init(pthread_barrier_t * barrier,
+ const pthread_barrierattr_t * attr,
+ unsigned count)
+{
+ return 0;
+}
+
+#endif /* NO_PTHREAD_BARRIER */
+
+#endif /* BIONIC_H */
diff --git a/src/arch/bionic/Makefile b/src/arch/bionic/Makefile
index 410d2c9..8e169f0 100644
--- a/src/arch/bionic/Makefile
+++ b/src/arch/bionic/Makefile
@@ -19,7 +19,5 @@ ifeq (android,$(ostype))
# and link properly:
# - cyclictest
# - hackbench
-# - hwlatdetect
- sources := cyclictest.c hackbench.c hwlatdetect.c
+ sources := cyclictest.c hackbench.c
endif
-
diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 22932e9..b52cabd 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -40,6 +40,8 @@
#include "rt-utils.h"
+#include <bionic.h>
+
#define DEFAULT_INTERVAL 1000
#define DEFAULT_DISTANCE 500
@@ -203,14 +205,12 @@ static pthread_mutex_t break_thread_id_lock = PTHREAD_MUTEX_INITIALIZER;
static pid_t break_thread_id = 0;
static uint64_t break_thread_value = 0;
-#ifndef NO_PTHREAD_BARRIER
static int aligned = 0;
static int secaligned = 0;
static int offset = 0;
static pthread_barrier_t align_barr;
static pthread_barrier_t globalt_barr;
static struct timespec globalt;
-#endif
/* Backup of kernel variables that we modify */
static struct kvars {
@@ -815,7 +815,6 @@ static void *timerthread(void *param)
fatal("timerthread%d: failed to set priority to %d\n", par->cpu, par->prio);
/* Get current time */
-#ifndef NO_PTHREAD_BARRIER
if(aligned || secaligned){
pthread_barrier_wait(&globalt_barr);
if (par->tnum == 0) {
@@ -841,7 +840,6 @@ static void *timerthread(void *param)
}
}
else
-#endif
clock_gettime(par->clock, &now);
next = now;
@@ -1048,9 +1046,7 @@ static void display_help(int error)
"-a [NUM] --affinity run thread #N on processor #N, if possible\n"
" with NUM pin all threads to the processor NUM\n"
#endif
-#ifndef NO_PTHREAD_BARRIER
"-A USEC --aligned=USEC align thread wakeups to a specific offset\n"
-#endif
"-b USEC --breaktrace=USEC send break trace command when latency > USEC\n"
"-B --preemptirqs both preempt and irqsoff tracing (used with -b)\n"
"-c CLOCK --clock=CLOCK select clock\n"
@@ -1091,9 +1087,7 @@ static void display_help(int error)
"-R --resolution check clock resolution, calling clock_gettime() many\n"
" times. list of clock_gettime() values will be\n"
" reported with -X\n"
-#ifndef NO_PTHREAD_BARRIER
" --secaligned [USEC] align thread wakeups to the next full second,\n"
-#endif
" and apply the optional offset\n"
"-s --system use sys_nanosleep and sys_setitimer\n"
"-S --smp Standard SMP testing: options -a -t -n and\n"
@@ -1243,10 +1237,7 @@ enum option_values {
OPT_PRIOSPREAD, OPT_RELATIVE, OPT_RESOLUTION, OPT_SYSTEM, OPT_SMP, OPT_THREADS,
OPT_TRACER, OPT_UNBUFFERED, OPT_NUMA, OPT_VERBOSE, OPT_WAKEUP, OPT_WAKEUPRT,
OPT_DBGCYCLIC, OPT_POLICY, OPT_HELP, OPT_NUMOPTS,
-#ifndef NO_PTHREAD_BARRIER
- OPT_ALIGNED, OPT_SECALIGNED,
-#endif
- OPT_LAPTOP,
+ OPT_ALIGNED, OPT_SECALIGNED, OPT_LAPTOP,
};
/* Process commandline options */
@@ -1264,9 +1255,7 @@ static void process_options (int argc, char *argv[], int max_cpus)
static struct option long_options[] = {
{"affinity", optional_argument, NULL, OPT_AFFINITY},
{"notrace", no_argument, NULL, OPT_NOTRACE },
-#ifndef NO_PTHREAD_BARRIER
{"aligned", optional_argument, NULL, OPT_ALIGNED },
-#endif
{"breaktrace", required_argument, NULL, OPT_BREAKTRACE },
{"preemptirqs", no_argument, NULL, OPT_PREEMPTIRQ },
{"clock", required_argument, NULL, OPT_CLOCK },
@@ -1296,9 +1285,7 @@ static void process_options (int argc, char *argv[], int max_cpus)
{"priospread", no_argument, NULL, OPT_PRIOSPREAD },
{"relative", no_argument, NULL, OPT_RELATIVE },
{"resolution", no_argument, NULL, OPT_RESOLUTION },
-#ifndef NO_PTHREAD_BARRIER
{"secaligned", optional_argument, NULL, OPT_SECALIGNED },
-#endif
{"system", no_argument, NULL, OPT_SYSTEM },
{"smp", no_argument, NULL, OPT_SMP },
{"threads", optional_argument, NULL, OPT_THREADS },
@@ -1333,7 +1320,6 @@ static void process_options (int argc, char *argv[], int max_cpus)
setaffinity = AFFINITY_USEALL;
}
break;
-#ifndef NO_PTHREAD_BARRIER
case 'A':
case OPT_ALIGNED:
aligned=1;
@@ -1344,7 +1330,6 @@ static void process_options (int argc, char *argv[], int max_cpus)
else
offset = 0;
break;
-#endif
case 'b':
case OPT_BREAKTRACE:
tracelimit = atoi(optarg); break;
@@ -1445,7 +1430,6 @@ static void process_options (int argc, char *argv[], int max_cpus)
case OPT_RESOLUTION:
check_clock_resolution = 1; break;
case 's':
-#ifndef NO_PTHREAD_BARRIER
case OPT_SECALIGNED:
secaligned = 1;
if (optarg != NULL)
@@ -1455,7 +1439,6 @@ static void process_options (int argc, char *argv[], int max_cpus)
else
offset = 0;
break;
-#endif
case OPT_SYSTEM:
use_system = MODE_SYS_OFFSET; break;
case 'S':
@@ -1592,7 +1575,6 @@ static void process_options (int argc, char *argv[], int max_cpus)
if (num_threads < 1)
error = 1;
-#ifndef NO_PTHREAD_BARRIER
if (aligned && secaligned)
error = 1;
@@ -1600,7 +1582,6 @@ static void process_options (int argc, char *argv[], int max_cpus)
pthread_barrier_init(&globalt_barr, NULL, num_threads);
pthread_barrier_init(&align_barr, NULL, num_threads);
}
-#endif
if (error) {
if (affinity_mask)
rt_bitmask_free(affinity_mask);
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/4] Android: clean up the bypass ifdeffery
2015-10-06 9:08 ` [PATCH 3/4] Android: clean up the bypass ifdeffery Henrik Austad
@ 2015-10-07 10:34 ` John Kacur
2015-10-07 11:14 ` Henrik Austad
0 siblings, 1 reply; 9+ messages in thread
From: John Kacur @ 2015-10-07 10:34 UTC (permalink / raw)
To: Henrik Austad; +Cc: linux-rt-users, Clark Williams
On Tue, 6 Oct 2015, Henrik Austad wrote:
> 88af643971b9 (android: adjust target for android) introduced some really
> ugly ifdefs to avoid calling into pthread_barrier_wait and
> pthread_barrier_init.
>
> This patch attempts to coalesce this into a single place and let the
> compiler handle the linking so that cyclictest.c is untouched by evil
> ifdefs.
>
> Compiled and tested on:
> - x86_64 (v3.13 kernel)
> - tilegx (v3.10 kernel)
> - arm64 android (v3.10 kernel)
>
> Signed-off-by: Henrik Austad <haustad@cisco.com>
> Cc: Clark Williams <williams@redhat.com>
> Cc: John Kacur <jkacur@redhat.com>
> ---
> Makefile | 2 +-
> include/bionic.h | 32 ++++++++++++++++++++++++++++++++
> src/arch/bionic/Makefile | 4 +---
> src/cyclictest/cyclictest.c | 25 +++----------------------
> 4 files changed, 37 insertions(+), 26 deletions(-)
> create mode 100644 include/bionic.h
>
> diff --git a/Makefile b/Makefile
> index 2d20ea4..24ef8fe 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -22,7 +22,7 @@ bindir ?= $(prefix)/bin
> mandir ?= $(prefix)/share/man
> srcdir ?= $(prefix)/src
>
> -CFLAGS ?= -Wall -Wno-nonnull
> +CFLAGS ?= -Wall -Wno-nonnull -Iinclude/
> CPPFLAGS += -D_GNU_SOURCE -Isrc/include
We already have an include dir. Why aren't you using it?
If you think it would be cleaner to create your own include dir, you at
least have to put it some where in the src dir
> LDFLAGS ?=
>
> diff --git a/include/bionic.h b/include/bionic.h
> new file mode 100644
> index 0000000..8526299
> --- /dev/null
> +++ b/include/bionic.h
> @@ -0,0 +1,32 @@
> +#ifndef BIONIC_H
> +#define BIONIC_H
> +
> +/* we do not have pthread barriers, add as small an emt */
> +#ifdef NO_PTHREAD_BARRIER
> +#warning Program is being compiled without PTHREAD_BARRIER support, some options may behave erratic.
> +typedef int pthread_barrier_t;
> +typedef int pthread_barrierattr_t;
> +
> +#ifndef PTHREAD_BARRIER_SERIAL_THREAD
> +#define PTHREAD_BARRIER_SERIAL_THREAD 0
> +#endif
> +
> +static inline int pthread_barrier_wait(pthread_barrier_t *barrier)
> +{
> + return PTHREAD_BARRIER_SERIAL_THREAD;
> +}
> +
> +static inline int pthread_barrier_destroy(pthread_barrier_t *barrier)
> +{
> + return 0;
> +}
> +static inline int pthread_barrier_init(pthread_barrier_t * barrier,
> + const pthread_barrierattr_t * attr,
> + unsigned count)
> +{
> + return 0;
> +}
> +
> +#endif /* NO_PTHREAD_BARRIER */
> +
> +#endif /* BIONIC_H */
> diff --git a/src/arch/bionic/Makefile b/src/arch/bionic/Makefile
> index 410d2c9..8e169f0 100644
> --- a/src/arch/bionic/Makefile
> +++ b/src/arch/bionic/Makefile
> @@ -19,7 +19,5 @@ ifeq (android,$(ostype))
> # and link properly:
> # - cyclictest
> # - hackbench
> -# - hwlatdetect
> - sources := cyclictest.c hackbench.c hwlatdetect.c
> + sources := cyclictest.c hackbench.c
> endif
> -
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 22932e9..b52cabd 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -40,6 +40,8 @@
>
> #include "rt-utils.h"
>
> +#include <bionic.h>
> +
> #define DEFAULT_INTERVAL 1000
> #define DEFAULT_DISTANCE 500
>
> @@ -203,14 +205,12 @@ static pthread_mutex_t break_thread_id_lock = PTHREAD_MUTEX_INITIALIZER;
> static pid_t break_thread_id = 0;
> static uint64_t break_thread_value = 0;
>
> -#ifndef NO_PTHREAD_BARRIER
> static int aligned = 0;
> static int secaligned = 0;
> static int offset = 0;
> static pthread_barrier_t align_barr;
> static pthread_barrier_t globalt_barr;
> static struct timespec globalt;
> -#endif
>
> /* Backup of kernel variables that we modify */
> static struct kvars {
> @@ -815,7 +815,6 @@ static void *timerthread(void *param)
> fatal("timerthread%d: failed to set priority to %d\n", par->cpu, par->prio);
>
> /* Get current time */
> -#ifndef NO_PTHREAD_BARRIER
> if(aligned || secaligned){
> pthread_barrier_wait(&globalt_barr);
> if (par->tnum == 0) {
> @@ -841,7 +840,6 @@ static void *timerthread(void *param)
> }
> }
> else
> -#endif
> clock_gettime(par->clock, &now);
>
> next = now;
> @@ -1048,9 +1046,7 @@ static void display_help(int error)
> "-a [NUM] --affinity run thread #N on processor #N, if possible\n"
> " with NUM pin all threads to the processor NUM\n"
> #endif
> -#ifndef NO_PTHREAD_BARRIER
> "-A USEC --aligned=USEC align thread wakeups to a specific offset\n"
> -#endif
> "-b USEC --breaktrace=USEC send break trace command when latency > USEC\n"
> "-B --preemptirqs both preempt and irqsoff tracing (used with -b)\n"
> "-c CLOCK --clock=CLOCK select clock\n"
> @@ -1091,9 +1087,7 @@ static void display_help(int error)
> "-R --resolution check clock resolution, calling clock_gettime() many\n"
> " times. list of clock_gettime() values will be\n"
> " reported with -X\n"
> -#ifndef NO_PTHREAD_BARRIER
> " --secaligned [USEC] align thread wakeups to the next full second,\n"
> -#endif
> " and apply the optional offset\n"
> "-s --system use sys_nanosleep and sys_setitimer\n"
> "-S --smp Standard SMP testing: options -a -t -n and\n"
> @@ -1243,10 +1237,7 @@ enum option_values {
> OPT_PRIOSPREAD, OPT_RELATIVE, OPT_RESOLUTION, OPT_SYSTEM, OPT_SMP, OPT_THREADS,
> OPT_TRACER, OPT_UNBUFFERED, OPT_NUMA, OPT_VERBOSE, OPT_WAKEUP, OPT_WAKEUPRT,
> OPT_DBGCYCLIC, OPT_POLICY, OPT_HELP, OPT_NUMOPTS,
> -#ifndef NO_PTHREAD_BARRIER
> - OPT_ALIGNED, OPT_SECALIGNED,
> -#endif
> - OPT_LAPTOP,
> + OPT_ALIGNED, OPT_SECALIGNED, OPT_LAPTOP,
> };
>
> /* Process commandline options */
> @@ -1264,9 +1255,7 @@ static void process_options (int argc, char *argv[], int max_cpus)
> static struct option long_options[] = {
> {"affinity", optional_argument, NULL, OPT_AFFINITY},
> {"notrace", no_argument, NULL, OPT_NOTRACE },
> -#ifndef NO_PTHREAD_BARRIER
> {"aligned", optional_argument, NULL, OPT_ALIGNED },
> -#endif
> {"breaktrace", required_argument, NULL, OPT_BREAKTRACE },
> {"preemptirqs", no_argument, NULL, OPT_PREEMPTIRQ },
> {"clock", required_argument, NULL, OPT_CLOCK },
> @@ -1296,9 +1285,7 @@ static void process_options (int argc, char *argv[], int max_cpus)
> {"priospread", no_argument, NULL, OPT_PRIOSPREAD },
> {"relative", no_argument, NULL, OPT_RELATIVE },
> {"resolution", no_argument, NULL, OPT_RESOLUTION },
> -#ifndef NO_PTHREAD_BARRIER
> {"secaligned", optional_argument, NULL, OPT_SECALIGNED },
> -#endif
> {"system", no_argument, NULL, OPT_SYSTEM },
> {"smp", no_argument, NULL, OPT_SMP },
> {"threads", optional_argument, NULL, OPT_THREADS },
> @@ -1333,7 +1320,6 @@ static void process_options (int argc, char *argv[], int max_cpus)
> setaffinity = AFFINITY_USEALL;
> }
> break;
> -#ifndef NO_PTHREAD_BARRIER
> case 'A':
> case OPT_ALIGNED:
> aligned=1;
> @@ -1344,7 +1330,6 @@ static void process_options (int argc, char *argv[], int max_cpus)
> else
> offset = 0;
> break;
> -#endif
> case 'b':
> case OPT_BREAKTRACE:
> tracelimit = atoi(optarg); break;
> @@ -1445,7 +1430,6 @@ static void process_options (int argc, char *argv[], int max_cpus)
> case OPT_RESOLUTION:
> check_clock_resolution = 1; break;
> case 's':
> -#ifndef NO_PTHREAD_BARRIER
> case OPT_SECALIGNED:
> secaligned = 1;
> if (optarg != NULL)
> @@ -1455,7 +1439,6 @@ static void process_options (int argc, char *argv[], int max_cpus)
> else
> offset = 0;
> break;
> -#endif
> case OPT_SYSTEM:
> use_system = MODE_SYS_OFFSET; break;
> case 'S':
> @@ -1592,7 +1575,6 @@ static void process_options (int argc, char *argv[], int max_cpus)
> if (num_threads < 1)
> error = 1;
>
> -#ifndef NO_PTHREAD_BARRIER
> if (aligned && secaligned)
> error = 1;
>
> @@ -1600,7 +1582,6 @@ static void process_options (int argc, char *argv[], int max_cpus)
> pthread_barrier_init(&globalt_barr, NULL, num_threads);
> pthread_barrier_init(&align_barr, NULL, num_threads);
> }
> -#endif
> if (error) {
> if (affinity_mask)
> rt_bitmask_free(affinity_mask);
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/4] Android: clean up the bypass ifdeffery
2015-10-07 10:34 ` John Kacur
@ 2015-10-07 11:14 ` Henrik Austad
2015-10-07 11:55 ` John Kacur
0 siblings, 1 reply; 9+ messages in thread
From: Henrik Austad @ 2015-10-07 11:14 UTC (permalink / raw)
To: John Kacur; +Cc: linux-rt-users, Clark Williams
On Wed, Oct 07, 2015 at 12:34:11PM +0200, John Kacur wrote:
>
>
> On Tue, 6 Oct 2015, Henrik Austad wrote:
>
> > 88af643971b9 (android: adjust target for android) introduced some really
> > ugly ifdefs to avoid calling into pthread_barrier_wait and
> > pthread_barrier_init.
> >
> > This patch attempts to coalesce this into a single place and let the
> > compiler handle the linking so that cyclictest.c is untouched by evil
> > ifdefs.
> >
> > Compiled and tested on:
> > - x86_64 (v3.13 kernel)
> > - tilegx (v3.10 kernel)
> > - arm64 android (v3.10 kernel)
> >
> > Signed-off-by: Henrik Austad <haustad@cisco.com>
> > Cc: Clark Williams <williams@redhat.com>
> > Cc: John Kacur <jkacur@redhat.com>
> > ---
> > Makefile | 2 +-
> > include/bionic.h | 32 ++++++++++++++++++++++++++++++++
> > src/arch/bionic/Makefile | 4 +---
> > src/cyclictest/cyclictest.c | 25 +++----------------------
> > 4 files changed, 37 insertions(+), 26 deletions(-)
> > create mode 100644 include/bionic.h
> >
> > diff --git a/Makefile b/Makefile
> > index 2d20ea4..24ef8fe 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -22,7 +22,7 @@ bindir ?= $(prefix)/bin
> > mandir ?= $(prefix)/share/man
> > srcdir ?= $(prefix)/src
> >
> > -CFLAGS ?= -Wall -Wno-nonnull
> > +CFLAGS ?= -Wall -Wno-nonnull -Iinclude/
> > CPPFLAGS += -D_GNU_SOURCE -Isrc/include
>
> We already have an include dir. Why aren't you using it?
> If you think it would be cleaner to create your own include dir, you at
> least have to put it some where in the src dir
You're right!
moving it to src/include/ now, do you want a new patch right away, or shall
I wait for v0.96-devel before submitting?
--
Henrik Austad
TIPBU Eng
Cisco Systems Norway
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/4] Android: clean up the bypass ifdeffery
2015-10-07 11:14 ` Henrik Austad
@ 2015-10-07 11:55 ` John Kacur
0 siblings, 0 replies; 9+ messages in thread
From: John Kacur @ 2015-10-07 11:55 UTC (permalink / raw)
To: Henrik Austad; +Cc: linux-rt-users, Clark Williams
On Wed, 7 Oct 2015, Henrik Austad wrote:
> On Wed, Oct 07, 2015 at 12:34:11PM +0200, John Kacur wrote:
> >
> >
> > On Tue, 6 Oct 2015, Henrik Austad wrote:
> >
> > > 88af643971b9 (android: adjust target for android) introduced some really
> > > ugly ifdefs to avoid calling into pthread_barrier_wait and
> > > pthread_barrier_init.
> > >
> > > This patch attempts to coalesce this into a single place and let the
> > > compiler handle the linking so that cyclictest.c is untouched by evil
> > > ifdefs.
> > >
> > > Compiled and tested on:
> > > - x86_64 (v3.13 kernel)
> > > - tilegx (v3.10 kernel)
> > > - arm64 android (v3.10 kernel)
> > >
> > > Signed-off-by: Henrik Austad <haustad@cisco.com>
> > > Cc: Clark Williams <williams@redhat.com>
> > > Cc: John Kacur <jkacur@redhat.com>
> > > ---
> > > Makefile | 2 +-
> > > include/bionic.h | 32 ++++++++++++++++++++++++++++++++
> > > src/arch/bionic/Makefile | 4 +---
> > > src/cyclictest/cyclictest.c | 25 +++----------------------
> > > 4 files changed, 37 insertions(+), 26 deletions(-)
> > > create mode 100644 include/bionic.h
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 2d20ea4..24ef8fe 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -22,7 +22,7 @@ bindir ?= $(prefix)/bin
> > > mandir ?= $(prefix)/share/man
> > > srcdir ?= $(prefix)/src
> > >
> > > -CFLAGS ?= -Wall -Wno-nonnull
> > > +CFLAGS ?= -Wall -Wno-nonnull -Iinclude/
> > > CPPFLAGS += -D_GNU_SOURCE -Isrc/include
> >
> > We already have an include dir. Why aren't you using it?
> > If you think it would be cleaner to create your own include dir, you at
> > least have to put it some where in the src dir
>
> You're right!
>
> moving it to src/include/ now, do you want a new patch right away, or shall
> I wait for v0.96-devel before submitting?
>
>
> --
> Henrik Austad
> TIPBU Eng
Let's take our time and get it right, so wait for v0.96-devel
Thanks
John
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/4] cyclictest: move redefine of CPUSET back to uclib
2015-10-06 9:08 [PATCH 1/4] Properly test for return-values from read() and write() Henrik Austad
2015-10-06 9:08 ` [PATCH 2/4] signaltest: properly test return value from system() Henrik Austad
2015-10-06 9:08 ` [PATCH 3/4] Android: clean up the bypass ifdeffery Henrik Austad
@ 2015-10-06 9:08 ` Henrik Austad
2015-10-07 10:28 ` John Kacur
2 siblings, 1 reply; 9+ messages in thread
From: Henrik Austad @ 2015-10-06 9:08 UTC (permalink / raw)
To: linux-rt-users; +Cc: Henrik Austad, Clark Williams, John Kacur
This was mistakenly included in the #ifdef in 88af643971b9 (android:
adjust target for android). Moved back into the correct #ifdef-entry.
Cc: Clark Williams <williams@redhat.com>
Cc: John Kacur <jkacur@redhat.com>
Signed-off-by: Henrik Austad <haustad@cisco.com>
---
src/cyclictest/cyclictest.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index b52cabd..87a2d7e 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -80,6 +80,11 @@ int sched_setaffinity (__pid_t __pid, size_t __cpusetsize,
return -EINVAL;
}
+#undef CPU_SET
+#undef CPU_ZERO
+#define CPU_SET(cpu, cpusetp)
+#define CPU_ZERO(cpusetp)
+
#endif
#ifdef NO_PTHREAD_SETAFFINITY
@@ -89,11 +94,6 @@ static inline int pthread_setaffinity_np(pthread_t thread, size_t cpusetsize,
return sched_setaffinity(0, cpusetsize, cpuset);
}
-#undef CPU_SET
-#undef CPU_ZERO
-#define CPU_SET(cpu, cpusetp)
-#define CPU_ZERO(cpusetp)
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 4/4] cyclictest: move redefine of CPUSET back to uclib
2015-10-06 9:08 ` [PATCH 4/4] cyclictest: move redefine of CPUSET back to uclib Henrik Austad
@ 2015-10-07 10:28 ` John Kacur
0 siblings, 0 replies; 9+ messages in thread
From: John Kacur @ 2015-10-07 10:28 UTC (permalink / raw)
To: Henrik Austad; +Cc: linux-rt-users, Clark Williams
On Tue, 6 Oct 2015, Henrik Austad wrote:
> This was mistakenly included in the #ifdef in 88af643971b9 (android:
> adjust target for android). Moved back into the correct #ifdef-entry.
>
> Cc: Clark Williams <williams@redhat.com>
> Cc: John Kacur <jkacur@redhat.com>
> Signed-off-by: Henrik Austad <haustad@cisco.com>
> ---
> src/cyclictest/cyclictest.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index b52cabd..87a2d7e 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -80,6 +80,11 @@ int sched_setaffinity (__pid_t __pid, size_t __cpusetsize,
> return -EINVAL;
> }
>
> +#undef CPU_SET
> +#undef CPU_ZERO
> +#define CPU_SET(cpu, cpusetp)
> +#define CPU_ZERO(cpusetp)
> +
> #endif
>
> #ifdef NO_PTHREAD_SETAFFINITY
> @@ -89,11 +94,6 @@ static inline int pthread_setaffinity_np(pthread_t thread, size_t cpusetsize,
> return sched_setaffinity(0, cpusetsize, cpuset);
> }
>
> -#undef CPU_SET
> -#undef CPU_ZERO
> -#define CPU_SET(cpu, cpusetp)
> -#define CPU_ZERO(cpusetp)
> -
> #else
> extern int clock_nanosleep(clockid_t __clock_id, int __flags,
> __const struct timespec *__req,
> --
> 1.9.1
>
> --
Signed-off-by: John Kacur <jkacur@redhat.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-10-07 11:55 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-06 9:08 [PATCH 1/4] Properly test for return-values from read() and write() Henrik Austad
2015-10-06 9:08 ` [PATCH 2/4] signaltest: properly test return value from system() Henrik Austad
2015-10-07 10:44 ` John Kacur
2015-10-06 9:08 ` [PATCH 3/4] Android: clean up the bypass ifdeffery Henrik Austad
2015-10-07 10:34 ` John Kacur
2015-10-07 11:14 ` Henrik Austad
2015-10-07 11:55 ` John Kacur
2015-10-06 9:08 ` [PATCH 4/4] cyclictest: move redefine of CPUSET back to uclib Henrik Austad
2015-10-07 10:28 ` John Kacur
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).