From: Namhyung Kim <namhyung@kernel.org>
To: David Howells <dhowells@redhat.com>
Cc: mingo@kernel.org, tglx@linutronix.de, acme@ghostprotocols.net,
davem@davemloft.net, torvalds@linux-foundation.org,
paulus@samba.org, linux-arch@vger.kernel.org,
linux-kernel@vger.kernel.org, x86@kernel.org
Subject: Re: [PATCH 3/5] perf: Make perf build for x86 with UAPI disintegration applied
Date: Fri, 26 Oct 2012 14:49:22 +0900 [thread overview]
Message-ID: <87d305940t.fsf@sejong.aot.lge.com> (raw)
In-Reply-To: <20121019165624.23037.1780.stgit@warthog.procyon.org.uk> (David Howells's message of "Fri, 19 Oct 2012 17:56:24 +0100")
On Fri, 19 Oct 2012 17:56:24 +0100, David Howells wrote:
> Make perf build for x86 once the UAPI disintegration patches for that arch
> have been applied by adding the appropriate -I flags - in the right order -
> and then converting some #includes that use ../.. notation to find main kernel
> headerfiles to use <asm/foo.h> and <linux/foo.h> instead.
Looks nice.
>
> Note that -Iarch/foo/include/uapi is present _before_ -Iarch/foo/include.
> This makes sure we get the userspace version of the pt_regs struct. Ideally,
> we wouldn't have the latter -I flag at all, but unfortunately we want
> asm/svm.h and asm/vmx.h in buildin-kvm.c and these aren't part of the UAPI -
> at least not for x86. I wonder if the bits outside of the __KERNEL__ guards
> *should* be transferred there.
What about asm/kvm.h? Is it a part of the UAPI?
>
> I note also that perf seems to do its dependency handling manually by listing
> all the header files it might want to use in LIB_H in the Makefile. Can this
> be changed to use -MD?
Yeah, that part could be improved, probably with -MMD.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
>
> tools/perf/Makefile | 16 +++++++++++++++-
> tools/perf/builtin-kvm.c | 6 +++---
> tools/perf/perf.h | 16 +++-------------
> 3 files changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index f7c968a..9024a42 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -169,7 +169,21 @@ endif
>
> ### --- END CONFIGURATION SECTION ---
>
> -BASIC_CFLAGS = -Iutil/include -Iarch/$(ARCH)/include -I$(OUTPUT)util -I$(TRACE_EVENT_DIR) -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE
> +ifeq ($(srctree),)
> +srctree := $(shell pwd)
> +endif
Isn't the srctree intended to point to kernel root? Also you missed to
define the objtree which used below.
> +
> +BASIC_CFLAGS = \
> + -Iutil/include \
> + -Iarch/$(ARCH)/include \
> + -I$(objtree)/arch/$(ARCH)/include/generated/uapi \
> + -I$(srctree)/arch/$(ARCH)/include/uapi \
> + -I$(srctree)/arch/$(ARCH)/include \
> + -I$(objtree)/include/generated/uapi \
> + -I$(srctree)/include/uapi \
> + -I$(OUTPUT)util \
> + -I$(TRACE_EVENT_DIR) \
> + -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE
This isn't bad, but using '+=' looks more natural IMHO.
BASIC_CFLAGS = -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE
BASIC_CFLAGS += -Iutil/include
BASIC_CFLAGS += -Iarch/$(ARCH)/include
...
> BASIC_LDFLAGS =
>
> # Guard against environment variables
> diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
> index 260abc5..e013bdb 100644
> --- a/tools/perf/builtin-kvm.c
> +++ b/tools/perf/builtin-kvm.c
> @@ -22,9 +22,9 @@
> #include <pthread.h>
> #include <math.h>
>
> -#include "../../arch/x86/include/asm/svm.h"
> -#include "../../arch/x86/include/asm/vmx.h"
> -#include "../../arch/x86/include/asm/kvm.h"
> +#include <asm/svm.h>
> +#include <asm/vmx.h>
> +#include <asm/kvm.h>
>
> struct event_key {
> #define INVALID_KEY (~0ULL)
> diff --git a/tools/perf/perf.h b/tools/perf/perf.h
> index 2762877..238f923 100644
> --- a/tools/perf/perf.h
> +++ b/tools/perf/perf.h
> @@ -5,8 +5,9 @@ struct winsize;
>
> void get_term_dimensions(struct winsize *ws);
>
> +#include <asm/unistd.h>
> +
> #if defined(__i386__)
> -#include "../../arch/x86/include/asm/unistd.h"
> #define rmb() asm volatile("lock; addl $0,0(%%esp)" ::: "memory")
> #define cpu_relax() asm volatile("rep; nop" ::: "memory");
> #define CPUINFO_PROC "model name"
> @@ -16,7 +17,6 @@ void get_term_dimensions(struct winsize *ws);
> #endif
>
> #if defined(__x86_64__)
> -#include "../../arch/x86/include/asm/unistd.h"
> #define rmb() asm volatile("lfence" ::: "memory")
> #define cpu_relax() asm volatile("rep; nop" ::: "memory");
> #define CPUINFO_PROC "model name"
> @@ -26,20 +26,17 @@ void get_term_dimensions(struct winsize *ws);
> #endif
>
> #ifdef __powerpc__
> -#include "../../arch/powerpc/include/asm/unistd.h"
> #define rmb() asm volatile ("sync" ::: "memory")
> #define cpu_relax() asm volatile ("" ::: "memory");
> #define CPUINFO_PROC "cpu"
> #endif
>
> #ifdef __s390__
> -#include "../../arch/s390/include/asm/unistd.h"
> #define rmb() asm volatile("bcr 15,0" ::: "memory")
> #define cpu_relax() asm volatile("" ::: "memory");
> #endif
>
> #ifdef __sh__
> -#include "../../arch/sh/include/asm/unistd.h"
> #if defined(__SH4A__) || defined(__SH5__)
> # define rmb() asm volatile("synco" ::: "memory")
> #else
> @@ -50,35 +47,30 @@ void get_term_dimensions(struct winsize *ws);
> #endif
>
> #ifdef __hppa__
> -#include "../../arch/parisc/include/asm/unistd.h"
> #define rmb() asm volatile("" ::: "memory")
> #define cpu_relax() asm volatile("" ::: "memory");
> #define CPUINFO_PROC "cpu"
> #endif
>
> #ifdef __sparc__
> -#include "../../arch/sparc/include/asm/unistd.h"
It might conflict with davem's sparc uapi patch which merged into tip:
commit 77626081849c9050b20670e5d832aca54c966936
Author: David Miller <davem@davemloft.net>
Date: Wed Oct 17 01:06:56 2012 -0400
perf tools: Fix build on sparc.
More UAPI stuff.
> #define rmb() asm volatile("":::"memory")
> #define cpu_relax() asm volatile("":::"memory")
> #define CPUINFO_PROC "cpu"
> #endif
>
> #ifdef __alpha__
> -#include "../../arch/alpha/include/asm/unistd.h"
> #define rmb() asm volatile("mb" ::: "memory")
> #define cpu_relax() asm volatile("" ::: "memory")
> #define CPUINFO_PROC "cpu model"
> #endif
>
> #ifdef __ia64__
> -#include "../../arch/ia64/include/asm/unistd.h"
> #define rmb() asm volatile ("mf" ::: "memory")
> #define cpu_relax() asm volatile ("hint @pause" ::: "memory")
> #define CPUINFO_PROC "model name"
> #endif
>
> #ifdef __arm__
> -#include "../../arch/arm/include/asm/unistd.h"
> /*
> * Use the __kuser_memory_barrier helper in the CPU helper page. See
> * arch/arm/kernel/entry-armv.S in the kernel source for details.
> @@ -89,13 +81,11 @@ void get_term_dimensions(struct winsize *ws);
> #endif
>
> #ifdef __aarch64__
> -#include "../../arch/arm64/include/asm/unistd.h"
> #define rmb() asm volatile("dmb ld" ::: "memory")
> #define cpu_relax() asm volatile("yield" ::: "memory")
> #endif
>
> #ifdef __mips__
> -#include "../../arch/mips/include/asm/unistd.h"
> #define rmb() asm volatile( \
> ".set mips2\n\t" \
> "sync\n\t" \
> @@ -112,7 +102,7 @@ void get_term_dimensions(struct winsize *ws);
> #include <sys/types.h>
> #include <sys/syscall.h>
>
> -#include "../../include/uapi/linux/perf_event.h"
And I got a conflict here.
Thanks,
Namhyung
> +#include <linux/perf_event.h>
> #include "util/types.h"
> #include <stdbool.h>
>
next prev parent reply other threads:[~2012-10-26 5:49 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-19 16:55 [RFC][PATCH 0/5] tools, perf: Fix up for x86 UAPI disintegration David Howells
2012-10-19 16:56 ` [PATCH 1/5] tools: Define a Makefile function to do subdir processing David Howells
2012-10-19 16:56 ` [PATCH 2/5] tools: Honour the O= flag when tool build called from a higher Makefile David Howells
2012-10-26 5:34 ` Namhyung Kim
2012-10-19 16:56 ` [PATCH 3/5] perf: Make perf build for x86 with UAPI disintegration applied David Howells
2012-10-26 5:49 ` Namhyung Kim [this message]
2012-10-19 16:56 ` [PATCH 4/5] x86: Disintegrate asm/svm.h and asm/vmx.h to produce UAPI components for perf David Howells
2012-10-19 16:56 ` [PATCH 5/5] x86: UAPI Disintegrate asm/perf_regs.h David Howells
2012-10-24 18:43 ` [RFC][PATCH 0/5] tools, perf: Fix up for x86 UAPI disintegration Arnaldo Carvalho de Melo
2012-10-24 19:46 ` Borislav Petkov
2012-10-25 7:57 ` David Howells
2012-10-26 4:33 ` Namhyung Kim
2012-10-26 6:04 ` Namhyung Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87d305940t.fsf@sejong.aot.lge.com \
--to=namhyung@kernel.org \
--cc=acme@ghostprotocols.net \
--cc=davem@davemloft.net \
--cc=dhowells@redhat.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=paulus@samba.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.