2012-08-23 15:37:47

by Bernhard Rosenkraenzer

[permalink] [raw]
Subject: [PATCHv2 1/1] perf: Port to Android

commit 4dc79eed16e3bb03b3cf92fcc6127e107e7537aa
Author: Bernhard Rosenkraenzer <[email protected]>
Date: Sat Jun 23 06:18:05 2012 +0200

perf: Port to Android

Adapt perf to deal with some missing functions in Bionic etc.

Change-Id: I0cda2aad3edba26e1be3aebc9475a229ea9e8356
Signed-off-by: Bernhard Rosenkraenzer <[email protected]>

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 92271d3..d15cdae 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -117,7 +117,7 @@ ifndef PERF_DEBUG
endif

CFLAGS = -fno-omit-frame-pointer -ggdb3 -Wall -Wextra -std=gnu99
$(CFLAGS_WERROR) $(CFLAGS_OPTIMIZE) -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS)
$(EXTRA_CFLAGS)
-EXTLIBS = -lpthread -lrt -lelf -lm
+EXTLIBS = -lpthread -lelf -lm
ALL_CFLAGS = $(CFLAGS) -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -
D_GNU_SOURCE
ALL_LDFLAGS = $(LDFLAGS)
STRIP ?= strip
@@ -474,12 +474,23 @@ FLAGS_LIBELF=$(ALL_CFLAGS) $(ALL_LDFLAGS) $(EXTLIBS)
ifneq ($(call try-cc,$(SOURCE_LIBELF),$(FLAGS_LIBELF)),y)
FLAGS_GLIBC=$(ALL_CFLAGS) $(ALL_LDFLAGS)
ifneq ($(call try-cc,$(SOURCE_GLIBC),$(FLAGS_GLIBC)),y)
- msg := $(error No gnu/libc-version.h found, please install glibc-
dev[el]/glibc-static);
+ ifeq ($(call try-cc,$(SOURCE_BIONIC),$(FLAGS_GLIBC)),y)
+ # Found Bionic instead of glibc...
+ # That works too, but needs a bit of special treatment
+ BASIC_CFLAGS += -DANDROID -include compat-android.h
+ ANDROID := 1
+ else
+ msg := $(error No gnu/libc-version.h found, please install glibc-
dev[el]/glibc-static);
+ endif
else
msg := $(error No libelf.h/libelf found, please install libelf-
dev/elfutils-libelf-devel);
endif
endif

+ifneq ($(ANDROID),1)
+EXTLIBS += -lrt
+endif
+
ifneq ($(call try-cc,$(SOURCE_ELF_MMAP),$(FLAGS_COMMON)),y)
BASIC_CFLAGS += -DLIBELF_NO_MMAP
endif
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index be4e1ee..3a1d0cc 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -27,10 +27,43 @@
#include "util/cpumap.h"
#include "util/thread_map.h"

+#include <stdlib.h>
#include <unistd.h>
#include <sched.h>
#include <sys/mman.h>

+#ifdef ANDROID
+/* While stdlib.h has a prototype for it,
+ Bionic doesn't actually implement on_exit() */
+#ifndef ATEXIT_MAX
+#define ATEXIT_MAX 32
+#endif
+static int __on_exit_count = 0;
+typedef void (*on_exit_func_t)(int, void*);
+static on_exit_func_t __on_exit_funcs[ATEXIT_MAX];
+static void *__on_exit_args[ATEXIT_MAX];
+static int __exitcode = 0;
+static void __handle_on_exit_funcs();
+static int on_exit(on_exit_func_t function, void *arg);
+#define exit(x) (exit)(__exitcode = (x))
+
+static int on_exit(on_exit_func_t function, void *arg) {
+ if(__on_exit_count == ATEXIT_MAX)
+ return ENOMEM;
+ else if(__on_exit_count == 0)
+ atexit(__handle_on_exit_funcs);
+ __on_exit_funcs[__on_exit_count] = function;
+ __on_exit_args[__on_exit_count++] = arg;
+ return 0;
+}
+
+static void __handle_on_exit_funcs() {
+ for(int i=0; i<__on_exit_count; i++) {
+ __on_exit_funcs[i](__exitcode, __on_exit_args[i]);
+ }
+}
+#endif
+
enum write_mode_t {
WRITE_FORCE,
WRITE_APPEND
diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 223ffdc..6dbd2ee 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -469,10 +469,17 @@ static int test__basic_mmap(void)
.watermark = 0,
};
cpu_set_t cpu_set;
+#ifndef ANDROID
const char *syscall_names[] = { "getsid", "getppid", "getpgrp",
"getpgid", };
pid_t (*syscalls[])(void) = { (void *)getsid, getppid, getpgrp,
(void*)getpgid };
+#else
+ /* No getsid() on Android */
+ const char *syscall_names[] = { "getppid", "getpgrp",
+ "getpgid", };
+ pid_t (*syscalls[])(void) = { getppid, getpgrp, (void*)getpgid };
+#endif
#define nsyscalls ARRAY_SIZE(syscall_names)
int ids[nsyscalls];
unsigned int nr_events[nsyscalls],
diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
index b382bd5..1521a275 100644
--- a/tools/perf/builtin.h
+++ b/tools/perf/builtin.h
@@ -1,6 +1,7 @@
#ifndef BUILTIN_H
#define BUILTIN_H

