[2.6 patch] mm/swap_state.c: unexport swapper_space

[2.6 patch] mm/swap_state.c: unexport swapper_space

[Adrian Bunk]

I didn't find any possible modular usage in the kernel.

Signed-off-by: Adrian Bunk <bunk@stusta.de>

--- linux-2.6.11-mm1-full/mm/swap_state.c.old	2005-03-04 16:25:54.000000000 +0100
+++ linux-2.6.11-mm1-full/mm/swap_state.c	2005-03-04 16:26:16.000000000 +0100
@@ -40,7 +40,6 @@
 	.i_mmap_nonlinear = LIST_HEAD_INIT(swapper_space.i_mmap_nonlinear),
 	.backing_dev_info = &swap_backing_dev_info,
 };
-EXPORT_SYMBOL(swapper_space);
 
 #define INC_CACHE_INFO(x)	do { swap_cache_info.x++; } while (0)
 

Re: [2.6 patch] mm/swap_state.c: unexport swapper_space

[Hugh Dickins]

On Sun, 6 Mar 2005, Adrian Bunk wrote:

> I didn't find any possible modular usage in the kernel.
> 
> Signed-off-by: Adrian Bunk <bunk@stusta.de>
> 
> --- linux-2.6.11-mm1-full/mm/swap_state.c.old	2005-03-04 16:25:54.000000000 +0100
> +++ linux-2.6.11-mm1-full/mm/swap_state.c	2005-03-04 16:26:16.000000000 +0100
> @@ -40,7 +40,6 @@
>  	.i_mmap_nonlinear = LIST_HEAD_INIT(swapper_space.i_mmap_nonlinear),
>  	.backing_dev_info = &swap_backing_dev_info,
>  };
> -EXPORT_SYMBOL(swapper_space);
>  
>  #define INC_CACHE_INFO(x)	do { swap_cache_info.x++; } while (0)

I think this one should stay exported (or at least be re-exported
immediately on demand).  It's used by the inline page_mapping() in
include/linux/mm.h, which _was_ used by various arch cacheflushing
inlines, which could reasonably be called from modular filesystems.

I think those architectures hit the missed export when the dependence
on &swapper_space got added to page_mapping(), the export was soon
added to mainline, but meanwhile they moved their inlines out-of-line
- perhaps temporarily, but not yet reverted.

Better leave it exported so long as page_mapping is using it.

Hugh

Re: [2.6 patch] mm/swap_state.c: unexport swapper_space

[Christoph Hellwig]

On Sun, Mar 06, 2005 at 03:28:19PM +0000, Hugh Dickins wrote:
> immediately on demand).  It's used by the inline page_mapping() in
> include/linux/mm.h, which _was_ used by various arch cacheflushing
> inlines, which could reasonably be called from modular filesystems.
> 
> I think those architectures hit the missed export when the dependence
> on &swapper_space got added to page_mapping(), the export was soon
> added to mainline, but meanwhile they moved their inlines out-of-line
> - perhaps temporarily, but not yet reverted.
> 
> Better leave it exported so long as page_mapping is using it.

I disagree.  swapper_state is far too much of an internal detail to be
exported.  I argued that way when page_mapping was changed to use it and
that's why the architectures moved their helpers out of line.
Looks like the exported unfortunately got added anyway although we settled
that discussion.

Re: [2.6 patch] mm/swap_state.c: unexport swapper_space

[Russell King]

On Sun, Mar 06, 2005 at 10:49:12PM +0000, Christoph Hellwig wrote:
> I disagree.  swapper_state is far too much of an internal detail to be
> exported.  I argued that way when page_mapping was changed to use it and
> that's why the architectures moved their helpers out of line.
> Looks like the exported unfortunately got added anyway although we settled
> that discussion.

Well, since ARM's usage of page_mapping() is out of line (which is
where it'll now stay) I think Christoph is correct.  Maybe this is
something which should be aired on linux-arch for the other arch
maintainers?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: [2.6 patch] mm/swap_state.c: unexport swapper_space

[Hugh Dickins]

On Tue, 8 Mar 2005, Russell King wrote:
> On Sun, Mar 06, 2005 at 10:49:12PM +0000, Christoph Hellwig wrote:
> > I disagree.  swapper_state is far too much of an internal detail to be
> > exported.  I argued that way when page_mapping was changed to use it and
> > that's why the architectures moved their helpers out of line.
> > Looks like the exported unfortunately got added anyway although we settled
> > that discussion.
> 
> Well, since ARM's usage of page_mapping() is out of line

ARM is out of line, again?

> (which is where it'll now stay) I think Christoph is correct.

Oh, I misunderstood you, sorree ;)

> Maybe this is something which should be aired on linux-arch
> for the other arch maintainers?

I've heard of that, I got the impression we're discouraged from
mailing it.  This is probably too minor to engage their attention.

Currently there is no arch using page_mapping in its header files,
presumably they were all forced out of line at that time.  I think
it's wrong to bump people into rearranging their code to get around
a missing export, but if you're happy with the status quo, so be it.

I expect Christoph and I would agree that what's really wrong is
for page_mapping to be referring to that strange implementation
detail swapper_space at all.  I'd say the export should remain so
long as the reference remains, to avoid post-release surprises
like last time; but I've had my say, que sera sera.

Hugh

[2.6 patch] mm/swap_state.c: unexport swapper_space

[Adrian Bunk]

I didn't find any possible modular usage in the kernel.

Signed-off-by: Adrian Bunk <bunk@stusta.de>

---

This patch was already sent on:
- 6 Mar 2005

--- linux-2.6.11-mm1-full/mm/swap_state.c.old	2005-03-04 16:25:54.000000000 +0100
+++ linux-2.6.11-mm1-full/mm/swap_state.c	2005-03-04 16:26:16.000000000 +0100
@@ -40,7 +40,6 @@
 	.i_mmap_nonlinear = LIST_HEAD_INIT(swapper_space.i_mmap_nonlinear),
 	.backing_dev_info = &swap_backing_dev_info,
 };
-EXPORT_SYMBOL(swapper_space);
 
 #define INC_CACHE_INFO(x)	do { swap_cache_info.x++; } while (0)