2012-06-23 10:04:42

by Cong Wang

[permalink] [raw]
Subject: [PATCH 00/12] kmap_atomic cleanup for 3.6

After few releases, it seems there are no more callers
using the deprecated form of kmap_atomic(), the one
with two parameters. So we can remove it now and remove
the KM_* definition except KM_TYPE_NR together.

All the patches are available at:

git://github.com/congwang/linux.git #kmap_atomic

Signed-off-by: Cong Wang <[email protected]>

-------------------

Cong Wang (12):
jbd2: remove the second argument of kmap_atomic
arm: remove km_type definitions
powerpc: remove km_type definitions
frv: remove km_type definitions
avr32: remove km_type definitions
asm-generic: remove km_type definitions
um: remove km_type definitions
tile: remove km_type definitions
highmem: remove the deprecated form of kmap_atomic
feature-removal-schedule.txt: remove kmap_atomic(page, km_type)
vmalloc: remove KM_USER0 from comments
pipe: remove KM_USER0 from comments

Documentation/feature-removal-schedule.txt | 8 -----
arch/arm/include/asm/kmap_types.h | 26 +-----------------
arch/avr32/include/asm/kmap_types.h | 24 +---------------
arch/frv/include/asm/kmap_types.h | 24 +---------------
arch/powerpc/include/asm/kmap_types.h | 31 +--------------------
arch/tile/include/asm/kmap_types.h | 26 -----------------
arch/um/include/asm/kmap_types.h | 18 +-----------
fs/jbd2/commit.c | 4 +-
fs/pipe.c | 2 +-
include/asm-generic/kmap_types.h | 34 +---------------------
include/linux/highmem.h | 41 +---------------------------
include/linux/pipe_fs_i.h | 8 ++---
mm/vmalloc.c | 8 +----
13 files changed, 17 insertions(+), 237 deletions(-)

--
1.7.7.6


2012-06-23 10:05:45

by Cong Wang

[permalink] [raw]
Subject: [PATCH 02/12] arm: remove km_type definitions

Signed-off-by: Cong Wang <[email protected]>
---
arch/arm/include/asm/kmap_types.h | 26 +-------------------------
1 files changed, 1 insertions(+), 25 deletions(-)

diff --git a/arch/arm/include/asm/kmap_types.h b/arch/arm/include/asm/kmap_types.h
index e51b1e8..83eb2f7 100644
--- a/arch/arm/include/asm/kmap_types.h
+++ b/arch/arm/include/asm/kmap_types.h
@@ -4,30 +4,6 @@
/*
* This is the "bare minimum". AIO seems to require this.
*/
-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_L1_CACHE,
- KM_L2_CACHE,
- KM_KDB,
- KM_TYPE_NR
-};
-
-#ifdef CONFIG_DEBUG_HIGHMEM
-#define KM_NMI (-1)
-#define KM_NMI_PTE (-1)
-#define KM_IRQ_PTE (-1)
-#endif
+#define KM_TYPE_NR 16

#endif
--
1.7.7.6

2012-06-23 10:05:57

by Cong Wang

[permalink] [raw]
Subject: [PATCH 07/12] um: remove km_type definitions

Signed-off-by: Cong Wang <[email protected]>
---
arch/um/include/asm/kmap_types.h | 18 +-----------------
1 files changed, 1 insertions(+), 17 deletions(-)

diff --git a/arch/um/include/asm/kmap_types.h b/arch/um/include/asm/kmap_types.h
index 6c03acd..2e0a6b1 100644
--- a/arch/um/include/asm/kmap_types.h
+++ b/arch/um/include/asm/kmap_types.h
@@ -8,22 +8,6 @@

/* No more #include "asm/arch/kmap_types.h" ! */

-enum km_type {
- KM_BOUNCE_READ,
- KM_SKB_SUNRPC_DATA,
- KM_SKB_DATA_SOFTIRQ,
- KM_USER0,
- KM_USER1,
- KM_UML_USERCOPY, /* UML specific, for copy_*_user - used in do_op_one_page */
- KM_BIO_SRC_IRQ,
- KM_BIO_DST_IRQ,
- KM_PTE0,
- KM_PTE1,
- KM_IRQ0,
- KM_IRQ1,
- KM_SOFTIRQ0,
- KM_SOFTIRQ1,
- KM_TYPE_NR
-};
+#define KM_TYPE_NR 14

