Skip to contentSkip to conversation

mm/swap_state.c: unexport swapper_space

PATCH In review Adrian Bunk 4 participants 5 messages Mar 6, 2005 – Mar 8, 2005

Subsystems (2)
Memory management MM swap
lore
Share & tools
lore

Adrian Bunk

author
#0 lore raw

PATCH 1/1 mm/swap_state.c: unexport swapper_space

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

Signed-off-by Adrian Bunk <[email protected]>
mm/swap_state.c renamed from mm/swap_state.c.old +0 -1
@@ -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)
#1 lore raw

mm/swap_state.c line 44

Quoted 13 lines · Adrian Bunk

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

Signed-off-by Adrian Bunk <[email protected]>
mm/swap_state.c renamed from mm/swap_state.c.old +0 -1
@@ -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

#2 lore raw
Quoted 10 lines · Hugh Dickins

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.

#3 lore raw
Quoted 5 lines · Christoph Hellwig

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?

Signature · 3 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core
#4 lore raw
Quoted 8 lines · Russell King
Quoted 5 lines · Christoph Hellwig

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?

Quoted 1 line

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

Oh, I misunderstood you, sorree ;)

Quoted 2 lines

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