+#include "compat-android.h"
#include "util/util.h"
#include "util/strbuf.h"

diff --git a/tools/perf/compat-android.h b/tools/perf/compat-android.h
new file mode 100644
index 0000000..9a33f49
--- /dev/null
+++ b/tools/perf/compat-android.h
@@ -0,0 +1,96 @@
+/* Android compatibility header
+ * Provides missing bits in Bionic on Android, ignored
+ * on regular Linux.
+ *
+ * Written by [email protected]
+ *
+ * Released into the public domain. Do with this file
+ * whatever you want.
+ */
+#ifdef ANDROID
+/* Bionic has its own idea about ALIGN, and kills other definitions.
+ * Done outside the multiple-inclusion wrapper to make sure we
+ * can override Bionic's ALIGN by simply including compat-android.h
+ * again after including Bionic headers.
+ */
+#undef ALIGN
+#undef __ALIGN_MASK
+#define ALIGN(x,a) __ALIGN_MASK(x,(typeof(x))(a)-1)
+#define __ALIGN_MASK(x,mask) (((x)+(mask))&~(mask))
+
+#ifndef _COMPAT_ANDROID_H_
+#define _COMPAT_ANDROID_H_ 1
+#include <stdio.h>
+#include <signal.h>
+#include <asm/page.h> /* for PAGE_SIZE */
+#include <asm/termios.h> /* for winsize */
+
+#ifndef __WORDSIZE
+#include <stdint.h>
+#define __WORDSIZE _BITSIZE
+#endif
+
+#ifndef roundup
+#define roundup(x, y) ((((x)+((y)-1))/(y))*(y))
+#endif
+
+#ifndef __force
+#define __force
+#endif
+
+#ifndef __le32
+#define __le32 uint32_t
+#endif
+
+/* Assorted functions that are missing from Bionic */
+static void psignal(int sig, const char *s)
+{
+ if(sig >= 0 && sig < NSIG) {
+ if(s)
+ fprintf(stderr, "%s: %s\n", s, sys_siglist[sig]);
+ else
+ fprintf(stderr, "%s\n", sys_siglist[sig]);
+ } else {
+ if(s)
+ fprintf(stderr, "%s: invalid signal\n", s);
+ else
+ fputs("invalid signal\n", stderr);
+ }
+}
+
+static ssize_t getline(char **lineptr, size_t *n, FILE *stream)
+{
+ size_t ret = 0;
+
+ if (!lineptr || !n || !stream)
+ return -1;
+
+ if(!*lineptr) {
+ *n = 128;
+ *lineptr = (char*)malloc(*n);
+ if(!*lineptr)
+ return -1;
+ }
+
+ while(!feof(stream) && !ferror(stream)) {
+ int c;
+ if(ret == *n) {
+ *n += 128;
+ *lineptr = (char*)realloc(*lineptr, *n);
+ if(!*lineptr) {
+ *n = 0;
+ return -1;
+ }
+ }
+ c = fgetc(stream);
+ if(c == EOF)
+ break;
+ *lineptr[ret++] = c;
+ if(c == '\n')
+ break;
+ }
+ *lineptr[ret] = 0;
+ return ret;
+}
+#endif
+#endif
diff --git a/tools/perf/config/feature-tests.mak b/tools/perf/config/feature-
tests.mak
index d9084e0..498cd4d 100644
--- a/tools/perf/config/feature-tests.mak
+++ b/tools/perf/config/feature-tests.mak
@@ -43,6 +43,19 @@ int main(void)
}
endef

+define SOURCE_BIONIC
+#include <android/api-level.h>
+
+int main(void)
+{
+#ifndef __ANDROID_API__
+ error out
+#else
+ return 0;
+#endif
+}
+endef
+
define SOURCE_ELF_MMAP
#include <libelf.h>
int main(void)
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index efa5dc8..ead88ea 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -6,6 +6,7 @@
#include "symbol.h"
#include <linux/list.h>
#include <linux/rbtree.h>
+#include <pthread.h>

