export ia64_max_cacheline_size

PATCH In review Dave Jones 4 participants 7 messages Nov 4, 2005 – Nov 5, 2005

Share & tools

Dave Jones

author

2005-11-04 22:07 UTC #0 lore raw

PATCH 1/1 export ia64_max_cacheline_size

on ia64, dma_get_cache_alignment() needs ia64_max_cacheline_size, which isn't exported. This breaks modular compilation of the b44 network driver (and possibly others).

Signed-off-by Dave Jones <[email protected]>

arch/ia64/kernel/setup.c renamed from arch/ia64/kernel/setup.c~ +1 -0

@@ -92,6 +92,7 @@ extern void efi_initialize_iomem_resourc
 extern char _text[], _end[], _etext[];
 
 unsigned long ia64_max_cacheline_size;
+EXPORT_SYMBOL(ia64_max_cacheline_size);
 unsigned long ia64_iobase;	/* virtual address for I/O accesses */
 EXPORT_SYMBOL(ia64_iobase);
 struct io_space io_space[MAX_IO_SPACES];

Christoph Hellwig

2005-11-04 22:34 UTC #1 lore raw

Quoted 3 lines · Dave Jones

on ia64, dma_get_cache_alignment() needs ia64_max_cacheline_size, which isn't exported. This breaks modular compilation of the b44 network driver (and possibly others).

Can we please move dma_get_cache_alignment() out of line instead? Always export sane APIs instead of random internals.

Andrew Morton

2005-11-04 22:55 UTC #2 lore raw

Quoted 7 lines · Christoph Hellwig

Quoted 3 lines · Dave Jones
on ia64, dma_get_cache_alignment() needs ia64_max_cacheline_size, which isn't exported. This breaks modular compilation of the b44 network driver (and possibly others).
Can we please move dma_get_cache_alignment() out of line instead?

It's a single statement! It wants to be inlined.

Quoted 1 line

Always export sane APIs instead of random internals.

The exported API is dma_get_cache_alignment(). For internal implementation reasons we need to export an ia64 symbol to modules to support it. That kinda sucks, but I don't think that we need to compromise kernel size and performance because of it.

Christoph Hellwig

2005-11-04 23:53 UTC #3 lore raw

Quoted 10 lines · Andrew Morton

Quoted 1 line
Can we please move dma_get_cache_alignment() out of line instead?
It's a single statement! It wants to be inlined.
Quoted 1 line
Always export sane APIs instead of random internals.
The exported API is dma_get_cache_alignment(). For internal implementation reasons we need to export an ia64 symbol to modules to support it. That kinda sucks, but I don't think that we need to compromise kernel size and performance because of it.

It's an API used only in slow pathes. It's much better to enforce modularity in that case.

Andrew Morton

2005-11-05 00:05 UTC #4 lore raw

Quoted 8 lines · Christoph Hellwig

Quoted 6 lines · Andrew Morton
Quoted 1 line
Can we please move dma_get_cache_alignment() out of line instead?
It's a single statement! It wants to be inlined.
Quoted 1 line
Always export sane APIs instead of random internals.

Quoted 7 lines

Quoted 4 lines
The exported API is dma_get_cache_alignment(). For internal implementation reasons we need to export an ia64 symbol to modules to support it. That kinda sucks, but I don't think that we need to compromise kernel size and performance because of it.
It's an API used only in slow pathes. It's much better to enforce modularity in that case.

hm, spose so. Putting it into .c means that all arches except one implement it under include/, which is also a bit irritating sometimes, such as $EDITOR include/asm-*/dma-mapping.h.

It's a 51%/49% decision, but I'm not sure which way.

John W. Linville

2005-11-05 04:36 UTC #5 lore raw

Quoted 10 lines · Andrew Morton

Quoted 2 lines · Christoph Hellwig
It's an API used only in slow pathes. It's much better to enforce modularity in that case.
hm, spose so. Putting it into .c means that all arches except one implement it under include/, which is also a bit irritating sometimes, such as $EDITOR include/asm-*/dma-mapping.h.
It's a 51%/49% decision, but I'm not sure which way.

I posted a patch for this mid-September. (Actually, I posted two. The first one was basically identical to davej's, and Christoph disliked it... :-)

	http://marc.theaimsgroup.com/?l=linux-kernel&m=112657055513967&w=2

I had hoped that it would get mainlined well before the b44 patch, but apparently it fell through the cracks. The patch still applies, so I'll follow-up with it after this message.

John

Signature · 2 lines

John W. Linville
[email protected]

John W. Linville

2005-11-05 04:38 UTC #6 lore raw

PATCH 1/1 ia64: re-implement dma_get_cache_alignment to avoid EXPORT_SYMBOL

The current ia64 implementation of dma_get_cache_alignment does not work for modules because it relies on a symbol which is not exported. Direct access to a global is a little ugly anyway, so this patch re-implements dma_get_cache_alignment in a manner similar to what is currently used for x86_64.

Signed-off-by John W. Linville <[email protected]>

---
This patch replaces the patch at the link below:

	http://www.ussg.iu.edu/hypermail/linux/kernel/0509.1/1365.html

 arch/ia64/kernel/setup.c       |    7 +++++++
 include/asm-ia64/dma-mapping.h |    7 +------
 2 files changed, 8 insertions(+), 6 deletions(-)

arch/ia64/kernel/setup.c +7 -0

index fc56ca2..3af6de3 100644
@@ -92,6 +92,13 @@ extern void efi_initialize_iomem_resourc
 extern char _text[], _end[], _etext[];
 
 unsigned long ia64_max_cacheline_size;
+
+int dma_get_cache_alignment(void)
+{
+        return ia64_max_cacheline_size;
+}
+EXPORT_SYMBOL(dma_get_cache_alignment);
+
 unsigned long ia64_iobase;	/* virtual address for I/O accesses */
 EXPORT_SYMBOL(ia64_iobase);
 struct io_space io_space[MAX_IO_SPACES];

include/asm-ia64/dma-mapping.h +1 -6

index 6347c98..df67d40 100644
@@ -48,12 +48,7 @@ dma_set_mask (struct device *dev, u64 ma
 	return 0;
 }
 
-static inline int
-dma_get_cache_alignment (void)
-{
-	extern int ia64_max_cacheline_size;
-	return ia64_max_cacheline_size;
-}
+extern int dma_get_cache_alignment(void);
 
 static inline void
 dma_cache_sync (void *vaddr, size_t size, enum dma_data_direction dir)

Signature · 2 lines

John W. Linville
[email protected]