[PATCH] Clean up namespace in kmap_types.h

[PATCH] Clean up namespace in kmap_types.h

[Andi Kleen]

Clean up namespace in kmap_types.h

During debugging I often use custom "D" macros. But I ran
into a name space conflict with kmap_types.h which also defines a D macro.
Rename the kmap_types.h one to KMAPD.

No semantical changes.

Signed-off-by: Andi Kleen <ak@linux.intel.com>

---
 arch/x86/include/asm/kmap_types.h |   34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

Index: linux-2.6.28-rc4-test/arch/x86/include/asm/kmap_types.h
===================================================================
--- linux-2.6.28-rc4-test.orig/arch/x86/include/asm/kmap_types.h	2008-10-24 13:34:40.000000000 +0200
+++ linux-2.6.28-rc4-test/arch/x86/include/asm/kmap_types.h	2008-12-02 22:51:16.000000000 +0100
@@ -2,28 +2,28 @@
 #define _ASM_X86_KMAP_TYPES_H
 
 #if defined(CONFIG_X86_32) && defined(CONFIG_DEBUG_HIGHMEM)
-# define D(n) __KM_FENCE_##n ,
+# define KMAPD(n) __KM_FENCE_##n ,
 #else
-# define D(n)
+# define KMAPD(n)
 #endif
 
 enum km_type {
-D(0)	KM_BOUNCE_READ,
-D(1)	KM_SKB_SUNRPC_DATA,
-D(2)	KM_SKB_DATA_SOFTIRQ,
-D(3)	KM_USER0,
-D(4)	KM_USER1,
-D(5)	KM_BIO_SRC_IRQ,
-D(6)	KM_BIO_DST_IRQ,
-D(7)	KM_PTE0,
-D(8)	KM_PTE1,
-D(9)	KM_IRQ0,
-D(10)	KM_IRQ1,
-D(11)	KM_SOFTIRQ0,
-D(12)	KM_SOFTIRQ1,
-D(13)	KM_TYPE_NR
+KMAPD(0)	KM_BOUNCE_READ,
+KMAPD(1)	KM_SKB_SUNRPC_DATA,
+KMAPD(2)	KM_SKB_DATA_SOFTIRQ,
+KMAPD(3)	KM_USER0,
+KMAPD(4)	KM_USER1,
+KMAPD(5)	KM_BIO_SRC_IRQ,
+KMAPD(6)	KM_BIO_DST_IRQ,
+KMAPD(7)	KM_PTE0,
+KMAPD(8)	KM_PTE1,
+KMAPD(9)	KM_IRQ0,
+KMAPD(10)	KM_IRQ1,
+KMAPD(11)	KM_SOFTIRQ0,
+KMAPD(12)	KM_SOFTIRQ1,
+KMAPD(13)	KM_TYPE_NR
 };
 
-#undef D
+#undef KMAPD
 
 #endif /* _ASM_X86_KMAP_TYPES_H */

Re: [PATCH] Clean up namespace in kmap_types.h

[Arnd Bergmann]

On Wednesday 03 December 2008, Andi Kleen wrote:
> Clean up namespace in kmap_types.h
> 
> During debugging I often use custom "D" macros. But I ran
> into a name space conflict with kmap_types.h which also defines a D macro.
> Rename the kmap_types.h one to KMAPD.
> 
> No semantical changes.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

The patch looks ok, but it seems wrong to do this only on one architecture,
because kmap_types.h is mostly identical across all architectures.

I'm working on patches to consolidate a lot of generic headers, why not
add this in a generic way and make x86 use that version?

	Arnd <><