#endif
--
1.7.7.6

2012-06-23 10:05:56

by Cong Wang

[permalink] [raw]
Subject: [PATCH 10/12] feature-removal-schedule.txt: remove kmap_atomic(page, km_type)

Signed-off-by: Cong Wang <[email protected]>
---
Documentation/feature-removal-schedule.txt | 8 --------
1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index 56000b3..2cd5202 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -523,14 +523,6 @@ Who: Sebastian Andrzej Siewior <[email protected]>

----------------------------

-What: kmap_atomic(page, km_type)
-When: 3.5
-Why: The old kmap_atomic() with two arguments is deprecated, we only
- keep it for backward compatibility for few cycles and then drop it.
-Who: Cong Wang <[email protected]>
-
-----------------------------
-
What: get_robust_list syscall
When: 2013
Why: There appear to be no production users of the get_robust_list syscall,
--
1.7.7.6

2012-06-23 10:05:54

by Cong Wang

[permalink] [raw]
Subject: [PATCH 09/12] highmem: remove the deprecated form of kmap_atomic

Signed-off-by: Cong Wang <[email protected]>
---
include/linux/highmem.h | 41 +----------------------------------------
1 files changed, 1 insertions(+), 40 deletions(-)

diff --git a/include/linux/highmem.h b/include/linux/highmem.h
index d3999b4..774fa47 100644
--- a/include/linux/highmem.h
+++ b/include/linux/highmem.h
@@ -110,54 +110,15 @@ static inline void kmap_atomic_idx_pop(void)
#endif

/*
- * NOTE:
- * kmap_atomic() and kunmap_atomic() with two arguments are deprecated.
- * We only keep them for backward compatibility, any usage of them
- * are now warned.
- */
-
-#define PASTE(a, b) a ## b
-#define PASTE2(a, b) PASTE(a, b)
-
-#define NARG_(_2, _1, n, ...) n
-#define NARG(...) NARG_(__VA_ARGS__, 2, 1, :)
-
-static inline void __deprecated *kmap_atomic_deprecated(struct page *page,
- enum km_type km)
-{
- return kmap_atomic(page);
-}
-
-#define kmap_atomic1(...) kmap_atomic(__VA_ARGS__)
-#define kmap_atomic2(...) kmap_atomic_deprecated(__VA_ARGS__)
-#define kmap_atomic(...) PASTE2(kmap_atomic, NARG(__VA_ARGS__)(__VA_ARGS__))
-
-static inline void __deprecated __kunmap_atomic_deprecated(void *addr,
- enum km_type km)
-{
- __kunmap_atomic(addr);
-}
-
-/*
* Prevent people trying to call kunmap_atomic() as if it were kunmap()
* kunmap_atomic() should get the return value of kmap_atomic, not the page.
*/
-#define kunmap_atomic_deprecated(addr, km) \
-do { \
- BUILD_BUG_ON(__same_type((addr), struct page *)); \
- __kunmap_atomic_deprecated(addr, km); \
-} while (0)
-
-#define kunmap_atomic_withcheck(addr) \
+#define kunmap_atomic(addr) \
do { \
BUILD_BUG_ON(__same_type((addr), struct page *)); \
__kunmap_atomic(addr); \
} while (0)

-#define kunmap_atomic1(...) kunmap_atomic_withcheck(__VA_ARGS__)
-#define kunmap_atomic2(...) kunmap_atomic_deprecated(__VA_ARGS__)
-#define kunmap_atomic(...) PASTE2(kunmap_atomic, NARG(__VA_ARGS__)(__VA_ARGS__))
-/**** End of C pre-processor tricks for deprecated macros ****/

/* when CONFIG_HIGHMEM is not set these will be plain clear/copy_page */
#ifndef clear_user_highpage
--
1.7.7.6

2012-06-23 10:05:53

by Cong Wang

[permalink] [raw]
Subject: [PATCH 08/12] tile: remove km_type definitions

Signed-off-by: Cong Wang <[email protected]>
---
arch/tile/include/asm/kmap_types.h | 26 --------------------------
1 files changed, 0 insertions(+), 26 deletions(-)

