PATCH 2.4.23-pre6 add kmap_types.h for CONFIG_CRYPTO

PATCH 2.4.23-pre6 add kmap_types.h for CONFIG_CRYPTO

[Grant Grundler]

CONFIG_CRYPTO requires asm/kmaps_types.h.
patch below clones kmap_types.h from asm-i386.

This doesn't feel like a great solution.
Willy suggested the right fix might be to not require all arches to
provide asm/kmap_types.h.  I'll let the folks who know WTH kmap_types.h
is really for debate and submit this with the hope of getting the
ia64-linux to build with CONFIG_CRYPTO.

grant


diff -Nur --exclude=RCS --exclude=CVS --exclude=SCCS --exclude=BitKeeper --exclude=ChangeSet linux-ia64-2.4-orig/include/asm-ia64/kmap_types.h linux-ia64-2.4/include/asm-ia64/kmap_types.h
--- linux-ia64-2.4-orig/include/asm-ia64/kmap_types.h	Wed Dec 31 16:00:00 1969
+++ linux-ia64-2.4/include/asm-ia64/kmap_types.h	Thu Oct 16 08:29:53 2003
@@ -0,0 +1,16 @@
+#ifndef _ASM_KMAP_TYPES_H
+#define _ASM_KMAP_TYPES_H
+
+enum km_type {
+	KM_BOUNCE_READ,
+	KM_SKB_SUNRPC_DATA,
+	KM_SKB_DATA_SOFTIRQ,
+	KM_USER0,
+	KM_USER1,
+	KM_BH_IRQ,
+	KM_SOFTIRQ0,
+	KM_SOFTIRQ1,
+	KM_TYPE_NR
+};
+
+#endif

Re: PATCH 2.4.23-pre6 add kmap_types.h for CONFIG_CRYPTO

[Matthew Wilcox]

On Thu, Oct 16, 2003 at 12:29:58PM -0700, Grant Grundler wrote:
> This doesn't feel like a great solution.
> Willy suggested the right fix might be to not require all arches to
> provide asm/kmap_types.h.  I'll let the folks who know WTH kmap_types.h
> is really for debate and submit this with the hope of getting the
> ia64-linux to build with CONFIG_CRYPTO.

Cloning kmap_types from i386 is a pretty bad idea since that architecture
actually supports highmem.  Better to clone from an arch which has no
support for highmem such as ARM.

-- 
"It's not Hollywood.  War is real, war is primarily not about defeat or
victory, it is about death.  I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk

Re: PATCH 2.4.23-pre6 add kmap_types.h for CONFIG_CRYPTO

[Grant Grundler]

On Thu, Oct 16, 2003 at 08:40:22PM +0100, Matthew Wilcox wrote:
> Cloning kmap_types from i386 is a pretty bad idea since that architecture
> actually supports highmem.  Better to clone from an arch which has no
> support for highmem such as ARM.

uhm, I would, but...
grundler@gsyprf3:/usr/src/linux-ia64-2.4-orig$ ls -l include/asm-arm/kma*
ls: include/asm-arm/kma*: No such file or directory

Anyway, given ia64 doesn't otherwise need kmap_types.h, I suspect it
doesn't matter. Except that it's misleading. Maybe a stripped down version
of the file would be better.

grant

Re: PATCH 2.4.23-pre6 add kmap_types.h for CONFIG_CRYPTO

[Keith Owens]

On Thu, 16 Oct 2003 12:29:58 -0700, 
Grant Grundler <iod00d@hp.com> wrote:
>CONFIG_CRYPTO requires asm/kmaps_types.h.
>patch below clones kmap_types.h from asm-i386.

That is an error in crypto/internal.h.  Nothing should include
asm/kmap_types.h directly, they should include linux/highmem.h and let
that decide if the arch needs kmap or not.

Re: PATCH 2.4.23-pre6 add kmap_types.h for CONFIG_CRYPTO

[Christoph Hellwig]

On Fri, Oct 17, 2003 at 09:20:33AM +1000, Keith Owens wrote:
> That is an error in crypto/internal.h.  Nothing should include
> asm/kmap_types.h directly, they should include linux/highmem.h and let
> that decide if the arch needs kmap or not.