--- /dev/null
+++ b/include/asm-generic/kmap_types.h
@@ -0,0 +1,29 @@
+#ifndef __ASM_GENERIC_KMAP_TYPES_H
+#define __ASM_GENERIC_KMAP_TYPES_H
+
+#ifdef CONFIG_DEBUG_HIGHMEM
+# define KMAPD(n) __KM_FENCE_##n ,
+#else
+# define KMAPD(n)
+#endif
+
+enum km_type {
+KMAPD(0)	KM_BOUNCE_READ,
+KMAPD(1)	KM_SKB_SUNRPC_DATA,
+KMAPD(2)	KM_SKB_DATA_SOFTIRQ,
+KMAPD(3)	KM_USER0,
+KMAPD(4)	KM_USER1,
+KMAPD(5)	KM_BIO_SRC_IRQ,
+KMAPD(6)	KM_BIO_DST_IRQ,
+KMAPD(7)	KM_PTE0,
+KMAPD(8)	KM_PTE1,
+KMAPD(9)	KM_IRQ0,
+KMAPD(10)	KM_IRQ1,
+KMAPD(11)	KM_SOFTIRQ0,
+KMAPD(12)	KM_SOFTIRQ1,
+KMAPD(13)	KM_TYPE_NR
+};
+
+#undef KMAPD
+
+#endif /* __ASM_GENERIC_KMAP_TYPES_H */

Re: [PATCH] Clean up namespace in kmap_types.h

[Andi Kleen]

On Wed, Dec 03, 2008 at 11:40:01AM +0100, Arnd Bergmann wrote:
> On Wednesday 03 December 2008, Andi Kleen wrote:
> > Clean up namespace in kmap_types.h
> > 
> > During debugging I often use custom "D" macros. But I ran
> > into a name space conflict with kmap_types.h which also defines a D macro.
> > Rename the kmap_types.h one to KMAPD.
> > 
> > No semantical changes.
> > 
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> The patch looks ok, but it seems wrong to do this only on one architecture,
> because kmap_types.h is mostly identical across all architectures.
> 
> I'm working on patches to consolidate a lot of generic headers, why not
> add this in a generic way and make x86 use that version?

I was hoping that it could be still fixed for 2.6.28. Perhaps
that would be too intrusive that late? That is why I hope
it can be still considered even if it's x86 only.

For .29 I suspect in fact the best way would be to just move it into linux
and use a superset because I don't think it's a big problem to waste 3-4
pages of virtual space for this.

> --- /dev/null
> +++ b/include/asm-generic/kmap_types.h
> @@ -0,0 +1,29 @@
> +#ifndef __ASM_GENERIC_KMAP_TYPES_H
> +#define __ASM_GENERIC_KMAP_TYPES_H

So are you submitting this?

-Andi

New location for generic headers?

[Sam Ravnborg]

On Wed, Dec 03, 2008 at 11:40:01AM +0100, Arnd Bergmann wrote:
> On Wednesday 03 December 2008, Andi Kleen wrote:
> > Clean up namespace in kmap_types.h
> > 
> > During debugging I often use custom "D" macros. But I ran
> > into a name space conflict with kmap_types.h which also defines a D macro.
> > Rename the kmap_types.h one to KMAPD.
> > 
> > No semantical changes.
> > 
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> The patch looks ok, but it seems wrong to do this only on one architecture,
> because kmap_types.h is mostly identical across all architectures.
> 
> I'm working on patches to consolidate a lot of generic headers, why not
> add this in a generic way and make x86 use that version?

Should we try to get rid of all those silly:
#include <asm-generic/foo.h>
files we have in arch/$ARCH/include/asm?

We could for example add include/generic/ last in the search path.
Or we could add the files in include/asm and add that last in the
search path.

Then if we do not find <asm/foo.h> in arch/$ARCH/include/asm/
then we search include/asm.

Before we can do this we need to get rid of the last users of
include/asm-$ARCH - but we are almost there.
Only m68k seems to require some effort.

	Sam

Re: [PATCH] Clean up namespace in kmap_types.h

[Arnd Bergmann]

On Wednesday 03 December 2008, Andi Kleen wrote:

> > The patch looks ok, but it seems wrong to do this only on one architecture,
> > because kmap_types.h is mostly identical across all architectures.
> > 
> > I'm working on patches to consolidate a lot of generic headers, why not
> > add this in a generic way and make x86 use that version?
> 
> I was hoping that it could be still fixed for 2.6.28. Perhaps
> that would be too intrusive that late? That is why I hope
> it can be still considered even if it's x86 only.

I don't think it's too intrusive, it just doesn't fit the 'strictly
bug fixes' definition.

> For .29 I suspect in fact the best way would be to just move it into linux
> and use a superset because I don't think it's a big problem to waste 3-4
> pages of virtual space for this.