diff --git a/arch/tile/include/asm/kmap_types.h b/arch/tile/include/asm/kmap_types.h
index 3d0f202..054374c 100644
--- a/arch/tile/include/asm/kmap_types.h
+++ b/arch/tile/include/asm/kmap_types.h
@@ -27,31 +27,5 @@ enum km_type {
KM_TYPE_NR = 8
};

-/*
- * We provide dummy definitions of all the stray values that used to be
- * required for kmap_atomic() and no longer are.
- */
-enum {
- 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_SYNC_ICACHE,
- KM_SYNC_DCACHE,
- KM_UML_USERCOPY,
- KM_IRQ_PTE,
- KM_NMI,
- KM_NMI_PTE,
- KM_KDB
-};

#endif /* _ASM_TILE_KMAP_TYPES_H */
--
1.7.7.6

2012-06-23 10:05:52

by Cong Wang

[permalink] [raw]
Subject: [PATCH 06/12] asm-generic: remove km_type definitions

Signed-off-by: Cong Wang <[email protected]>
---
include/asm-generic/kmap_types.h | 34 ++--------------------------------
1 files changed, 2 insertions(+), 32 deletions(-)

diff --git a/include/asm-generic/kmap_types.h b/include/asm-generic/kmap_types.h
index 0232ccb..90f99c7 100644
--- a/include/asm-generic/kmap_types.h
+++ b/include/asm-generic/kmap_types.h
@@ -2,39 +2,9 @@
#define _ASM_GENERIC_KMAP_TYPES_H

#ifdef __WITH_KM_FENCE
-# define KMAP_D(n) __KM_FENCE_##n ,
+# define KM_TYPE_NR 41
#else
-# define KMAP_D(n)
+# define KM_TYPE_NR 20
#endif

-enum km_type {
-KMAP_D(0) KM_BOUNCE_READ,
-KMAP_D(1) KM_SKB_SUNRPC_DATA,
-KMAP_D(2) KM_SKB_DATA_SOFTIRQ,
-KMAP_D(3) KM_USER0,
-KMAP_D(4) KM_USER1,
-KMAP_D(5) KM_BIO_SRC_IRQ,
-KMAP_D(6) KM_BIO_DST_IRQ,
-KMAP_D(7) KM_PTE0,
-KMAP_D(8) KM_PTE1,
-KMAP_D(9) KM_IRQ0,
-KMAP_D(10) KM_IRQ1,
-KMAP_D(11) KM_SOFTIRQ0,
-KMAP_D(12) KM_SOFTIRQ1,
-KMAP_D(13) KM_SYNC_ICACHE,
-KMAP_D(14) KM_SYNC_DCACHE,
-/* UML specific, for copy_*_user - used in do_op_one_page */
-KMAP_D(15) KM_UML_USERCOPY,
-KMAP_D(16) KM_IRQ_PTE,
-KMAP_D(17) KM_NMI,
-KMAP_D(18) KM_NMI_PTE,
-KMAP_D(19) KM_KDB,
-/*
- * Remember to update debug_kmap_atomic() when adding new kmap types!
- */
-KMAP_D(20) KM_TYPE_NR
-};
-
-#undef KMAP_D
-
#endif
--
1.7.7.6

2012-06-23 10:05:50

by Cong Wang

[permalink] [raw]
Subject: [PATCH 05/12] avr32: remove km_type definitions

Signed-off-by: Cong Wang <[email protected]>
---
arch/avr32/include/asm/kmap_types.h | 24 ++----------------------
1 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/arch/avr32/include/asm/kmap_types.h b/arch/avr32/include/asm/kmap_types.h
index b7f5c68..479330b 100644
--- a/arch/avr32/include/asm/kmap_types.h
+++ b/arch/avr32/include/asm/kmap_types.h
@@ -2,29 +2,9 @@
#define __ASM_AVR32_KMAP_TYPES_H