The would be the logical conclusion, but the crypto maintainers
decided this is a feature and not a bug..

Re: PATCH 2.4.23-pre6 add kmap_types.h for CONFIG_CRYPTO

[Jes Sorensen]

>>>>> "Matthew" = Matthew Wilcox <willy@debian.org> writes:

Matthew> On Thu, Oct 16, 2003 at 12:29:58PM -0700, Grant Grundler
Matthew> wrote:
>> This doesn't feel like a great solution.  Willy suggested the right
>> fix might be to not require all arches to provide asm/kmap_types.h.
>> I'll let the folks who know WTH kmap_types.h is really for debate
>> and submit this with the hope of getting the ia64-linux to build
>> with CONFIG_CRYPTO.

Matthew> Cloning kmap_types from i386 is a pretty bad idea since that
Matthew> architecture actually supports highmem.  Better to clone from
Matthew> an arch which has no support for highmem such as ARM.

I'd argue we should fix the crypto code instead, it doesn't seem
reasonable for them to require kmap types to be defined on archs that
don't have highmem.

Cheers,
Jes

Re: PATCH 2.4.23-pre6 add kmap_types.h for CONFIG_CRYPTO

[Jes Sorensen]

>>>>> "Christoph" = Christoph Hellwig <hch@infradead.org> writes:

Christoph> On Fri, Oct 17, 2003 at 09:20:33AM +1000, Keith Owens
Christoph> wrote:
>> That is an error in crypto/internal.h.  Nothing should include
>> asm/kmap_types.h directly, they should include linux/highmem.h and
>> let that decide if the arch needs kmap or not.

Christoph> The would be the logical conclusion, but the crypto
Christoph> maintainers decided this is a feature and not a bug..

Well then we send Linus a big-fix, clearly this is a bug, if the
crypto maintainer doesn't want to fix it, we'll have to fix it for
him/her.

Jes

Re: PATCH 2.4.23-pre6 add kmap_types.h for CONFIG_CRYPTO

[Bjorn Helgaas]

Here's my proposal for a fix.  I don't think I'll send it to Linus/Andrew
yet, because it's really a cleanup, not an actual bug fix, but here it
is for any comments.

Bjorn




Currently every architecture must supply kmap_types.h, even though
it only makes sense for architectures that use highmem.

This patch removes the need for generic code to know about kmap_types.h
by adding the appropriate #include to include/linux/highmem.h and
defining a minimal set of KM_ enums that are used by generic code.

One might argue that we shouldn't need to define even the minimal
set of KM_ enums, but generic code needs to be able to do, for
example:

	kmap_atomic(page, KM_USER);

In many cases the KM_USER needn't be defined at all because the
non-highmem kmap_atomic() never evaluates it, but leaving it
undefined also restricts generic code from doing other things
like defining an array of KM_ types.  crypto/cipher.c defines
such an array, and it seems non-intuitive to prevent that usage.

This allows the removal of the following dummy kmap_types.h files: 

	include/asm-alpha/kmap_types.h
	include/asm-arm/kmap_types.h
	include/asm-arm26/kmap_types.h
	include/asm-cris/kmap_types.h
	include/asm-h8300/kmap_types.h
	include/asm-ia64/kmap_types.h
	include/asm-m68k/kmap_types.h
	include/asm-m68knommu/kmap_types.h
	include/asm-parisc/kmap_types.h
	include/asm-ppc64/kmap_types.h
	include/asm-s390/kmap_types.h
	include/asm-sh/kmap_types.h
	include/asm-sparc64/kmap_types.h
	include/asm-v850/kmap_types.h
	include/asm-x86_64/kmap_types.h

This is against 2.6 and has been boot tested on ia64 and x86.

Bjorn


# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1368  -> 1.1369 
#	include/asm-m68knommu/kmap_types.h	1.1     ->         (deleted)      
#	   crypto/internal.h	1.19    -> 1.20   
#	include/asm-m68k/kmap_types.h	1.3     ->         (deleted)      
#	include/asm-v850/kmap_types.h	1.1     ->         (deleted)      
#	include/asm-cris/kmap_types.h	1.1     ->         (deleted)      
#	include/asm-arm26/kmap_types.h	1.1     ->         (deleted)      
#	            fs/aio.c	1.37    -> 1.38   
#	include/asm-sh/kmap_types.h	1.1     ->         (deleted)      
#	include/asm-ia64/kmap_types.h	1.5     ->         (deleted)      
#	include/asm-ppc64/kmap_types.h	1.4     ->         (deleted)      
#	include/asm-parisc/kmap_types.h	1.3     ->         (deleted)      
#	include/asm-s390/kmap_types.h	1.4     ->         (deleted)      
#	include/asm-h8300/kmap_types.h	1.1     ->         (deleted)      
#	include/asm-x86_64/kmap_types.h	1.7     ->         (deleted)      
#	include/linux/highmem.h	1.27    -> 1.28   
#	include/asm-sparc64/kmap_types.h	1.4     ->         (deleted)      
#	include/asm-arm/kmap_types.h	1.1     ->         (deleted)      
#	include/asm-alpha/kmap_types.h	1.5     ->         (deleted)      
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/10/23	bjorn.helgaas@hp.com	1.1369
# Remove dummy kmap_types.h files.  Only architectures that use highmem
# really need those files.
# --------------------------------------------
#
diff -Nru a/crypto/internal.h b/crypto/internal.h
--- a/crypto/internal.h	Thu Oct 23 10:54:38 2003
+++ b/crypto/internal.h	Thu Oct 23 10:54:38 2003
@@ -17,7 +17,6 @@
 #include <linux/init.h>
 #include <linux/kmod.h>
 #include <asm/hardirq.h>
-#include <asm/kmap_types.h>
 
 extern enum km_type crypto_km_types[];
 
diff -Nru a/fs/aio.c b/fs/aio.c
--- a/fs/aio.c	Thu Oct 23 10:54:38 2003
+++ b/fs/aio.c	Thu Oct 23 10:54:38 2003
@@ -28,7 +28,6 @@
 #include <linux/highmem.h>
 #include <linux/workqueue.h>
 
-#include <asm/kmap_types.h>
 #include <asm/uaccess.h>
 #include <asm/mmu_context.h>
 
