linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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>
>  

  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 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).