#ifdef CONFIG_DEBUG_HIGHMEM
-# define D(n) __KM_FENCE_##n ,
+# define KM_TYPE_NR 29
#else
-# define D(n)
+# define KM_TYPE_NR 14
#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_PTE2,
-D(10) KM_IRQ0,
-D(11) KM_IRQ1,
-D(12) KM_SOFTIRQ0,
-D(13) KM_SOFTIRQ1,
-D(14) KM_TYPE_NR
-};
-
-#undef D
-
#endif /* __ASM_AVR32_KMAP_TYPES_H */
--
1.7.7.6

2012-06-23 10:05:49

by Cong Wang

[permalink] [raw]
Subject: [PATCH 03/12] powerpc: remove km_type definitions

Signed-off-by: Cong Wang <[email protected]>
---
arch/powerpc/include/asm/kmap_types.h | 31 +------------------------------
1 files changed, 1 insertions(+), 30 deletions(-)

diff --git a/arch/powerpc/include/asm/kmap_types.h b/arch/powerpc/include/asm/kmap_types.h
index bca8fdc..5acabbd 100644
--- a/arch/powerpc/include/asm/kmap_types.h
+++ b/arch/powerpc/include/asm/kmap_types.h
@@ -10,36 +10,7 @@
* 2 of the License, or (at your option) any later version.
*/

-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_PPC_SYNC_PAGE,
- KM_PPC_SYNC_ICACHE,
- KM_KDB,
- KM_TYPE_NR
-};
-
-/*
- * This is a temporary build fix that (so they say on lkml....) should no longer
- * be required after 2.6.33, because of changes planned to the kmap code.
- * Let's try to remove this cruft then.
- */
-#ifdef CONFIG_DEBUG_HIGHMEM
-#define KM_NMI (-1)
-#define KM_NMI_PTE (-1)
-#define KM_IRQ_PTE (-1)
-#endif
+#define KM_TYPE_NR 16

#endif /* __KERNEL__ */
#endif /* _ASM_POWERPC_KMAP_TYPES_H */
--
1.7.7.6

2012-06-23 10:05:47

by Cong Wang

[permalink] [raw]
Subject: [PATCH 04/12] frv: remove km_type definitions

Signed-off-by: Cong Wang <[email protected]>
---
arch/frv/include/asm/kmap_types.h | 24 +-----------------------
1 files changed, 1 insertions(+), 23 deletions(-)

diff --git a/arch/frv/include/asm/kmap_types.h b/arch/frv/include/asm/kmap_types.h
index f8e16b2..43901f2 100644
--- a/arch/frv/include/asm/kmap_types.h
+++ b/arch/frv/include/asm/kmap_types.h
@@ -2,28 +2,6 @@
#ifndef _ASM_KMAP_TYPES_H
#define _ASM_KMAP_TYPES_H

-enum km_type {
- /* arch specific kmaps - change the numbers attached to these at your peril */
- __KM_CACHE, /* cache flush page attachment point */
- __KM_PGD, /* current page directory */
- __KM_ITLB_PTD, /* current instruction TLB miss page table lookup */
- __KM_DTLB_PTD, /* current data TLB miss page table lookup */
-
- /* general kmaps */
- 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
-};
+#define KM_TYPE_NR 17

#endif
--
1.7.7.6

2012-06-23 10:07:54

by Cong Wang

[permalink] [raw]
Subject: [PATCH 11/12] vmalloc: remove KM_USER0 from comments

Signed-off-by: Cong Wang <[email protected]>
---
mm/vmalloc.c | 8 ++------
1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 2aad499..c7ac8e1 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1975,9 +1975,7 @@ static int aligned_vwrite(char *buf, char *addr, unsigned long count)
* IOREMAP area is treated as memory hole and no copy is done.
*
* If [addr...addr+count) doesn't includes any intersects with alive
- * vm_struct area, returns 0.
- * @buf should be kernel's buffer. Because this function uses KM_USER0,
- * the caller should guarantee KM_USER0 is not used.
+ * vm_struct area, returns 0. @buf should be kernel's buffer.
*
* Note: In usual ops, vread() is never necessary because the caller
* should know vmalloc() area is valid and can use memcpy().
@@ -2051,9 +2049,7 @@ finished:
* IOREMAP area is treated as memory hole and no copy is done.
*
* If [addr...addr+count) doesn't includes any intersects with alive
- * vm_struct area, returns 0.
- * @buf should be kernel's buffer. Because this function uses KM_USER0,
- * the caller should guarantee KM_USER0 is not used.
+ * vm_struct area, returns 0. @buf should be kernel's buffer.
*
* Note: In usual ops, vwrite() is never necessary because the caller
* should know vmalloc() area is valid and can use memcpy().
--
1.7.7.6