diff -Nru a/include/asm-alpha/kmap_types.h b/include/asm-alpha/kmap_types.h
--- a/include/asm-alpha/kmap_types.h	Thu Oct 23 10:54:38 2003
+++ /dev/null	Wed Dec 31 16:00:00 1969
@@ -1,33 +0,0 @@
-#ifndef _ASM_KMAP_TYPES_H
-#define _ASM_KMAP_TYPES_H
-
-/* Dummy header just to define km_type. */
-
-#include <linux/config.h>
-
-#ifdef CONFIG_DEBUG_HIGHMEM
-# define D(n) __KM_FENCE_##n ,
-#else
-# define D(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
-};
-
-#undef D
-
-#endif
diff -Nru a/include/asm-arm/kmap_types.h b/include/asm-arm/kmap_types.h
--- a/include/asm-arm/kmap_types.h	Thu Oct 23 10:54:38 2003
+++ /dev/null	Wed Dec 31 16:00:00 1969
@@ -1,12 +0,0 @@
-#ifndef __ARM_KMAP_TYPES_H
-#define __ARM_KMAP_TYPES_H
-
-/*
- * This is the "bare minimum".  AIO seems to require this.
- */
-enum km_type {
-	KM_IRQ0,
-	KM_USER1
-};
-
-#endif
diff -Nru a/include/asm-arm26/kmap_types.h b/include/asm-arm26/kmap_types.h
--- a/include/asm-arm26/kmap_types.h	Thu Oct 23 10:54:38 2003
+++ /dev/null	Wed Dec 31 16:00:00 1969
@@ -1,12 +0,0 @@
-#ifndef __ARM_KMAP_TYPES_H
-#define __ARM_KMAP_TYPES_H
-
-/*
- * This is the "bare minimum".  AIO seems to require this.
- */
-enum km_type {
-        KM_IRQ0,
-        KM_USER1
-};
-
-#endif
diff -Nru a/include/asm-cris/kmap_types.h b/include/asm-cris/kmap_types.h
--- a/include/asm-cris/kmap_types.h	Thu Oct 23 10:54:38 2003
+++ /dev/null	Wed Dec 31 16:00:00 1969
@@ -1,25 +0,0 @@
-#ifndef _ASM_KMAP_TYPES_H
-#define _ASM_KMAP_TYPES_H
-
-/* Dummy header just to define km_type.  None of this
- * is actually used on cris. 
- */
-
-enum km_type {
-	KM_BOUNCE_READ,
-	KM_SKB_SUNRPC_DATA,
-	KM_SKB_DATA_SOFTIRQ,
-	KM_USER0,
-	KM_USER1,
-	KM_BIO_SRC_IRQ,
-	KM_BIO_DST_IRQ,
-	KM_PTE0,
-	KM_PTE1,
-	KM_IRQ0,
-	KM_IRQ1,
-	KM_CRYPTO_USER,
-	KM_CRYPTO_SOFTIRQ,
-	KM_TYPE_NR
-};
-
-#endif
diff -Nru a/include/asm-h8300/kmap_types.h b/include/asm-h8300/kmap_types.h
--- a/include/asm-h8300/kmap_types.h	Thu Oct 23 10:54:38 2003
+++ /dev/null	Wed Dec 31 16:00:00 1969
@@ -1,19 +0,0 @@
-#ifndef _ASM_KMAP_TYPES_H
-#define _ASM_KMAP_TYPES_H
-
-enum km_type {
-	KM_BOUNCE_READ,
-	KM_SKB_SUNRPC_DATA,
-	KM_SKB_DATA_SOFTIRQ,
-	KM_USER0,
-	KM_USER1,
-	KM_BIO_SRC_IRQ,
-	KM_BIO_DST_IRQ,
-	KM_PTE0,
-	KM_PTE1,
-	KM_IRQ0,
-	KM_IRQ1,
-	KM_TYPE_NR
-};
-
-#endif
diff -Nru a/include/asm-ia64/kmap_types.h b/include/asm-ia64/kmap_types.h
--- a/include/asm-ia64/kmap_types.h	Thu Oct 23 10:54:38 2003
+++ /dev/null	Wed Dec 31 16:00:00 1969
@@ -1,31 +0,0 @@
-#ifndef _ASM_IA64_KMAP_TYPES_H
-#define _ASM_IA64_KMAP_TYPES_H
-
-#include <linux/config.h>
-
-#ifdef CONFIG_DEBUG_HIGHMEM
-# define D(n) __KM_FENCE_##n ,
-#else
-# define D(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
-};
-
-#undef D
-
-#endif /* _ASM_IA64_KMAP_TYPES_H */
diff -Nru a/include/asm-m68k/kmap_types.h b/include/asm-m68k/kmap_types.h
--- a/include/asm-m68k/kmap_types.h	Thu Oct 23 10:54:38 2003
+++ /dev/null	Wed Dec 31 16:00:00 1969
@@ -1,21 +0,0 @@
-#ifndef __ASM_M68K_KMAP_TYPES_H
-#define __ASM_M68K_KMAP_TYPES_H
-
-enum km_type {
-	KM_BOUNCE_READ,
-	KM_SKB_SUNRPC_DATA,
-	KM_SKB_DATA_SOFTIRQ,
-	KM_USER0,
-	KM_USER1,
-	KM_BIO_SRC_IRQ,
-	KM_BIO_DST_IRQ,
-	KM_PTE0,
-	KM_PTE1,
-	KM_IRQ0,
-	KM_IRQ1,
-	KM_SOFTIRQ0,
-	KM_SOFTIRQ1,
-	KM_TYPE_NR
-};
-
-#endif	/* __ASM_M68K_KMAP_TYPES_H */
diff -Nru a/include/asm-m68knommu/kmap_types.h b/include/asm-m68knommu/kmap_types.h
--- a/include/asm-m68knommu/kmap_types.h	Thu Oct 23 10:54:38 2003
+++ /dev/null	Wed Dec 31 16:00:00 1969
@@ -1,19 +0,0 @@
-#ifndef _ASM_KMAP_TYPES_H
-#define _ASM_KMAP_TYPES_H
-
-enum km_type {
-	KM_BOUNCE_READ,
-	KM_SKB_SUNRPC_DATA,
-	KM_SKB_DATA_SOFTIRQ,
-	KM_USER0,
-	KM_USER1,
-	KM_BIO_SRC_IRQ,
-	KM_BIO_DST_IRQ,
-	KM_PTE0,
-	KM_PTE1,
-	KM_IRQ0,
-	KM_IRQ1,
-	KM_TYPE_NR
-};
-
-#endif
diff -Nru a/include/asm-parisc/kmap_types.h b/include/asm-parisc/kmap_types.h
--- a/include/asm-parisc/kmap_types.h	Thu Oct 23 10:54:38 2003
+++ /dev/null	Wed Dec 31 16:00:00 1969
@@ -1,31 +0,0 @@
-#ifndef _ASM_KMAP_TYPES_H
-#define _ASM_KMAP_TYPES_H
-
-#include <linux/config.h>
-
-#ifdef CONFIG_DEBUG_HIGHMEM
-# define D(n) __KM_FENCE_##n ,
-#else
-# define D(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
-};
-
-#undef D
-
-#endif
diff -Nru a/include/asm-ppc64/kmap_types.h b/include/asm-ppc64/kmap_types.h
--- a/include/asm-ppc64/kmap_types.h	Thu Oct 23 10:54:38 2003
+++ /dev/null	Wed Dec 31 16:00:00 1969
@@ -1,23 +0,0 @@
-#ifdef __KERNEL__
-#ifndef _ASM_KMAP_TYPES_H
-#define _ASM_KMAP_TYPES_H
-
-enum km_type {
-	KM_BOUNCE_READ,
-	KM_SKB_SUNRPC_DATA,
-	KM_SKB_DATA_SOFTIRQ,
-	KM_USER0,
-	KM_USER1,
-	KM_BIO_SRC_IRQ,
-	KM_BIO_DST_IRQ,
-	KM_PTE0,
-	KM_PTE1,
-	KM_IRQ0,
-	KM_IRQ1,
-	KM_SOFTIRQ0,
-	KM_SOFTIRQ1,	
-	KM_TYPE_NR
-};
-
-#endif
-#endif /* __KERNEL__ */
diff -Nru a/include/asm-s390/kmap_types.h b/include/asm-s390/kmap_types.h
--- a/include/asm-s390/kmap_types.h	Thu Oct 23 10:54:38 2003
+++ /dev/null	Wed Dec 31 16:00:00 1969
@@ -1,23 +0,0 @@
-#ifdef __KERNEL__
-#ifndef _ASM_KMAP_TYPES_H
-#define _ASM_KMAP_TYPES_H
-
-enum km_type {
-	KM_BOUNCE_READ,
-	KM_SKB_SUNRPC_DATA,
-	KM_SKB_DATA_SOFTIRQ,
-	KM_USER0,
-	KM_USER1,
-	KM_BIO_SRC_IRQ,
-	KM_BIO_DST_IRQ,
-	KM_PTE0,
-	KM_PTE1,
-	KM_IRQ0,
-	KM_IRQ1,
-	KM_SOFTIRQ0,
-	KM_SOFTIRQ1,	
-	KM_TYPE_NR
-};
-
-#endif
-#endif /* __KERNEL__ */
diff -Nru a/include/asm-sh/kmap_types.h b/include/asm-sh/kmap_types.h
--- a/include/asm-sh/kmap_types.h	Thu Oct 23 10:54:38 2003
+++ /dev/null	Wed Dec 31 16:00:00 1969
@@ -1,33 +0,0 @@
-#ifndef __SH_KMAP_TYPES_H
-#define __SH_KMAP_TYPES_H
-
-/* Dummy header just to define km_type. */
-
-#include <linux/config.h>
-
-#if CONFIG_DEBUG_HIGHMEM
-# define D(n) __KM_FENCE_##n ,
-#else
-# define D(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
-};
-
-#undef D
-
-#endif
diff -Nru a/include/asm-sparc64/kmap_types.h b/include/asm-sparc64/kmap_types.h
--- a/include/asm-sparc64/kmap_types.h	Thu Oct 23 10:54:38 2003
+++ /dev/null	Wed Dec 31 16:00:00 1969
@@ -1,25 +0,0 @@
-#ifndef _ASM_KMAP_TYPES_H
-#define _ASM_KMAP_TYPES_H
-
-/* Dummy header just to define km_type.  None of this
- * is actually used on sparc64.  -DaveM
- */
-
-enum km_type {
-	KM_BOUNCE_READ,
-	KM_SKB_SUNRPC_DATA,
-	KM_SKB_DATA_SOFTIRQ,
-	KM_USER0,
-	KM_USER1,
-	KM_BIO_SRC_IRQ,
-	KM_BIO_DST_IRQ,
-	KM_PTE0,
-	KM_PTE1,
-	KM_IRQ0,
-	KM_IRQ1,
-	KM_SOFTIRQ0,
-	KM_SOFTIRQ1,
-	KM_TYPE_NR
-};
-
-#endif
diff -Nru a/include/asm-v850/kmap_types.h b/include/asm-v850/kmap_types.h
--- a/include/asm-v850/kmap_types.h	Thu Oct 23 10:54:38 2003
+++ /dev/null	Wed Dec 31 16:00:00 1969
@@ -1,19 +0,0 @@
-#ifndef __V850_KMAP_TYPES_H__
-#define __V850_KMAP_TYPES_H__
-
-enum km_type {
-	KM_BOUNCE_READ,
-	KM_SKB_SUNRPC_DATA,
-	KM_SKB_DATA_SOFTIRQ,
-	KM_USER0,
-	KM_USER1,
-	KM_BIO_SRC_IRQ,
-	KM_BIO_DST_IRQ,
-	KM_PTE0,
-	KM_PTE1,
-	KM_IRQ0,
-	KM_IRQ1,
-	KM_TYPE_NR
-};
-
-#endif /* __V850_KMAP_TYPES_H__ */
diff -Nru a/include/asm-x86_64/kmap_types.h b/include/asm-x86_64/kmap_types.h
--- a/include/asm-x86_64/kmap_types.h	Thu Oct 23 10:54:38 2003
+++ /dev/null	Wed Dec 31 16:00:00 1969
@@ -1,19 +0,0 @@
-#ifndef _ASM_KMAP_TYPES_H
-#define _ASM_KMAP_TYPES_H
-
-enum km_type {
-	KM_BOUNCE_READ,
-	KM_SKB_SUNRPC_DATA,
-	KM_SKB_DATA_SOFTIRQ,
-	KM_USER0,
-	KM_USER1,
-	KM_BIO_SRC_IRQ,
-	KM_BIO_DST_IRQ,
-	KM_IRQ0,
-	KM_IRQ1,
-	KM_SOFTIRQ0,
-	KM_SOFTIRQ1,
-	KM_TYPE_NR
-};
-
-#endif
diff -Nru a/include/linux/highmem.h b/include/linux/highmem.h
--- a/include/linux/highmem.h	Thu Oct 23 10:54:38 2003
+++ b/include/linux/highmem.h	Thu Oct 23 10:54:38 2003
@@ -12,6 +12,7 @@
 extern struct page *highmem_start_page;
 
 #include <asm/highmem.h>
