Prev: [PATCH 0/4] enhanced reimplemention of the kfifo API
Next: memblock/arm: Use new accessors
From: Sam Ravnborg on 10 Aug 2010 10:40
On Tue, Aug 10, 2010 at 10:46:00AM -0300, Arnaldo Carvalho de Melo wrote:
> 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?

I have not analyzed deeper in what parts perf uses so the
following may not fly at all.

One solution could be to let perf rely on a set of
exported userspace headers from the kernel.

And on top of this copy a small set of kernel internal
headers for use by perf only.
The copying will be a good way to document what is actually
used of the kernel internal stuff.

But I also realize that just copying over list.h does
not suffice. It pulls in lots of other stuff.
So this is most likely hard to do.

I guess first step is to identify what is really used
beside the exported stuff and start to conclude from there.

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: Matthew Wilcox on 10 Aug 2010 11:20
On Mon, Aug 09, 2010 at 11:57:46PM -0700, 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.

Sorry about that. Obviously, I didn't test-build this patch.

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

list.h isn't a header-y file, so it's not an interface we expect userspace
to use. So one reasonable way to fix this is for perf to take its own copy.

Reverting the patch gets us back to the former problem (of not being
able to use list.h in certain cases).

We could do a glibc-like _WANT_LIST_HEAD macro. I don't think the
resulting uglification is reasonable.

perf could pretend that it's real userspace instead of being part of
the kernel and use a list implementation designed for userspace. I
don't know of a good one though.

Any preferences which solution we take?
--
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 10 Aug 2010 12:10
Em Tue, Aug 10, 2010 at 04:29:53PM +0200, Arnd Bergmann escreveu:
> 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.

Nah, no more ifdefs, the goal was more to share things with tools
developed/shipped in the kernel source repository, not to do something
just for perf.

> diff --git 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;

- 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/
First  |  Prev  | 
Pages: 1 2
Prev: [PATCH 0/4] enhanced reimplemention of the kfifo API
Next: memblock/arm: Use new accessors