2012-06-23 10:08:16

by Cong Wang

[permalink] [raw]
Subject: [PATCH 12/12] pipe: remove KM_USER0 from comments

Signed-off-by: Cong Wang <[email protected]>
---
fs/pipe.c | 2 +-
include/linux/pipe_fs_i.h | 8 +++-----
2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index 49c1065..95cbd6b 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -224,7 +224,7 @@ static void anon_pipe_buf_release(struct pipe_inode_info *pipe,
* and the caller has to be careful not to fault before calling
* the unmap function.
*
- * Note that this function occupies KM_USER0 if @atomic != 0.
+ * Note that this function calls kmap_atomic() if @atomic != 0.
*/
void *generic_pipe_buf_map(struct pipe_inode_info *pipe,
struct pipe_buffer *buf, int atomic)
diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h
index e1ac1ce..e11d1c0 100644
--- a/include/linux/pipe_fs_i.h
+++ b/include/linux/pipe_fs_i.h
@@ -86,11 +86,9 @@ struct pipe_buf_operations {
* mapping or not. The atomic map is faster, however you can't take
* page faults before calling ->unmap() again. So if you need to eg
* access user data through copy_to/from_user(), then you must get
- * a non-atomic map. ->map() uses the KM_USER0 atomic slot for
- * atomic maps, so you can't map more than one pipe_buffer at once
- * and you have to be careful if mapping another page as source
- * or destination for a copy (IOW, it has to use something else
- * than KM_USER0).
+ * a non-atomic map. ->map() uses the kmap_atomic slot for
+ * atomic maps, you have to be careful if mapping another page as
+ * source or destination for a copy.
*/
void * (*map)(struct pipe_inode_info *, struct pipe_buffer *, int);

--
1.7.7.6

2012-06-23 10:05:43

by Cong Wang

[permalink] [raw]
Subject: [PATCH 01/12] jbd2: remove the second argument of kmap_atomic

Signed-off-by: Cong Wang <[email protected]>

---
fs/jbd2/commit.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index 216f429..af5280f 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -349,12 +349,12 @@ static void jbd2_block_tag_csum_set(journal_t *j, journal_block_tag_t *tag,
return;

sequence = cpu_to_be32(sequence);
- addr = kmap_atomic(page, KM_USER0);
+ addr = kmap_atomic(page);
csum = jbd2_chksum(j, j->j_csum_seed, (__u8 *)&sequence,
sizeof(sequence));
csum = jbd2_chksum(j, csum, addr + offset_in_page(bh->b_data),
bh->b_size);
- kunmap_atomic(addr, KM_USER0);
+ kunmap_atomic(addr);

tag->t_checksum = cpu_to_be32(csum);
}
--
1.7.7.6

2012-06-23 21:11:34

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 00/12] kmap_atomic cleanup for 3.6

On Saturday 23 June 2012, Cong Wang wrote:
> After few releases, it seems there are no more callers
> using the deprecated form of kmap_atomic(), the one
> with two parameters. So we can remove it now and remove
> the KM_* definition except KM_TYPE_NR together.
>
> All the patches are available at:
>
> git://github.com/congwang/linux.git #kmap_atomic
>

What is the significance of having an architecture-specific
definition for KM_TYPE_NR now? Should that be replaced
with a fixed value in include/linux/highmem.h so we can
remove the asm/kmap_types.h files entirely?

Arnd

2012-06-25 04:26:36

by Cong Wang

[permalink] [raw]
Subject: Re: [PATCH 00/12] kmap_atomic cleanup for 3.6

On Sat, 2012-06-23 at 21:11 +0000, Arnd Bergmann wrote:
> On Saturday 23 June 2012, Cong Wang wrote:
> > After few releases, it seems there are no more callers
> > using the deprecated form of kmap_atomic(), the one
> > with two parameters. So we can remove it now and remove
> > the KM_* definition except KM_TYPE_NR together.
> >
> > All the patches are available at:
> >
> > git://github.com/congwang/linux.git #kmap_atomic
> >
>
> What is the significance of having an architecture-specific
> definition for KM_TYPE_NR now? Should that be replaced
> with a fixed value in include/linux/highmem.h so we can
> remove the asm/kmap_types.h files entirely?
>

Different arch has different values for KM_TYPE_NR, I am not sure if
unifying them to a fixed value could fit all? For safety, I kept their
original values.

Thanks.

2012-06-25 15:18:18

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 00/12] kmap_atomic cleanup for 3.6