Yes, that would work as well. FWIW, we currently have the following
nonstandard kmap types:

avr32: KM_PTE2 (unused)
frv: __KM_CACHE, __KM_PGD, __KM_ITLB_PTD, __KM_DTLB_PTD (unused
	except for __KM_CACHE)
powerpc: KM_PPC_SYNC_PAGE, KM_PPC_SYNC_ICACHE

It should be fair to introduce common types KM_DCACHE (__KM_CACHE
and KM_PPC_SYNC_PAGE) and KM_ICACHE (KM_PPC_SYNC_ICACHE) for these.

> > --- /dev/null
> > +++ b/include/asm-generic/kmap_types.h
> > @@ -0,0 +1,29 @@
> > +#ifndef __ASM_GENERIC_KMAP_TYPES_H
> > +#define __ASM_GENERIC_KMAP_TYPES_H
> 
> So are you submitting this?

It's part of a larger series that I'm planning to submit for 2.6.29,
but I wouldn't mind if you take care of this file independently.

	Arnd <><

Re: New location for generic headers?

[Andi Kleen]

> Should we try to get rid of all those silly:
> #include <asm-generic/foo.h>
> files we have in arch/$ARCH/include/asm?
> 
> We could for example add include/generic/ last in the search path.
> Or we could add the files in include/asm and add that last in the
> search path.
> 
> Then if we do not find <asm/foo.h> in arch/$ARCH/include/asm/
> then we search include/asm.

The only problem I see is that it would user space use of asm/ 
without make headers_install impossible. I suppose that could
break the setup of a lot of people? I think it would cause
problems on some of my older boxes at least.

-Andi

Re: New location for generic headers?

[Christoph Hellwig]

On Wed, Dec 03, 2008 at 01:19:55PM +0100, Andi Kleen wrote:
> The only problem I see is that it would user space use of asm/ 
> without make headers_install impossible.

Well, they aren't supposed to do that anyway, so I would consider
breaking it a good idea.

Re: New location for generic headers?

[Arnd Bergmann]

On Wednesday 03 December 2008, Sam Ravnborg wrote:
> On Wed, Dec 03, 2008 at 11:40:01AM +0100, Arnd Bergmann wrote:
>
> > I'm working on patches to consolidate a lot of generic headers, why not
> > add this in a generic way and make x86 use that version?
> 
> Should we try to get rid of all those silly:
> #include <asm-generic/foo.h>
> files we have in arch/$ARCH/include/asm?
> 
> We could for example add include/generic/ last in the search path.
> Or we could add the files in include/asm and add that last in the
> search path.

My current idea is to have an explicit list of asm-generic headers
that can act as drop-in replacements, either in include/asm-generic/Makefile
or in arch/*/include/asm/Makefile, and then generate these trivial
wrappers in $(objdir)arch/*/include/asm/ and $(INSTALL_HDR_PATH)/asm
if it doesn't already exist there.

This way we don't have to change all the headers that do #include
<asm-generic/foo.h> plus something else.
 
	Arnd <><

Re: New location for generic headers?

[Christoph Hellwig]

On Wed, Dec 03, 2008 at 01:42:18PM +0100, Andi Kleen wrote:
> On Wed, Dec 03, 2008 at 07:21:49AM -0500, Christoph Hellwig wrote:
> > On Wed, Dec 03, 2008 at 01:19:55PM +0100, Andi Kleen wrote:
> > > The only problem I see is that it would user space use of asm/ 
> > > without make headers_install impossible.
> > 
> > Well, they aren't supposed to do that anyway, so I would consider
> > breaking it a good idea.
> 
> Sorry, but working against your users is never a good strategy.

It's not working against users.  It's just preventing something that
is a very bad thing already. 

Re: New location for generic headers?

[Andi Kleen]

On Wed, Dec 03, 2008 at 07:21:49AM -0500, Christoph Hellwig wrote:
> On Wed, Dec 03, 2008 at 01:19:55PM +0100, Andi Kleen wrote:
> > The only problem I see is that it would user space use of asm/ 
> > without make headers_install impossible.
> 
> Well, they aren't supposed to do that anyway, so I would consider
> breaking it a good idea.

Sorry, but working against your users is never a good strategy.

-Andi

-- 
ak@linux.intel.com