+#include <asm/kmap_types.h>
 
 /* declarations for linux/mm/highmem.c */
 unsigned int nr_free_highpages(void);
@@ -25,6 +26,16 @@
 	might_sleep();
 	return page_address(page);
 }
+
+/* generic code may use these, even when CONFIG_HIGHMEM is not set */
+enum km_type {
+	KM_USER0,
+	KM_USER1,
+	KM_IRQ0,
+	KM_IRQ1,
+	KM_SOFTIRQ0,
+	KM_SOFTIRQ1,
+};
 
 #define kunmap(page) do { (void) (page); } while (0)
 

Re: PATCH 2.4.23-pre6 add kmap_types.h for CONFIG_CRYPTO

[Keith Owens]

On Thu, 23 Oct 2003 11:01:40 -0600, 
Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
>Here's my proposal for a fix.  I don't think I'll send it to Linus/Andrew
>yet, because it's really a cleanup, not an actual bug fix, but here it
>is for any comments.
>
>Currently every architecture must supply kmap_types.h, even though
>it only makes sense for architectures that use highmem.
>
>This patch removes the need for generic code to know about kmap_types.h
>by adding the appropriate #include to include/linux/highmem.h and
>defining a minimal set of KM_ enums that are used by generic code.
>
>One might argue that we shouldn't need to define even the minimal
>set of KM_ enums, but generic code needs to be able to do, for
>example:
>
>	kmap_atomic(page, KM_USER);
>
>In many cases the KM_USER needn't be defined at all because the
>non-highmem kmap_atomic() never evaluates it, but leaving it
>undefined also restricts generic code from doing other things
>like defining an array of KM_ types.  crypto/cipher.c defines
>such an array, and it seems non-intuitive to prevent that usage.

I disagree with this fix.  All uses of KM_ variables are restricted to
headers that make their use conditional on CONFIG_HIGHMEM.  crypto is
the only expection and the correct fix is to change crypto to test
CONFIG_HIGHMEM, not to change every architecture.  Untested.


Index: 23-pre7.1/crypto/internal.h
--- 23-pre7.1/crypto/internal.h Sun, 06 Jul 2003 14:16:24 +1000 kaos (linux-2.4/Y/g/31_internal.h 1.1 644)
+++ 23-pre7.1(w)/crypto/internal.h Fri, 24 Oct 2003 07:00:24 +1000 kaos (linux-2.4/Y/g/31_internal.h 1.1 644)
@@ -16,14 +16,17 @@
 #include <linux/init.h>
 #include <asm/hardirq.h>
 #include <asm/softirq.h>
-#include <asm/kmap_types.h>
 
+#ifdef CONFIG_HIGHMEM
 extern enum km_type crypto_km_types[];
 
 static inline enum km_type crypto_kmap_type(int out)
 {
 	return crypto_km_types[(in_softirq() ? 2 : 0) + out];
 }
+#else
+#define crypto_kmap_type(out) crypto_kmap_type_not_used_on_this_arch
+#endif
 
 static inline void *crypto_kmap(struct page *page, int out)
 {
Index: 23-pre7.1/crypto/cipher.c
--- 23-pre7.1/crypto/cipher.c Sun, 06 Jul 2003 14:16:24 +1000 kaos (linux-2.4/Y/g/38_cipher.c 1.1 644)
+++ 23-pre7.1(w)/crypto/cipher.c Fri, 24 Oct 2003 06:58:11 +1000 kaos (linux-2.4/Y/g/38_cipher.c 1.1 644)
@@ -35,12 +35,14 @@ struct scatter_walk {
 	unsigned int		offset;
 };
 
+#ifdef CONFIG_HIGHMEM
 enum km_type crypto_km_types[] = {
 	KM_USER0,
 	KM_USER1,
 	KM_SOFTIRQ0,
 	KM_SOFTIRQ1,
 };
+#endif
 
 static inline void xor_64(u8 *a, const u8 *b)
 {

Re: PATCH 2.4.23-pre6 add kmap_types.h for CONFIG_CRYPTO

[Bjorn Helgaas]

On Thursday 23 October 2003 3:04 pm, Keith Owens wrote:
> I disagree with this fix.  All uses of KM_ variables are restricted to
> headers that make their use conditional on CONFIG_HIGHMEM.

It's not true that all uses of KM_ variables are restricted to headers.
As one counter-example, file_read_actor() in mm/filemap.c does this:

	kaddr = kmap_atomic(page, KM_USER0);

I guess your argument is that KM_ variables should *only* be
used in places where they'll never be evaluated, i.e., as
arguments to kmap_atomic() and kunmap_atomic().

I think that if you can pass KM_USER0 to a function, you ought
to be able to copy KM_USER0 somewhere and pass the copy to
the function.  Otherwise we'll just have to keep explaining
this funny wart on the kmap interface.

> crypto is
> the only expection and the correct fix is to change crypto to test
> CONFIG_HIGHMEM, not to change every architecture.

I only changed so many architectures because the whole point was to
avoid requiring a dummy kmap_types.h file if you don't use highmem.
So I just removed the now-unnecessary kmap_types.h files, and I
assume you would do the same.  (Although you didn't mention changes
to include/linux/highmem.h and fs/aio.c to remove generic knowledge
of <asm/kmap_types.h>, so I could be misunderstanding your proposal.)

Bjorn

Re: PATCH 2.4.23-pre6 add kmap_types.h for CONFIG_CRYPTO

[Keith Owens]

On Thu, 23 Oct 2003 16:15:56 -0600, 
Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
>On Thursday 23 October 2003 3:04 pm, Keith Owens wrote:
>> I disagree with this fix.  All uses of KM_ variables are restricted to
>> headers that make their use conditional on CONFIG_HIGHMEM.
>
>It's not true that all uses of KM_ variables are restricted to headers.
>As one counter-example, file_read_actor() in mm/filemap.c does this:
>
>	kaddr = kmap_atomic(page, KM_USER0);
>
>I guess your argument is that KM_ variables should *only* be
>used in places where they'll never be evaluated, i.e., as
>arguments to kmap_atomic() and kunmap_atomic().

Right.

>I think that if you can pass KM_USER0 to a function, you ought
>to be able to copy KM_USER0 somewhere and pass the copy to
>the function.  Otherwise we'll just have to keep explaining
>this funny wart on the kmap interface.

Disagree.  kmap is currently nicely encapsulated.  Letting code play
with km types just makes that code more fragile.  crypto wants to use
different km types based on context (softirq or not).  If crypto needs
that functionallity then I expect other code to need it as well, which
means the facility should be part of kmap, not implemented by each code
area.

Failing that, crypto should only use km types on builds that have
CONFIG_HIGHMEM defined.

>I only changed so many architectures because the whole point was to
>avoid requiring a dummy kmap_types.h file if you don't use highmem.
>So I just removed the now-unnecessary kmap_types.h files, and I
>assume you would do the same.  (Although you didn't mention changes
>to include/linux/highmem.h and fs/aio.c to remove generic knowledge
>of <asm/kmap_types.h>, so I could be misunderstanding your proposal.)

Fix crypto to test CONFIG_HIGHMEM and no other changes are required.

Re: PATCH 2.4.23-pre6 add kmap_types.h for CONFIG_CRYPTO

[Bjorn Helgaas]

On Thursday 23 October 2003 4:28 pm, Keith Owens wrote:
> Letting code play
> with km types just makes that code more fragile.  crypto wants to use
> different km types based on context (softirq or not).  If crypto needs
> that functionallity then I expect other code to need it as well, which
> means the facility should be part of kmap, not implemented by each code
> area.

Can you give an example of what such a new kmap interface would
look like?  The crypto code looks reasonable to me, and I don't
know enough about kmap to guess what abstraction you might have
in mind.

I agree it's not optimal to define the KM_ enums in highmem.h
for the non-highmem case, but I think it's better than having
to use #ifdef CONFIG_HIGHMEM in the crypto code.

You seem to be concerned about the fact that my patch touches
so many architectures, but you said earlier:

> That is an error in crypto/internal.h.  Nothing should include
> asm/kmap_types.h directly, they should include linux/highmem.h and let
> that decide if the arch needs kmap or not.

So I assume that if you did a complete patch it would
	1) Remove "#include <asm/kmap_types.h" from crypto/internal.h
	2) Remove "#include <asm/kmap_types.h" from fs/aio.c
	3) Add "#include <asm/kmap_types.h" to include/linux/highmem.h
	   (under #ifdef CONFIG_HIGHMEM).
	4) Remove kmap_types.h from non-highmem architectures, since
	   they're no longer referenced.

Which means that your approach would touch just as many arches.

Bjorn