On Monday 25 June 2012, Cong Wang wrote:
> On Sat, 2012-06-23 at 21:11 +0000, Arnd Bergmann wrote:
> > On Saturday 23 June 2012, Cong Wang wrote:
> > > After few releases, it seems there are no more callers
> > > using the deprecated form of kmap_atomic(), the one
> > > with two parameters. So we can remove it now and remove
> > > the KM_* definition except KM_TYPE_NR together.
> > >
> > > All the patches are available at:
> > >
> > > git://github.com/congwang/linux.git #kmap_atomic
> > >
> >
> > What is the significance of having an architecture-specific
> > definition for KM_TYPE_NR now? Should that be replaced
> > with a fixed value in include/linux/highmem.h so we can
> > remove the asm/kmap_types.h files entirely?
> >
>
> Different arch has different values for KM_TYPE_NR, I am not sure if
> unifying them to a fixed value could fit all?

Well, that's something you could find out in the review.

> For safety, I kept their original values.

My fear is that it will make it harder to clean that code up for
real, when there is no longer an indication about where the number
comes from.

The only architecture that actually seems to have a restriction
here is tile, which defines it to 8.

I would suggest putting that value into include/linux/highmem.h,
it seems more than sufficient. I would structure the series to
have your patches 1 and 9 through 12 first, then remove
all explicit #include <asm/kmap_types.h> statements (you have
proven that they are not required already) and finally add the
define in linux/highmem.h and remove the arch specific headers.

Arnd

2012-06-25 17:35:42

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 00/12] kmap_atomic cleanup for 3.6

On Mon, 2012-06-25 at 15:18 +0000, Arnd Bergmann wrote:
> > Different arch has different values for KM_TYPE_NR, I am not sure if
> > unifying them to a fixed value could fit all?

No you can't. Some arch's have arch specific KM_TYPE thingies, like FRV.

> > For safety, I kept their original values.
>
> My fear is that it will make it harder to clean that code up for
> real, when there is no longer an indication about where the number
> comes from.

Agreed, I'd much prefer it if we'd come up with a sane way to compute
the max value before doing away with these enums.

Sadly I haven't been able to come up with a sane way short of whole
program analysis.

2012-06-25 20:43:59

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 00/12] kmap_atomic cleanup for 3.6

On Monday 25 June 2012, Peter Zijlstra wrote:
>
> On Mon, 2012-06-25 at 15:18 +0000, Arnd Bergmann wrote:
> > > Different arch has different values for KM_TYPE_NR, I am not sure if
> > > unifying them to a fixed value could fit all?
>
> No you can't. Some arch's have arch specific KM_TYPE thingies, like FRV.

Ah, right. Is it only FRV or are there any others?

> > > For safety, I kept their original values.
> >
> > My fear is that it will make it harder to clean that code up for
> > real, when there is no longer an indication about where the number
> > comes from.
>
> Agreed, I'd much prefer it if we'd come up with a sane way to compute
> the max value before doing away with these enums.
>
> Sadly I haven't been able to come up with a sane way short of whole
> program analysis.

How about putting that constant into asm/highmem.h then, and adding a
default like

#ifndef KM_TYPE_NR
#define KM_TYPE_NR 8
#endif

in linux/highmem.h? Then FRV and anything else that needs it can override
the value and the other ones don't need to bother.

Arnd

Subject: Re: [PATCH 05/12] avr32: remove km_type definitions

Around Sat 23 Jun 2012 18:04:16 +0800 or thereabout, Cong Wang wrote:
> Signed-off-by: Cong Wang <[email protected]>

Acked-by: Hans-Christian Egtvedt <[email protected]>

