2005-11-04 22:07:49

by Dave Jones

[permalink] [raw]
Subject: [PATCH] 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]>


--- linux-2.6.14/arch/ia64/kernel/setup.c~ 2005-11-04 17:05:23.000000000 -0500
+++ linux-2.6.14/arch/ia64/kernel/setup.c 2005-11-04 17:05:40.000000000 -0500
@@ -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];


2005-11-04 22:34:42

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] export ia64_max_cacheline_size

On Fri, Nov 04, 2005 at 05:07:37PM -0500, Dave Jones wrote:
> 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.

2005-11-04 22:55:35

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] export ia64_max_cacheline_size

Christoph Hellwig <[email protected]> wrote:
>
> On Fri, Nov 04, 2005 at 05:07:37PM -0500, Dave Jones wrote:
> > 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.

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

2005-11-04 23:53:05

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] export ia64_max_cacheline_size

On Fri, Nov 04, 2005 at 02:55:34PM -0800, Andrew Morton wrote:
> > Can we please move dma_get_cache_alignment() out of line instead?
>
> It's a single statement! It wants to be inlined.
>
> > 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.

2005-11-05 00:05:47

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] export ia64_max_cacheline_size

Christoph Hellwig <[email protected]> wrote:
>
> On Fri, Nov 04, 2005 at 02:55:34PM -0800, Andrew Morton wrote:
> > > Can we please move dma_get_cache_alignment() out of line instead?
> >
> > It's a single statement! It wants to be inlined.
> >
> > > 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.

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.

2005-11-05 04:38:19

by John W. Linville

[permalink] [raw]
Subject: [patch 2.6.14] 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(-)

diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index fc56ca2..3af6de3 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -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];
diff --git a/include/asm-ia64/dma-mapping.h b/include/asm-ia64/dma-mapping.h
index 6347c98..df67d40 100644
--- a/include/asm-ia64/dma-mapping.h
+++ b/include/asm-ia64/dma-mapping.h
@@ -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)
--
John W. Linville
[email protected]

2005-11-05 04:36:24

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH] export ia64_max_cacheline_size

On Fri, Nov 04, 2005 at 04:05:40PM -0800, Andrew Morton wrote:
> Christoph Hellwig <[email protected]> wrote:

> > 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
--
John W. Linville
[email protected]