struct objdump_line {
struct list_head node;
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 2a6f33c..867415a 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -6,6 +6,7 @@
#include "strlist.h"
#include "thread.h"
#include "thread_map.h"
+#include "../compat-android.h"

static const char *perf_event__names[] = {
[0] = "TOTAL",
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 1b19728..2b0554b 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -6,6 +6,7 @@

#include "../perf.h"
#include "map.h"
+#include "../compat-android.h"

/*
* PERF_SAMPLE_IP | PERF_SAMPLE_TID | *
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 0f99f39..77c5ced 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -70,15 +70,19 @@
#include <sys/socket.h>
#include <sys/ioctl.h>
#include <sys/select.h>
+#ifndef ANDROID
#include <netinet/in.h>
#include <netinet/tcp.h>
#include <arpa/inet.h>
+#endif
#include <netdb.h>
#include <pwd.h>
#include <inttypes.h>
#include "../../../include/linux/magic.h"
#include "types.h"
+#ifndef ANDROID
#include <sys/ttydefaults.h>
+#endif

extern const char *graph_line;
extern const char *graph_dotted_line;


2012-08-23 20:52:06

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCHv2 1/1] perf: Port to Android

Hi Bernard,

(You didn't CC perf maintainers.)

On Thu, Aug 23, 2012 at 6:01 PM, Bernhard Rosenkraenzer
<[email protected]> wrote:
> commit 4dc79eed16e3bb03b3cf92fcc6127e107e7537aa
> Author: Bernhard Rosenkraenzer <[email protected]>
> Date: Sat Jun 23 06:18:05 2012 +0200
>
> perf: Port to Android
>
> Adapt perf to deal with some missing functions in Bionic etc.
>
> Change-Id: I0cda2aad3edba26e1be3aebc9475a229ea9e8356
> Signed-off-by: Bernhard Rosenkraenzer <[email protected]>
>

Your patch changelog format is pretty funky.

> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 92271d3..d15cdae 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile

Whoa! I had no idea Android userspace was f*cked up...

> @@ -117,7 +117,7 @@ ifndef PERF_DEBUG
> endif
>
> CFLAGS = -fno-omit-frame-pointer -ggdb3 -Wall -Wextra -std=gnu99
> $(CFLAGS_WERROR) $(CFLAGS_OPTIMIZE) -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS)
> $(EXTRA_CFLAGS)
> -EXTLIBS = -lpthread -lrt -lelf -lm
> +EXTLIBS = -lpthread -lelf -lm
> ALL_CFLAGS = $(CFLAGS) -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -
> D_GNU_SOURCE
> ALL_LDFLAGS = $(LDFLAGS)
> STRIP ?= strip
> @@ -474,12 +474,23 @@ FLAGS_LIBELF=$(ALL_CFLAGS) $(ALL_LDFLAGS) $(EXTLIBS)
> ifneq ($(call try-cc,$(SOURCE_LIBELF),$(FLAGS_LIBELF)),y)
> FLAGS_GLIBC=$(ALL_CFLAGS) $(ALL_LDFLAGS)
> ifneq ($(call try-cc,$(SOURCE_GLIBC),$(FLAGS_GLIBC)),y)
> - msg := $(error No gnu/libc-version.h found, please install glibc-
> dev[el]/glibc-static);
> + ifeq ($(call try-cc,$(SOURCE_BIONIC),$(FLAGS_GLIBC)),y)
> + # Found Bionic instead of glibc...
> + # That works too, but needs a bit of special treatment
> + BASIC_CFLAGS += -DANDROID -include compat-android.h
> + ANDROID := 1
> + else
> + msg := $(error No gnu/libc-version.h found, please install glibc-
> dev[el]/glibc-static);
> + endif
> else
> msg := $(error No libelf.h/libelf found, please install libelf-
> dev/elfutils-libelf-devel);
> endif
> endif
>
> +ifneq ($(ANDROID),1)
> +EXTLIBS += -lrt
> +endif
> +
> ifneq ($(call try-cc,$(SOURCE_ELF_MMAP),$(FLAGS_COMMON)),y)
> BASIC_CFLAGS += -DLIBELF_NO_MMAP
> endif
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index be4e1ee..3a1d0cc 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -27,10 +27,43 @@
> #include "util/cpumap.h"
> #include "util/thread_map.h"
>
> +#include <stdlib.h>
> #include <unistd.h>
> #include <sched.h>
> #include <sys/mman.h>
>
> +#ifdef ANDROID
> +/* While stdlib.h has a prototype for it,
> + Bionic doesn't actually implement on_exit() */
> +#ifndef ATEXIT_MAX
> +#define ATEXIT_MAX 32
> +#endif
> +static int __on_exit_count = 0;
> +typedef void (*on_exit_func_t)(int, void*);
> +static on_exit_func_t __on_exit_funcs[ATEXIT_MAX];
> +static void *__on_exit_args[ATEXIT_MAX];
> +static int __exitcode = 0;
> +static void __handle_on_exit_funcs();
> +static int on_exit(on_exit_func_t function, void *arg);
> +#define exit(x) (exit)(__exitcode = (x))
> +
> +static int on_exit(on_exit_func_t function, void *arg) {
> + if(__on_exit_count == ATEXIT_MAX)
> + return ENOMEM;
> + else if(__on_exit_count == 0)
> + atexit(__handle_on_exit_funcs);
> + __on_exit_funcs[__on_exit_count] = function;
> + __on_exit_args[__on_exit_count++] = arg;
> + return 0;
> +}
> +
> +static void __handle_on_exit_funcs() {
> + for(int i=0; i<__on_exit_count; i++) {
> + __on_exit_funcs[i](__exitcode, __on_exit_args[i]);
> + }
> +}
> +#endif
> +
> enum write_mode_t {
> WRITE_FORCE,
> WRITE_APPEND
> diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
> index 223ffdc..6dbd2ee 100644
> --- a/tools/perf/builtin-test.c
> +++ b/tools/perf/builtin-test.c
> @@ -469,10 +469,17 @@ static int test__basic_mmap(void)
> .watermark = 0,
> };
> cpu_set_t cpu_set;
> +#ifndef ANDROID
> const char *syscall_names[] = { "getsid", "getppid", "getpgrp",
> "getpgid", };
> pid_t (*syscalls[])(void) = { (void *)getsid, getppid, getpgrp,
> (void*)getpgid };
> +#else
> + /* No getsid() on Android */
> + const char *syscall_names[] = { "getppid", "getpgrp",
> + "getpgid", };
> + pid_t (*syscalls[])(void) = { getppid, getpgrp, (void*)getpgid };
> +#endif
> #define nsyscalls ARRAY_SIZE(syscall_names)
> int ids[nsyscalls];
> unsigned int nr_events[nsyscalls],
> diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
> index b382bd5..1521a275 100644
> --- a/tools/perf/builtin.h
> +++ b/tools/perf/builtin.h
> @@ -1,6 +1,7 @@
> #ifndef BUILTIN_H
> #define BUILTIN_H
>
> +#include "compat-android.h"
> #include "util/util.h"
> #include "util/strbuf.h"
>
> diff --git a/tools/perf/compat-android.h b/tools/perf/compat-android.h
> new file mode 100644
> index 0000000..9a33f49
> --- /dev/null
> +++ b/tools/perf/compat-android.h
> @@ -0,0 +1,96 @@
> +/* Android compatibility header
> + * Provides missing bits in Bionic on Android, ignored
> + * on regular Linux.
> + *
> + * Written by [email protected]
> + *
> + * Released into the public domain. Do with this file
> + * whatever you want.
> + */
> +#ifdef ANDROID
> +/* Bionic has its own idea about ALIGN, and kills other definitions.
> + * Done outside the multiple-inclusion wrapper to make sure we
> + * can override Bionic's ALIGN by simply including compat-android.h
> + * again after including Bionic headers.
> + */
> +#undef ALIGN
> +#undef __ALIGN_MASK
> +#define ALIGN(x,a) __ALIGN_MASK(x,(typeof(x))(a)-1)
> +#define __ALIGN_MASK(x,mask) (((x)+(mask))&~(mask))
> +
> +#ifndef _COMPAT_ANDROID_H_
> +#define _COMPAT_ANDROID_H_ 1
> +#include <stdio.h>
> +#include <signal.h>
> +#include <asm/page.h> /* for PAGE_SIZE */
> +#include <asm/termios.h> /* for winsize */
> +
> +#ifndef __WORDSIZE
> +#include <stdint.h>
> +#define __WORDSIZE _BITSIZE
> +#endif
> +
> +#ifndef roundup
> +#define roundup(x, y) ((((x)+((y)-1))/(y))*(y))
> +#endif
> +
> +#ifndef __force
> +#define __force
> +#endif
> +
> +#ifndef __le32
> +#define __le32 uint32_t
> +#endif
> +
> +/* Assorted functions that are missing from Bionic */
> +static void psignal(int sig, const char *s)
> +{
> + if(sig >= 0 && sig < NSIG) {
> + if(s)
> + fprintf(stderr, "%s: %s\n", s, sys_siglist[sig]);
> + else
> + fprintf(stderr, "%s\n", sys_siglist[sig]);
> + } else {
> + if(s)
> + fprintf(stderr, "%s: invalid signal\n", s);
> + else
> + fputs("invalid signal\n", stderr);
> + }
> +}
> +
> +static ssize_t getline(char **lineptr, size_t *n, FILE *stream)
> +{
> + size_t ret = 0;
> +
> + if (!lineptr || !n || !stream)
> + return -1;
> +
> + if(!*lineptr) {
> + *n = 128;
> + *lineptr = (char*)malloc(*n);
> + if(!*lineptr)
> + return -1;
> + }
> +
> + while(!feof(stream) && !ferror(stream)) {
> + int c;
> + if(ret == *n) {
> + *n += 128;
> + *lineptr = (char*)realloc(*lineptr, *n);
> + if(!*lineptr) {
> + *n = 0;
> + return -1;
> + }
> + }
> + c = fgetc(stream);
> + if(c == EOF)
> + break;
> + *lineptr[ret++] = c;
> + if(c == '\n')
> + break;
> + }
> + *lineptr[ret] = 0;
> + return ret;
> +}
> +#endif
> +#endif
> diff --git a/tools/perf/config/feature-tests.mak b/tools/perf/config/feature-
> tests.mak
> index d9084e0..498cd4d 100644
> --- a/tools/perf/config/feature-tests.mak
> +++ b/tools/perf/config/feature-tests.mak
> @@ -43,6 +43,19 @@ int main(void)
> }
> endef
>
> +define SOURCE_BIONIC
> +#include <android/api-level.h>
> +
> +int main(void)
> +{
> +#ifndef __ANDROID_API__
> + error out
> +#else
> + return 0;
> +#endif
> +}
> +endef
> +
> define SOURCE_ELF_MMAP
> #include <libelf.h>
> int main(void)
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index efa5dc8..ead88ea 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -6,6 +6,7 @@
> #include "symbol.h"
> #include <linux/list.h>
> #include <linux/rbtree.h>
> +#include <pthread.h>
>
> struct objdump_line {
> struct list_head node;
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 2a6f33c..867415a 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -6,6 +6,7 @@
> #include "strlist.h"
> #include "thread.h"
> #include "thread_map.h"
> +#include "../compat-android.h"
>
> static const char *perf_event__names[] = {
> [0] = "TOTAL",
> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> index 1b19728..2b0554b 100644
> --- a/tools/perf/util/event.h
> +++ b/tools/perf/util/event.h
> @@ -6,6 +6,7 @@
>
> #include "../perf.h"
> #include "map.h"
> +#include "../compat-android.h"
>
> /*
> * PERF_SAMPLE_IP | PERF_SAMPLE_TID | *
> diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
> index 0f99f39..77c5ced 100644
> --- a/tools/perf/util/util.h
> +++ b/tools/perf/util/util.h
> @@ -70,15 +70,19 @@
> #include <sys/socket.h>
> #include <sys/ioctl.h>
> #include <sys/select.h>
> +#ifndef ANDROID
> #include <netinet/in.h>
> #include <netinet/tcp.h>
> #include <arpa/inet.h>
> +#endif
> #include <netdb.h>
> #include <pwd.h>
> #include <inttypes.h>
> #include "../../../include/linux/magic.h"
> #include "types.h"
> +#ifndef ANDROID
> #include <sys/ttydefaults.h>
> +#endif
>
> extern const char *graph_line;
> extern const char *graph_dotted_line;
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2012-08-23 21:32:56

by David Ahern

[permalink] [raw]
Subject: Re: [PATCHv2 1/1] perf: Port to Android

On 8/23/12 2:51 PM, Pekka Enberg wrote:
>> +#ifdef ANDROID
>> +/* While stdlib.h has a prototype for it,
>> + Bionic doesn't actually implement on_exit() */
>> +#ifndef ATEXIT_MAX
>> +#define ATEXIT_MAX 32
>> +#endif
>> +static int __on_exit_count = 0;
>> +typedef void (*on_exit_func_t)(int, void*);
>> +static on_exit_func_t __on_exit_funcs[ATEXIT_MAX];
>> +static void *__on_exit_args[ATEXIT_MAX];
>> +static int __exitcode = 0;
>> +static void __handle_on_exit_funcs();
>> +static int on_exit(on_exit_func_t function, void *arg);
>> +#define exit(x) (exit)(__exitcode = (x))
>> +
>> +static int on_exit(on_exit_func_t function, void *arg) {
>> + if(__on_exit_count == ATEXIT_MAX)
>> + return ENOMEM;
>> + else if(__on_exit_count == 0)
>> + atexit(__handle_on_exit_funcs);
>> + __on_exit_funcs[__on_exit_count] = function;
>> + __on_exit_args[__on_exit_count++] = arg;
>> + return 0;
>> +}
>> +
>> +static void __handle_on_exit_funcs() {
>> + for(int i=0; i<__on_exit_count; i++) {
>> + __on_exit_funcs[i](__exitcode, __on_exit_args[i]);
>> + }
>> +}
>> +#endif
>> +

Why not add support for the missing functions (on_exit, getsid, psignal
and getline) to Bionic instead of perf?


>> diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
>> index 0f99f39..77c5ced 100644
>> --- a/tools/perf/util/util.h
>> +++ b/tools/perf/util/util.h
>> @@ -70,15 +70,19 @@
>> #include <sys/socket.h>
>> #include <sys/ioctl.h>
>> #include <sys/select.h>
>> +#ifndef ANDROID
>> #include <netinet/in.h>
>> #include <netinet/tcp.h>
>> #include <arpa/inet.h>
>> +#endif
>> #include <netdb.h>

netinet/*, arpa/inet.h,netdb.h and sys/socket.h can be removed from util.h

David

2012-08-24 04:09:03

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCHv2 1/1] perf: Port to Android

On Thu, Aug 23, 2012 at 03:32:50PM -0600, David Ahern wrote:

> Why not add support for the missing functions (on_exit, getsid,
> psignal and getline) to Bionic instead of perf?

Many vendors need to target existing Android platforms and don't have
the luxury of waiting for code to get through Google review and then
land on devices between 6 months in the future and never. The embedded
world is dysfunctional in a whole bunch of ways.

--
Matthew Garrett | [email protected]

2012-08-24 08:35:57

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCHv2 1/1] perf: Port to Android


* Matthew Garrett <[email protected]> wrote:

> On Thu, Aug 23, 2012 at 03:32:50PM -0600, David Ahern wrote:
>
> > Why not add support for the missing functions (on_exit,
> > getsid, psignal and getline) to Bionic instead of perf?
>
> Many vendors need to target existing Android platforms and
> don't have the luxury of waiting for code to get through
> Google review and then land on devices between 6 months in the
> future and never. The embedded world is dysfunctional in a
> whole bunch of ways.

and the compat changes don't look overly large, so I guess we
could do something like this.

Would be nice to make it more robust in case Bionic does add
these functions? Feature tests and such? perf offering weak
symbols for some of these methods and the linker sorting it out?

Thanks,

Ingo

2012-08-24 09:09:32

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCHv2 1/1] perf: Port to Android

Hi, Bernhard

On Thu, 23 Aug 2012 17:01:52 +0200, Bernhard Rosenkraenzer wrote:
> commit 4dc79eed16e3bb03b3cf92fcc6127e107e7537aa
> Author: Bernhard Rosenkraenzer <[email protected]>
> Date: Sat Jun 23 06:18:05 2012 +0200
>
> perf: Port to Android
>
> Adapt perf to deal with some missing functions in Bionic etc.
>
> Change-Id: I0cda2aad3edba26e1be3aebc9475a229ea9e8356

Please remove this Change-Id line.


> Signed-off-by: Bernhard Rosenkraenzer <[email protected]>
>
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 92271d3..d15cdae 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -117,7 +117,7 @@ ifndef PERF_DEBUG
> endif
>
> CFLAGS = -fno-omit-frame-pointer -ggdb3 -Wall -Wextra -std=gnu99
> $(CFLAGS_WERROR) $(CFLAGS_OPTIMIZE) -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS)
> $(EXTRA_CFLAGS)
> -EXTLIBS = -lpthread -lrt -lelf -lm
> +EXTLIBS = -lpthread -lelf -lm
> ALL_CFLAGS = $(CFLAGS) -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -
> D_GNU_SOURCE

It seems your mail client wraps long line (again).


> ALL_LDFLAGS = $(LDFLAGS)
> STRIP ?= strip
> @@ -474,12 +474,23 @@ FLAGS_LIBELF=$(ALL_CFLAGS) $(ALL_LDFLAGS) $(EXTLIBS)
> ifneq ($(call try-cc,$(SOURCE_LIBELF),$(FLAGS_LIBELF)),y)
> FLAGS_GLIBC=$(ALL_CFLAGS) $(ALL_LDFLAGS)
> ifneq ($(call try-cc,$(SOURCE_GLIBC),$(FLAGS_GLIBC)),y)
> - msg := $(error No gnu/libc-version.h found, please install glibc-
> dev[el]/glibc-static);
> + ifeq ($(call try-cc,$(SOURCE_BIONIC),$(FLAGS_GLIBC)),y)
> + # Found Bionic instead of glibc...
> + # That works too, but needs a bit of special treatment
> + BASIC_CFLAGS += -DANDROID -include compat-android.h

Do we really need to include compat-android.h to every source file?


> + ANDROID := 1
> + else
> + msg := $(error No gnu/libc-version.h found, please install glibc-
> dev[el]/glibc-static);
> + endif
> else
> msg := $(error No libelf.h/libelf found, please install libelf-
> dev/elfutils-libelf-devel);
> endif
> endif
>
> +ifneq ($(ANDROID),1)
> +EXTLIBS += -lrt
> +endif
> +
> ifneq ($(call try-cc,$(SOURCE_ELF_MMAP),$(FLAGS_COMMON)),y)
> BASIC_CFLAGS += -DLIBELF_NO_MMAP
> endif
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index be4e1ee..3a1d0cc 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -27,10 +27,43 @@
> #include "util/cpumap.h"
> #include "util/thread_map.h"
>
> +#include <stdlib.h>
> #include <unistd.h>
> #include <sched.h>
> #include <sys/mman.h>
>
> +#ifdef ANDROID
> +/* While stdlib.h has a prototype for it,
> + Bionic doesn't actually implement on_exit() */
> +#ifndef ATEXIT_MAX
> +#define ATEXIT_MAX 32
> +#endif
> +static int __on_exit_count = 0;
> +typedef void (*on_exit_func_t)(int, void*);
> +static on_exit_func_t __on_exit_funcs[ATEXIT_MAX];
> +static void *__on_exit_args[ATEXIT_MAX];
> +static int __exitcode = 0;
> +static void __handle_on_exit_funcs();
> +static int on_exit(on_exit_func_t function, void *arg);
> +#define exit(x) (exit)(__exitcode = (x))
> +
> +static int on_exit(on_exit_func_t function, void *arg) {
> + if(__on_exit_count == ATEXIT_MAX)
> + return ENOMEM;
> + else if(__on_exit_count == 0)
> + atexit(__handle_on_exit_funcs);
> + __on_exit_funcs[__on_exit_count] = function;
> + __on_exit_args[__on_exit_count++] = arg;
> + return 0;
> +}
> +
> +static void __handle_on_exit_funcs() {
> + for(int i=0; i<__on_exit_count; i++) {
> + __on_exit_funcs[i](__exitcode, __on_exit_args[i]);
> + }
> +}
> +#endif
> +

Putting above in builtin-record.c seems not a good choice. How about
something like util/android.c?


> enum write_mode_t {
> WRITE_FORCE,
> WRITE_APPEND
> diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
> index 223ffdc..6dbd2ee 100644
> --- a/tools/perf/builtin-test.c
> +++ b/tools/perf/builtin-test.c
> @@ -469,10 +469,17 @@ static int test__basic_mmap(void)
> .watermark = 0,
> };
> cpu_set_t cpu_set;
> +#ifndef ANDROID
> const char *syscall_names[] = { "getsid", "getppid", "getpgrp",
> "getpgid", };
> pid_t (*syscalls[])(void) = { (void *)getsid, getppid, getpgrp,
> (void*)getpgid };
> +#else
> + /* No getsid() on Android */
> + const char *syscall_names[] = { "getppid", "getpgrp",
> + "getpgid", };
> + pid_t (*syscalls[])(void) = { getppid, getpgrp, (void*)getpgid };
> +#endif
> #define nsyscalls ARRAY_SIZE(syscall_names)
> int ids[nsyscalls];
> unsigned int nr_events[nsyscalls],
> diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
> index b382bd5..1521a275 100644
> --- a/tools/perf/builtin.h
> +++ b/tools/perf/builtin.h
> @@ -1,6 +1,7 @@
> #ifndef BUILTIN_H
> #define BUILTIN_H
>
> +#include "compat-android.h"
> #include "util/util.h"
> #include "util/strbuf.h"
>
> diff --git a/tools/perf/compat-android.h b/tools/perf/compat-android.h
> new file mode 100644
> index 0000000..9a33f49
> --- /dev/null
> +++ b/tools/perf/compat-android.h

This also can be moved under util/ along with the .c file.


> @@ -0,0 +1,96 @@
> +/* Android compatibility header
> + * Provides missing bits in Bionic on Android, ignored
> + * on regular Linux.
> + *
> + * Written by [email protected]
> + *
> + * Released into the public domain. Do with this file
> + * whatever you want.
> + */
> +#ifdef ANDROID
> +/* Bionic has its own idea about ALIGN, and kills other definitions.
> + * Done outside the multiple-inclusion wrapper to make sure we
> + * can override Bionic's ALIGN by simply including compat-android.h
> + * again after including Bionic headers.
> + */
> +#undef ALIGN
> +#undef __ALIGN_MASK
> +#define ALIGN(x,a) __ALIGN_MASK(x,(typeof(x))(a)-1)
> +#define __ALIGN_MASK(x,mask) (((x)+(mask))&~(mask))
> +
> +#ifndef _COMPAT_ANDROID_H_
> +#define _COMPAT_ANDROID_H_ 1
> +#include <stdio.h>
> +#include <signal.h>
> +#include <asm/page.h> /* for PAGE_SIZE */
> +#include <asm/termios.h> /* for winsize */
> +
> +#ifndef __WORDSIZE
> +#include <stdint.h>
> +#define __WORDSIZE _BITSIZE
> +#endif
> +
> +#ifndef roundup
> +#define roundup(x, y) ((((x)+((y)-1))/(y))*(y))
> +#endif
> +
> +#ifndef __force
> +#define __force
> +#endif
> +
> +#ifndef __le32
> +#define __le32 uint32_t
> +#endif
> +
> +/* Assorted functions that are missing from Bionic */
> +static void psignal(int sig, const char *s)
> +{
> + if(sig >= 0 && sig < NSIG) {
> + if(s)
> + fprintf(stderr, "%s: %s\n", s, sys_siglist[sig]);
> + else
> + fprintf(stderr, "%s\n", sys_siglist[sig]);
> + } else {
> + if(s)
> + fprintf(stderr, "%s: invalid signal\n", s);
> + else
> + fputs("invalid signal\n", stderr);
> + }
> +}
> +
> +static ssize_t getline(char **lineptr, size_t *n, FILE *stream)
> +{
> + size_t ret = 0;
> +
> + if (!lineptr || !n || !stream)
> + return -1;
> +
> + if(!*lineptr) {
> + *n = 128;
> + *lineptr = (char*)malloc(*n);
> + if(!*lineptr)
> + return -1;
> + }
> +
> + while(!feof(stream) && !ferror(stream)) {
> + int c;
> + if(ret == *n) {
> + *n += 128;
> + *lineptr = (char*)realloc(*lineptr, *n);
> + if(!*lineptr) {
> + *n = 0;
> + return -1;
> + }
> + }
> + c = fgetc(stream);
> + if(c == EOF)
> + break;
> + *lineptr[ret++] = c;
> + if(c == '\n')
> + break;
> + }
> + *lineptr[ret] = 0;
> + return ret;
> +}

And above two functions can be moved too somehow.


> +#endif
> +#endif
> diff --git a/tools/perf/config/feature-tests.mak b/tools/perf/config/feature-
> tests.mak
> index d9084e0..498cd4d 100644
> --- a/tools/perf/config/feature-tests.mak
> +++ b/tools/perf/config/feature-tests.mak
> @@ -43,6 +43,19 @@ int main(void)
> }
> endef
>
> +define SOURCE_BIONIC
> +#include <android/api-level.h>
> +
> +int main(void)
> +{
> +#ifndef __ANDROID_API__
> + error out
> +#else
> + return 0;
> +#endif
> +}
> +endef
> +
> define SOURCE_ELF_MMAP
> #include <libelf.h>
> int main(void)
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index efa5dc8..ead88ea 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -6,6 +6,7 @@
> #include "symbol.h"
> #include <linux/list.h>
> #include <linux/rbtree.h>
> +#include <pthread.h>
>
> struct objdump_line {
> struct list_head node;
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 2a6f33c..867415a 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -6,6 +6,7 @@
> #include "strlist.h"
> #include "thread.h"
> #include "thread_map.h"
> +#include "../compat-android.h"

Didn't we already include the header on command line? Also I'm curious
whether it can be built with O=$(DIR) option.

Btw, can you (or anyone) tell me the current status of TLS support in
bionic?

Thanks,
Namhyung


>
> static const char *perf_event__names[] = {
> [0] = "TOTAL",
> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> index 1b19728..2b0554b 100644
> --- a/tools/perf/util/event.h
> +++ b/tools/perf/util/event.h
> @@ -6,6 +6,7 @@
>
> #include "../perf.h"
> #include "map.h"
> +#include "../compat-android.h"
>
> /*
> * PERF_SAMPLE_IP | PERF_SAMPLE_TID | *
> diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
> index 0f99f39..77c5ced 100644
> --- a/tools/perf/util/util.h
> +++ b/tools/perf/util/util.h
> @@ -70,15 +70,19 @@
> #include <sys/socket.h>
> #include <sys/ioctl.h>
> #include <sys/select.h>
> +#ifndef ANDROID
> #include <netinet/in.h>
> #include <netinet/tcp.h>
> #include <arpa/inet.h>
> +#endif
> #include <netdb.h>
> #include <pwd.h>
> #include <inttypes.h>
> #include "../../../include/linux/magic.h"
> #include "types.h"
> +#ifndef ANDROID
> #include <sys/ttydefaults.h>
> +#endif
>
> extern const char *graph_line;
> extern const char *graph_dotted_line;

2012-08-24 09:16:31

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCHv2 1/1] perf: Port to Android

On Fri, 24 Aug 2012 18:02:24 +0900, Namhyung Kim wrote:
> On Thu, 23 Aug 2012 17:01:52 +0200, Bernhard Rosenkraenzer wrote:
[SNIP]
>> +
>> +/* Assorted functions that are missing from Bionic */
>> +static void psignal(int sig, const char *s)
>> +{
>> + if(sig >= 0 && sig < NSIG) {
>> + if(s)
>> + fprintf(stderr, "%s: %s\n", s, sys_siglist[sig]);
>> + else
>> + fprintf(stderr, "%s\n", sys_siglist[sig]);
>> + } else {
>> + if(s)
>> + fprintf(stderr, "%s: invalid signal\n", s);
>> + else
>> + fputs("invalid signal\n", stderr);
>> + }
>> +}
>> +
>> +static ssize_t getline(char **lineptr, size_t *n, FILE *stream)
>> +{
>> + size_t ret = 0;
>> +
>> + if (!lineptr || !n || !stream)
>> + return -1;
>> +
>> + if(!*lineptr) {
>> + *n = 128;
>> + *lineptr = (char*)malloc(*n);
>> + if(!*lineptr)
>> + return -1;
>> + }
>> +
>> + while(!feof(stream) && !ferror(stream)) {
>> + int c;
>> + if(ret == *n) {
>> + *n += 128;
>> + *lineptr = (char*)realloc(*lineptr, *n);
>> + if(!*lineptr) {
>> + *n = 0;
>> + return -1;
>> + }
>> + }
>> + c = fgetc(stream);
>> + if(c == EOF)
>> + break;
>> + *lineptr[ret++] = c;
>> + if(c == '\n')
>> + break;
>> + }
>> + *lineptr[ret] = 0;
>> + return ret;
>> +}
>
> And above two functions can be moved too somehow.

I meant it can be moved to the .c file.

Thanks,
Namhyung

2012-08-24 12:26:23

by Alan

[permalink] [raw]
Subject: Re: [PATCHv2 1/1] perf: Port to Android

On Fri, 24 Aug 2012 05:08:45 +0100
Matthew Garrett <[email protected]> wrote:

> On Thu, Aug 23, 2012 at 03:32:50PM -0600, David Ahern wrote:
>
> > Why not add support for the missing functions (on_exit, getsid,
> > psignal and getline) to Bionic instead of perf?
>
> Many vendors need to target existing Android platforms and don't have
> the luxury of waiting for code to get through Google review and then
> land on devices between 6 months in the future and never. The embedded
> world is dysfunctional in a whole bunch of ways.

So add a small library and link with -lstuffandroidforgot. That's
basically how the FSF dealt with all the proprietary Unix library messes.

Come to think of it call the library "integrated build runtime
environment" and you can like with -libre ;)

It's also going to be far more maintainable and generically useful for
porting tools from GNU to 'droid userspace.

Alan

2012-08-24 18:02:11

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCHv2 1/1] perf: Port to Android


* Alan Cox <[email protected]> wrote:

> On Fri, 24 Aug 2012 05:08:45 +0100
> Matthew Garrett <[email protected]> wrote:
>
> > On Thu, Aug 23, 2012 at 03:32:50PM -0600, David Ahern wrote:
> >
> > > Why not add support for the missing functions (on_exit, getsid,
> > > psignal and getline) to Bionic instead of perf?
> >
> > Many vendors need to target existing Android platforms and
> > don't have the luxury of waiting for code to get through
> > Google review and then land on devices between 6 months in
> > the future and never. The embedded world is dysfunctional in
> > a whole bunch of ways.
>
> So add a small library and link with -lstuffandroidforgot.
> That's basically how the FSF dealt with all the proprietary
> Unix library messes.
>
> Come to think of it call the library "integrated build runtime
> environment" and you can like with -libre ;)
>
> It's also going to be far more maintainable and generically
> useful for porting tools from GNU to 'droid userspace.

Well, you should see the uglies Git has to support Windows ;-)

Anyway, I'm not against making perf work on Android in
principle, it is Obviously Useful (tm).

Thanks,

Ingo