FYI: AVR32 doesn't use the KM_TYPE_NR symbol, so it can fallback to a common
define in include/linux/hugemem.h, ref. other discussions.

<snipp diff kmem_types.h cleanup>

--
mvh
Hans-Christian Egtvedt

2012-06-26 08:17:47

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 00/12] kmap_atomic cleanup for 3.6

On Mon, 2012-06-25 at 20:43 +0000, Arnd Bergmann wrote:
> On Monday 25 June 2012, Peter Zijlstra wrote:
> >
> > On Mon, 2012-06-25 at 15:18 +0000, Arnd Bergmann wrote:
> > > > Different arch has different values for KM_TYPE_NR, I am not sure if
> > > > unifying them to a fixed value could fit all?
> >
> > No you can't. Some arch's have arch specific KM_TYPE thingies, like FRV.
>
> Ah, right. Is it only FRV or are there any others?

FRV is the only one I can remember, but like always, just check all
archs.

> > > > For safety, I kept their original values.
> > >
> > > My fear is that it will make it harder to clean that code up for
> > > real, when there is no longer an indication about where the number
> > > comes from.
> >
> > Agreed, I'd much prefer it if we'd come up with a sane way to compute
> > the max value before doing away with these enums.
> >
> > Sadly I haven't been able to come up with a sane way short of whole
> > program analysis.
>
> How about putting that constant into asm/highmem.h then, and adding a
> default like
>
> #ifndef KM_TYPE_NR
> #define KM_TYPE_NR 8
> #endif
>
> in linux/highmem.h? Then FRV and anything else that needs it can override
> the value and the other ones don't need to bother.

At least put in a hand-wavy argument supporting whatever one number
that's being put in. That way a reader at least as some incling as to
where it comes from and what needs checking if it turns out its wrong.

2012-06-26 11:50:12

by Cong Wang

[permalink] [raw]
Subject: Re: [PATCH 00/12] kmap_atomic cleanup for 3.6

On Tue, 2012-06-26 at 10:17 +0200, Peter Zijlstra wrote:
> On Mon, 2012-06-25 at 20:43 +0000, Arnd Bergmann wrote:
> > How about putting that constant into asm/highmem.h then, and adding a
> > default like
> >
> > #ifndef KM_TYPE_NR
> > #define KM_TYPE_NR 8
> > #endif
> >
> > in linux/highmem.h? Then FRV and anything else that needs it can override
> > the value and the other ones don't need to bother.
>
> At least put in a hand-wavy argument supporting whatever one number
> that's being put in. That way a reader at least as some incling as to
> where it comes from and what needs checking if it turns out its wrong.

Ok, I will do it in a separated patch.

Thanks!

2012-06-26 17:48:48

by Chris Metcalf

[permalink] [raw]
Subject: Re: [PATCH 08/12] tile: remove km_type definitions

On 6/23/2012 6:04 AM, Cong Wang wrote:
> Signed-off-by: Cong Wang <[email protected]>
> ---
> arch/tile/include/asm/kmap_types.h | 26 --------------------------
> 1 files changed, 0 insertions(+), 26 deletions(-)

Acked-by: Chris Metcalf <[email protected]>

Looks like it almost works today, except for fs/jbd2/commit.c, at least
using the config I tried. But I see you have a change for that pending.

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


2012-06-26 20:25:17

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 04/12] frv: remove km_type definitions

On Sat, Jun 23, 2012 at 12:04 PM, Cong Wang <[email protected]> wrote:
> Signed-off-by: Cong Wang <[email protected]>
> ---
>  arch/frv/include/asm/kmap_types.h |   24 +-----------------------
>  1 files changed, 1 insertions(+), 23 deletions(-)
>
> diff --git a/arch/frv/include/asm/kmap_types.h b/arch/frv/include/asm/kmap_types.h
> index f8e16b2..43901f2 100644
> --- a/arch/frv/include/asm/kmap_types.h
> +++ b/arch/frv/include/asm/kmap_types.h
> @@ -2,28 +2,6 @@
>  #ifndef _ASM_KMAP_TYPES_H
>  #define _ASM_KMAP_TYPES_H
>
> -enum km_type {
> -       /* arch specific kmaps - change the numbers attached to these at your peril */
> -       __KM_CACHE,             /* cache flush page attachment point */
> -       __KM_PGD,               /* current page directory */
> -       __KM_ITLB_PTD,          /* current instruction TLB miss page table lookup */
> -       __KM_DTLB_PTD,          /* current data TLB miss page table lookup */
> -
> -       /* general kmaps */
> -        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
> -};
> +#define KM_TYPE_NR 17
>
>  #endif

