From: David Miller on

Commit:

commit de5d9bf6541736dc7ad264d2b5cc99bc1b2ad958
Author: Chris Metcalf <cmetcalf(a)tilera.com>
Date: Fri Jul 2 13:41:14 2010 -0400

Move list types from <linux/list.h> to <linux/types.h>.

broke the build of 'perf'.

If you move "struct list_head" into types.h, this means perf stops
building because it depends upon being able to include linux/list.h
from a userland application and at the same time be able to get the
basic data types without defining __KERNEL__ or similar.

Now that no longer occurs because the bulk of types.h is __KERNEL__
protected and thus the build breaks since "struct list_head" is
never defined.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Chris Metcalf on
On 8/10/2010 2:57 AM, David Miller wrote:
> Commit:
>
> commit de5d9bf6541736dc7ad264d2b5cc99bc1b2ad958
> Author: Chris Metcalf <cmetcalf(a)tilera.com>
> Date: Fri Jul 2 13:41:14 2010 -0400
>
> Move list types from <linux/list.h> to <linux/types.h>.
>
> broke the build of 'perf'.
>
> If you move "struct list_head" into types.h, this means perf stops
> building because it depends upon being able to include linux/list.h
> from a userland application and at the same time be able to get the
> basic data types without defining __KERNEL__ or similar.
>

If necessary, it certainly would be easy to move the list.h types to
follow struct ustat, then bump the #endif up above them with a comment
about the perf system's use of them.

I'm confused, though, since <linux/list.h> isn't installed by
headers_install, so how was perf finding that definition before anyway?

I assume the requirement is something fairly stringent, like parsing
binary data structures out of a memory-mapped region or some such;
normally you wouldn't even want to expose the list_head declaration to
user space. I haven't looked at the perf subsystem myself yet so I
don't really know.

--
Chris Metcalf, Tilera Corp.
http://www.tilera.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Sam Ravnborg on
On Tue, Aug 10, 2010 at 08:36:46AM -0400, Chris Metcalf wrote:
> On 8/10/2010 2:57 AM, David Miller wrote:
> > Commit:
> >
> > commit de5d9bf6541736dc7ad264d2b5cc99bc1b2ad958
> > Author: Chris Metcalf <cmetcalf(a)tilera.com>
> > Date: Fri Jul 2 13:41:14 2010 -0400
> >
> > Move list types from <linux/list.h> to <linux/types.h>.
> >
> > broke the build of 'perf'.
> >
> > If you move "struct list_head" into types.h, this means perf stops
> > building because it depends upon being able to include linux/list.h
> > from a userland application and at the same time be able to get the
> > basic data types without defining __KERNEL__ or similar.
> >
>
> If necessary, it certainly would be easy to move the list.h types to
> follow struct ustat, then bump the #endif up above them with a comment
> about the perf system's use of them.
>
> I'm confused, though, since <linux/list.h> isn't installed by
> headers_install, so how was perf finding that definition before anyway?

kernel devs usually tell people not to use kernel
headers direct.
But perf does so a lot.

$ cd tools/perf/util; git grep '../../..'
header.h:#include "../../../include/linux/perf_event.h"
include/asm/byteorder.h:#include "../../../../include/linux/swab.h"
include/linux/hash.h:#include "../../../../include/linux/hash.h"
include/linux/list.h:#include "../../../../include/linux/list.h"
include/linux/poison.h:#include "../../../../include/linux/poison.h"
include/linux/rbtree.h:#include "../../../../include/linux/rbtree.h"
parse-events.c:#include "../../../include/linux/hw_breakpoint.h"
parse-events.c: snprintf(evt_path, MAXPATHLEN, "%s/%s/%s/id", debugfs_path,
parse-events.c: snprintf(evt_path, MAXPATHLEN, "%s/%s/%s/id", debugfs_path,
session.h:#include "../../../include/linux/perf_event.h"
util.h:#include "../../../include/linux/magic.h"
util.h:#include "../../../include/linux/stringify.h"

IMO the patch that moves list_head to types.h is fine.
And perf needs to learn good manner with respect to
kernel headers.

Sam
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Arnaldo Carvalho de Melo on
Em Tue, Aug 10, 2010 at 02:44:45PM +0200, Sam Ravnborg escreveu:
> IMO the patch that moves list_head to types.h is fine.
> And perf needs to learn good manner with respect to
> kernel headers.

Idea is to try to have the perf tools, since they are hosted in the
kernel and developed mostly by people with kernel background, to use
code and practices used in the kernel proper.

It started just keeping private copies, I guess it should get back to
that since the reaction to this kind of same source repo code sharing
was, well, not good :-)

Alternatives?

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Arnd Bergmann on
On Tuesday 10 August 2010, Arnaldo Carvalho de Melo wrote:
> It started just keeping private copies, I guess it should get back to
> that since the reaction to this kind of same source repo code sharing
> was, well, not good :-)
>
> Alternatives?

If perf wants to play tricks with the header files, we should probably
make them explicit, as in this ugly bit of code.

Arnd

diff --git a/include/linux/types.h b/include/linux/types.h
index 01a082f..0e0998a 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -178,7 +178,7 @@ typedef __u64 __bitwise __be64;
typedef __u16 __bitwise __sum16;
typedef __u32 __bitwise __wsum;

-#ifdef __KERNEL__
+#if defined(__KERNEL__) || defined(__PERF__)
typedef unsigned __bitwise__ gfp_t;
typedef unsigned __bitwise__ fmode_t;

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 26f626d..9806dc9 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -225,7 +225,7 @@ ifndef PERF_DEBUG
CFLAGS_OPTIMIZE = -O6
endif

-CFLAGS = -ggdb3 -Wall -Wextra -std=gnu99 -Werror $(CFLAGS_OPTIMIZE) -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
+CFLAGS = -ggdb3 -Wall -Wextra -std=gnu99 -Werror $(CFLAGS_OPTIMIZE) -D__PERF__ -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
EXTLIBS = -lpthread -lrt -lelf -lm
ALL_CFLAGS = $(CFLAGS) -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64
ALL_LDFLAGS = $(LDFLAGS)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/