"enum km_type" and "__KM_CACHE" are still used in
arch/frv/include/asm/highmem.h:

In file included from include/linux/highmem.h:34:0,
from include/linux/pagemap.h:10,
from include/linux/mempolicy.h:80,
from init/main.c:49:
arch/frv/include/asm/highmem.h:115:136: warning: 'enum km_type'
declared inside parameter list [enabled by default]
arch/frv/include/asm/highmem.h:115:136: warning: its scope is only
this definition or declaration, which is probably not what you want
[enabled by default]
arch/frv/include/asm/highmem.h:115:144: error: parameter 2 ('type')
has incomplete type
arch/frv/include/asm/highmem.h:115:92: warning: function declaration
isn't a prototype [-Wstrict-prototypes]
arch/frv/include/asm/highmem.h: In function 'kmap_atomic_primary':
arch/frv/include/asm/highmem.h:123:144: error: '__KM_CACHE' undeclared
(first use in this function)
arch/frv/include/asm/highmem.h:123:144: note: each undeclared
identifier is reported only once for each function it appears in
In file included from include/linux/highmem.h:34:0,
from include/linux/pagemap.h:10,
from include/linux/mempolicy.h:80,
from init/main.c:49:
arch/frv/include/asm/highmem.h: At top level:
arch/frv/include/asm/highmem.h:146:132: warning: 'enum km_type'
declared inside parameter list [enabled by default]
arch/frv/include/asm/highmem.h:146:140: error: parameter 2 ('type')
has incomplete type
arch/frv/include/asm/highmem.h:146:91: warning: function declaration
isn't a prototype [-Wstrict-prototypes]
arch/frv/include/asm/highmem.h: In function 'kunmap_atomic_primary':
arch/frv/include/asm/highmem.h:149:84: error: '__KM_CACHE' undeclared
(first use in this function)
make[1]: *** [init/main.o] Error 1

http://kisskb.ellerman.id.au/kisskb/buildresult/6592702/

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

2012-06-27 03:25:02

by Cong Wang

[permalink] [raw]
Subject: Re: [PATCH 04/12] frv: remove km_type definitions

On Tue, 2012-06-26 at 22:25 +0200, Geert Uytterhoeven wrote:
>
> "enum km_type" and "__KM_CACHE" are still used in
> arch/frv/include/asm/highmem.h:

Oops! I missed __KM_* types... Fortunately, the following patch could
probably fix it.

Thanks!

----------------------->

All callers of kmap_atomic_primary() use __KM_CACHE, so it can be
removed safely, and __kmap_atomic_primary() only check if 'type' if
__KM_CACHE or not, so 'type' can be changed to a boolean as well.

Ditto for kunmap_atomic_primary()/__kunmap_atomic_primary().


Attachments:
frv.diff (5.62 kB)

2012-06-27 07:32:59

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 04/12] frv: remove km_type definitions

On Wed, Jun 27, 2012 at 5:24 AM, Cong Wang <[email protected]> wrote:
> On Tue, 2012-06-26 at 22:25 +0200, Geert Uytterhoeven wrote:
>>
>> "enum km_type" and "__KM_CACHE" are still used in
>> arch/frv/include/asm/highmem.h:
>
> Oops! I missed __KM_* types... Fortunately, the following patch could
> probably fix it.
>
> Thanks!
>
> ----------------------->
>
> All callers of kmap_atomic_primary() use __KM_CACHE, so it can be
> removed safely, and __kmap_atomic_primary() only check if 'type' if

s/if$/is/

> __KM_CACHE or not, so 'type' can be changed to a boolean as well.
>
> Ditto for kunmap_atomic_primary()/__kunmap_atomic_primary().

Looks OK to me (don't have the hardware). Thx!

Acked-by: Geert Uytterhoeven <[email protected]>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds