I think it would be good if you could specify a default namespace

per module, that could reduce the amount of necessary changes significantly.
For example, you can do
#define EXPORT_SYMBOL_GLOBAL(sym) __EXPORT_SYMBOL(sym, "_gpl",,, NULL)

#ifdef MODULE_NAMESPACE

#define EXPORT_SYMBOL_GPL(sym) EXPORT_SYMBOL_GLOBAL(sym)

#else

#define EXPORT_SYMBOL_GPL(sym) EXPORT_SYMBOL_NS(sym, MODULE_NAMESPACE)

#endif
If we go that way, it may be useful to extend the namespace mechanism to

non-GPL symbols as well, like
#define EXPORT_SYMBOL(sym) __EXPORT_SYMBOL(sym, "",__## MODULE_NAMESPACE, NS_SEPARATOR #MODULE_NAMESPACE, #MODULE_NAMESPACE)
Unfortunately, doing this automatic namespace selection requires to set

the namespace before #include <linux/module.h>. One way to work around this

could be to use Makefile magic so you can list a Makefile as
obj-$(CONFIG_COMBINED) += combined.o

combined-$(CONFIG_SUBOPTION) += combined_main.o combined_other.o

obj-$(CONFIG_SINGLE) += single.o

obj-$(CONFIG_OTHER) += other.o

obj-$(CONFIG_API) += api.o
NAMESPACE = subsys                       # default, used for other.o

NAMESPACE_single.o = single              # used only for single.o

NAMESPACE_combined.o = combined          # all parts of combined.o

NAMESPACE_combined_other.o = special     #    except this one

NAMESPACE_api.o =                        # api.o is put into the global ns
The Makefile logic here would basically just follow the rules we have for

CFLAGS etc, and then pass -DMODULE_NAMESPACE=$(NAMESPACE_$(obj)) to gcc.
	Arnd <><

-

But also give less documentation. It's also not that difficult to mark

the exports once. I've forward ported such patches over a few kernels
I would prefer to keep that inside the source files, again for

documentation purposes. One goal of namespace was to make something

that was previously kind of implicit explicit and the default name

spaces would work against that again I think.
-Andi
-

Part of your sentence seems to be missing, but I guess I understand your

point. How many files did you annotate this way? I can see it as being

useful to have the namespace explicit in each symbol, but doing it once

per module sounds like the 80% solution for 20% of the work, and the

two don't even conflict. In the current kernel, I count 12644 exported

symbols in 1646 files, in 540 directories.
One problem I can see with annotating every symbol is that it conflicts

with other patches that add more exported functions to a file without

adding the namespace, or that simply break because of context changes.
	Arnd <><

-

> This patch allows to export symbols only for specific modules by

 > introducing symbol name spaces. A module name space has a white

 > list of modules that are allowed to import symbols for it; all others

 > can't use the symbols.

 >

 > It adds two new macros:

 >

 > MODULE_NAMESPACE_ALLOW(namespace, module);
I definitely like the idea of organizing exported symbols into

namespaces.  However, I feel like it would make more sense to have

something like
MODULE_NAMESPACE_IMPORT(namespace);
inside the modules that use a namespace.  This matches the way C

preprocessor #includes, C++ namespaces, perl "use", etc. works and so

it is probably closer to how programmers think.  It does weaken the

enforcement of review a little bit, since there are no changes to the

site where things are exported to catch, but git makes it easy to

sneak that kind of change in anyway.
The practical benefits are that you don't end up with stupid patch

conflicts caused by one patch adding MODULE_NAMESPACE_ALLOW() for

module "foo" and another patch adding it for module "bar" at the same

place, and that it becomes simpler for people to test or deliver, say,

a new TCP congestion module without having to rebuild the whole kernel

(which is an especially huge pain for distro kernels).
In any case I would make use of this in whatever form it gets merged

in -- Mellanox ConnectX hardware can operate simultaneously as an

InfiniBand adapter and a 10Gb ethernet NIC, and so my driver is split

up into a low-level mlx4_core driver that exports symbols for other

mlx4 drivers to use; clearly it only makes sense to export them to

other parts of the driver, and in this case the difference between

"ALLOW" and "IMPORT" semantics is not a big deal.
 - R.

-

Except C doesn't have namespaces and this mechanism doesn't create them.  So

this is just complete and utter makework; as I said before, noone's going to

confuse all those udp_* functions if they're not in the udp namespace.
For better or worse, this is not C++.
Rusty.

-

On Mon, 26 Nov 2007 12:28:14 +1100
Agreed. On first glance, I was intrigued but:
1) Why is everyone so concerned that export symbol space is large?

	- does it cost cpu or running memory?

	- does it cause bugs?

	- or are you just worried about "evil modules"?
2) These aren't real namespaces

	- all global names still have to be unique

	- still have to handle the "non-modular build" namespace conflicts

	- there isn't a big problem with conflicting symbols today.
So why bother adding complexity.

--

Stephen Hemminger <shemminger@linux-foundation.org>

-

yes, bad apis are causing bugs... sys_open is just the starter of that.
--

If you want to reach me at my work email, use arjan@linux.intel.com

For development, discussion and tips for power savings,

visit http://www.lesswatts.org

-

Sure, but this doesn't change the APIs, either.  We seem to have fixed

sys_open the right way, and since we're not supposed to care about

out-of-tree modules...
Rusty.

-

On Mon, Nov 26, 2007 at 10:25:33AM -0800, Stephen Hemminger wrote:
 > 1) Why is everyone so concerned that export symbol space is large?

 > 	- does it cost cpu or running memory?

 > 	- does it cause bugs?

 > 	- or are you just worried about "evil modules"?
To clarify something here, by "evil", don't necessarily think "binary only". 
Out of tree modules are frequently using symbols that they shouldn't be.

Because they get no peer-review here, they 'get away with it' for the most part.

Until distro vendors push rebased kernel updates that removed exports that

should never have been exported, and suddenly people like me get bombed

with "Fedora broke my xyz driver" mails.
Reducing the opportunity for people to screw things up is a good thing.

If a symbol is exported, most out-of-tree driver authors seem to

think its "fair game" to use it.
	Dave
--

http://www.codemonkey.org.uk

-

The real problem is that these drivers are not in the upstream kernel.
cu

Adrian
-- 
       "Is there not promise of rain?" Ling Tan asked suddenly out

        of the darkness. There had been need of rain for many days.

       "Only a promise," Lao Er said.

                                       Pearl S. Buck - Dragon Seed
-

Well, on my laptop, I'm currently dragging along 3 out-of-tree kernel modules.

2 are well-known binary blobs so it's between me and the vendor, as usual.
The third is a USB webcam driver that happened to get caught at the wrong

end of the colorspace-conversion-in-kernel bunfight in the V4L playpen.

Somebody wants to figure out how to get the gspca drivers into the kernel,

they're at http://mxhaard.free.fr/download.html waiting for attention. ;)
(Don't look at *me*, I don't understand the code, or the bunfight - I just

happen to have one of the 244 supported webcams, and it works with that driver)

On Tue, Nov 27, 2007 at 10:09:42PM +0100, Adrian Bunk wrote:

 > On Tue, Nov 27, 2007 at 02:00:37PM -0500, Dave Jones wrote:

 > > On Mon, Nov 26, 2007 at 10:25:33AM -0800, Stephen Hemminger wrote:

 > >

 > >  > 1) Why is everyone so concerned that export symbol space is large?

 > >  > 	- does it cost cpu or running memory?

 > >  > 	- does it cause bugs?

 > >  > 	- or are you just worried about "evil modules"?

 > >

 > > To clarify something here, by "evil", don't necessarily think "binary only".

 > >

 > > Out of tree modules are frequently using symbols that they shouldn't be.

 > > Because they get no peer-review here, they 'get away with it' for the most part.

 > > Until distro vendors push rebased kernel updates that removed exports that

 > > should never have been exported, and suddenly people like me get bombed

 > > with "Fedora broke my xyz driver" mails.

 > >...

 >

 > The real problem is that these drivers are not in the upstream kernel.
You're preaching to the choir.
 > Are there common reasons why these drivers are not upstream?
It varies case by case.
	Dave
--

http://www.codemonkey.org.uk

-

One might be that upstream has not accepted them.  Anything doing or

smelling of TOE comes to mind right away.
rick jones

-

Which modules doing or smelling of TOE do work with unmodified vendor

kernels?
AFAIR TOE was both not a module and required some hooks in the network 
cu

Adrian
-- 
       "Is there not promise of rain?" Ling Tan asked suddenly out

        of the darkness. There had been need of rain for many days.

       "Only a promise," Lao Er said.

                                       Pearl S. Buck - Dragon Seed
-

At the very real risk of further demonstrating my Linux vocabulary

limitations, I believe there is a "Linux Sockets Acceleration"

module/whatnot for NetXen and related 10G NICs, and a cxgb3_toe (?)

module for Chelsio 10G NICs.
rick jones

-

> Agreed. On first glance, I was intrigued but:

 >

 > 1) Why is everyone so concerned that export symbol space is large?

 > 	- does it cost cpu or running memory?

 > 	- does it cause bugs?

 > 	- or are you just worried about "evil modules"?

 >

 > 2) These aren't real namespaces

 > 	- all global names still have to be unique

 > 	- still have to handle the "non-modular build" namespace conflicts

 > 	- there isn't a big problem with conflicting symbols today.
Perhaps changing the name from "namespace" to "interface" would help?

Then a module could have something like
MODULE_USE_INTERFACE(foo);
and I think that makes it clearer what the advantage of this is: it

marks symbols as being part of a certain interface, requires modules

that use that interface to declare that use explicitly, and allows

reviewers to say "Hey why is this code using the scsi interface when

it's a webcam driver?"
 - R.

-

> Except C doesn't have namespaces and this mechanism doesn't create them.  So

 > this is just complete and utter makework; as I said before, noone's going to

 > confuse all those udp_* functions if they're not in the udp namespace.
I don't understand why you're so opposed to organizing the kernel's

exported symbols in a more self-documenting way.  It seems pretty

clear to me that having a mechanism that requires modules to make

explicit which (semi-)internal APIs makes reviewing easier, makes it

easier to communicate "please don't use that API" to module authors,

and takes at least a small step towards bringing the kernel's exported

API under control.  What's the real downside?
 - R.

-

No, I was the one who moved exports near their declarations.  That's

organised.  I just don't see how this new "organization" will help: oh good, 
Perhaps you've got lots of patches were people are using internal APIs they 
Well, introduce an EXPORT_SYMBOL_INTERNAL().  It's a lot less code.  But you'd 
There is no "exported API" to bring under control.  There are symbols we

expose for the kernel's own use which can be used by external modules at 
No.  That's the wrong question.  What's the real upside?
Let's not put code in the core because "it doesn't seem to hurt".
I'm sure you think there's a real problem, but I'm still waiting for someone

to *show* it to me.  Then we can look at solutions.
Rusty.

-

With my "Enterprise" hat on, I can see where Andi was coming from

originally. Just like we do in RHEL, SuSE have a concept of a kernel ABI

and we too have a concept of a whitelist of symbols - a subset of kernel

ABI that is approved for use by third parties (this is nothing to do

with licensing, this is to do with ABI stability we try to ensure).
As part of figuring out what should and should not be used by external

modules (outside of the core kernel), we've gained a bit of experience,

and it would be nice to be able to help others - this is nothing to do

with upstream "ABI stability", but just to be of a public service by

documenting those functions that are never intended for modules but
I suggested this exact idea privately at OLS. I still think it's the

best compromise, though I like bits of the namespace idea. An

EXPORT_SYMBOL_INTERNAL would indeed be trivial.
Jon.
-

For the record my original motivation was to fix the "TCP exports everything

for ipv6.ko" case cleanly. I later realized that it would be useful for the

ABI stability issues too, but it was really not my primary motivation.

This is why I didn't even mention that in the original patch description.
-Andi
-

On Tue, 27 Nov 2007 23:37:43 +0100
Since ipv6 can never be removed because it references itself, the whole concept

of a modular ipv6 is flawed. Sometime ago a patch was sent to remove the ipv6 module

unload hooks but as I recall Dave rejected it believing the unload code could be

fixed.

--

Stephen Hemminger <shemminger@linux-foundation.org>

-

AFAIK that is obsolete anyways. It was done because it was feared it would

be broken, but at least the broken cases I knew about were all fixed
Modules that cannot be unloaded are still useful. Standard case: Distributions

like to offer an option to not use ipv6 because that is popular workaround

for the common "DNS server eats AAAA queries and causes delays" issue.

Forcing the user to rebuild the kernel for this wouldn't be practical.

If ipv6 wasn't modular that would be hard to do.
-Andi

-

cu

Adrian
-- 
       "Is there not promise of rain?" Ling Tan asked suddenly out

        of the darkness. There had been need of rain for many days.

       "Only a promise," Lao Er said.

                                       Pearl S. Buck - Dragon Seed
-

They safe also a few hundred KB of memory this way.

I know it is not en vogue anymore to care about memory bloat, but

I personally like that.
-Andi 
-

Might the recent discussion on the exporting of sys_open() and

sys_read() be an example here?  There would appear to be a consensus

that people should not have used those functions, but they are now

proving difficult to unexport.
Perhaps the best use of the namespace stuff might be for *future*

exports which are needed to help make the mainline kernel fully modular,

but which are not meant to be part of a more widely-used API?
jon

-

They're not.  The exports aren't used anymore and could just go away

any time.
-

Not sure about future only, but yes that is its intention.

Thanks for putting it clearly.
-Andi

-

Maybe the issue is "who can tell" since what is external and what is
Explicitly documenting what comprises the kernel API (external,

supported) and what comprises the kernel implementation (internal, not
I think the benefits should include:
- forcing developers to identify their exports as part of the

implementation or as part of the kernel API
- making it easier for reviewers to identify when developers are adding

to the kernel API and thereby focusing the appropriate level of review

to the new function
- making it obvious to developers when they are binding their
-

See, there's your problem.  All interfaces can, and will, change.  You're

always binding yourself to a particular release.
So you're not proposing we mark what's not stable, you're arguing that we

create a subset which is stable.
That's an argument we're not (yet) having.
Cheers,

Rusty.

-

Absolutely in the limit. But there are many bits of code that work quite

nicely from release to release because they use services that live in the

smooth water in the wake of the Linux head.
I think defining that smooth water has merit. I also think that it would be

nice to limit the scope of module externs to avoid polluting the global

namespace. I'm not sure that this particular patch reaches these goals, but
Well, this is an interesting question. The answer is I think both are

important. It would be nice (and arguably necessary long term) to limit the

scope of externs. This can be accomplished with name spaces "I want bob's

implementation of read."
I think it also has value to define interfaces that are considered stable

(but not inviolate) to allow developers to make better informed decisions

when choosing interfaces. Having this info explicit in the code seems
Yeah, maybe I'm off in the weeds on this one...
-

There is not, never was, and never will be, any supported external API

of the kernel.
cu

Adrian
-- 
       "Is there not promise of rain?" Ling Tan asked suddenly out

        of the darkness. There had been need of rain for many days.

       "Only a promise," Lao Er said.

                                       Pearl S. Buck - Dragon Seed
-

Philosophically I understand what you're saying, but in practical terms

there is the issue of managing core API like kmalloc. Although kmalloc

_could_ change, doing so would be extremely painful. In fact anyone who

proposed such a change would have to have a profoundly powerful argument

as to why it was necessary.
I think this patchset is an attempt to make it easier to identify and
-

The latter should at least in theory be required for all changes no 
As long as the submitter fixes all in-kernel users these interfaces are

not handled differently from interfaces with fewer users.
And I remember at least one commit that changed > 1000 files because it

changed a frequently used driver API. [1]
cu

Adrian
[1] commit 7d12e780e003f93433d49ce78cfedf4b4c52adc5
-- 
       "Is there not promise of rain?" Ling Tan asked suddenly out

        of the darkness. There had been need of rain for many days.

       "Only a promise," Lao Er said.

                                       Pearl S. Buck - Dragon Seed
-

Exactly.  Or rather it is not defined on the module level. We got

"static" of course, but I think we should have a similar mechanism
It would not be fully supported either -- can still change etc. --

but there is a reasonable expectation that those external
That is EXPORT_SYMBOL already. The trouble is just that it covers

too much. My patchkit is trying to limit it again for a specific

use case -- exporting an "internal" interface to another module.

Or rather a set of modules. 
Standard example is TCP: TCP exports nearly everything and the

single user is the TCP code in ipv6.ko. Instead those symbols should

be limited to be only accessable to ipv6.ko. 
The reason I went with the more generic namespace mechanism

instead of EXPORT_SYMBOL_TO() is that ipv6 is ever split up

it would still work. 
Also using namespaces doesn't have any more overhead than

EXPORT_SYMBOL_TO() and the complexity is about the same
That is another reason.
-ANdi

-

Let's forget about external modules that are anyway irrelevant for

upstream kernel development.
Do you have past examples where this would have brought advantages

for the upstream kernel justifying all the work required for creating

and maintaining these namespaces?
IOW, where modules were submitted for upstream inclusion and merging

them was impossible or much harder only because they were developed 
cu

Adrian
-- 
       "Is there not promise of rain?" Ling Tan asked suddenly out

        of the darkness. There had been need of rain for many days.

       "Only a promise," Lao Er said.

                                       Pearl S. Buck - Dragon Seed
-

Thank you for doing this.
Creating the DCCP and its congestion control infrastructure (CCID)

module namespaces is now on my TODO list. :-)
- Arnaldo

-

My original patchkit had DCCP actually done, but I ran into some problem

while forward porting and disabled it again. But should be reasonably

easy to resurrect.
-Andi

-

Very nice, looking forward to organize the exports mess a bit more.

-

I would need people to help me converting more subsystems to this new scheme.
In particular all exports that are only used by a single module are direct

candidates for a namespace. For the others it has to be checked by someone

who knows the particular subsystem well.
e.g. for SCSI it would be nice to put the symbols only used by sd/sr/sg etc.

into a namespace.
-Andi

-

True.  I'll take care of the scsi bits once this goes in.

-

Hi Andi,
    This is an interesting idea, thanks for the code!  My only question is

whether we can get most of this benefit by dropping the indirection of

namespaces and have something like "EXPORT_SYMBOL_TO(sym, modname)"?  It

doesn't work so well for exporting to a group of modules, but that seems

a reasonable line to draw anyway.
Cheers,

Rusty.

PS.  Probably better to use the standard warnx and errx in modpost, too.

-

That would explode quickly already even for my example "inet" namespace.

It already has several modules.  I don't think so much duplication would be a

good idea.
-Andi

-

Yes, and if a symbol is already used by multiple modules, it's generically

useful.  And if so, why restrict it to in-tree modules?
If your real intent is to bias against out-of-tree modules, let's just

generate a list of in-tree module names, and restrict some or all exports to

that set.
Rusty.

-

If it is so clear, you should be able to easily provide examples?
Rusty.
-

> > I agree that we shouldn't make things too hard for out-of-tree

 > > modules, but I disagree with your first statement: there clearly is a

 > > large class of symbols that are used by multiple modules but which are

 > > not generically useful -- they are only useful by a certain small class

 > > of modules.

 >

 > If it is so clear, you should be able to easily provide examples?
Sure -- Andi's example of symbols required only by TCP congestion

modules; the SCSI internals that Christoph wants to mark; the symbols

exported by my mlx4_core driver (which I admit are currently only used

by the mlx4_ib driver, but which will also be used by at least the

ethernet NIC driver for the same hardware).  I thought this was

already covered repeatedly in the thread and indeed in Andi's code so

there was no need to repeat it...

-

Right.  So presumably there will only ever be two drivers using this core

code, so no new users will ever be written?  Now we've found one use case, is

it worth the complexity of namespaces?  Is it worth the halfway point of

export-to-module?
No, we've seen the solution and various people applying it.  I'm still trying

to discover the problem it's solving.
Hope that helps,

Rusty.

-

Agreed the congestion modules are a corner case. I even mentioned that in the

patch. I would be happy to drop that one if that is the consensus.

It was more done as a example anyways. That is why I made it an separate

namespace from "tcp"
But for many other TCP symbols it makes a lot of sense: all the functions

only used by tcp_ipv6.c. If someone wants to write support for a "IPv7" or

similar they really should do it in tree. So I think the "tcp" and  "inet"
If there are new users they will need to get proper review and should
Again for rusty @)
Goals are:

- Limit the interfaces available for out of tree modules to reasonably

stable ones that are already used by a larger set of drivers.
This can also have further downstream advantages.

For example it might be a reasonable future rule to require all unconditionally

EXPORT_SYMBOL()s to have a complete LinuxDoc documentation entry.
- Explicitely declare in source what is clearly internal and not intended to

be a generally usable interface.
e.g. for the LinuxDoc example above such internal functions don't necessarily

need full LinuxDoc documentation.
- Force review from core maintainers for use of such internal interfaces
- Limit size of exported API to make stable ABIs for enterprise

distributions easier

[Yes I know that is not a popular topic on l-k, but it's a day-to-day

problem for these distros and out of tree solutions do not work]
-Andi

-

That's a real problem, and I sympathise with the idea of marking symbols as

externally useful (or, practically, mark internal).
But we now need to decide what's "externally useful".  The currently line for

exports is simple: someone in-tree needs it.  You dislike the suggestion to

extend this to "if more than one in-tree needs it it's open".
Currently your criterion seems to be "does the maintainer hate external

modules?" which I don't think will be what you want...
Cheers,

Rusty.

-

OK, short of making IPv4 a module (which would be a worthy task :)

do you have an example where a symbol is used by more than one module

but needs to be put into a namespace?
Thanks,

--

Visit Openswan at http://www.openswan.org/

Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>

Home Page: http://gondor.apana.org.au/~herbert/

PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

-

> OK, short of making IPv4 a module (which would be a worthy task :)
At some point there were patches, it is probably not very difficult.

But DaveM resisted at some point because he didn't want people

to replace the network stack (although I personally don't have a problem
For SCSI: SD,SR,SG etc.

For Networking: e.g. symbols i put into inet, which are only

used by protocols (sctp, dccp, udplite, ipv6)

I already caught someone doing something wrong with that BTW --

wanrouter clearly does some things it shouldn't be doing.

Or the fib namespace, where all the fib functions should be only

used by the two fib_* modules and ipv6/decnet.

For KVM: the exports used by kvm_amd/intel

For arch/x86: e.g. e820_*, IO_APIC_get_PCI_irq_vector, swiotlb,

a lot of stuff only used by acpi processor.c, pcibios*, etc.etc.

Roland gave another example for Infiniband.
Also in general it allows flexibility later if modules get split.
-Andi

-

Personally, I wouldn't find replacing the ipv4 stack very interesting.
However, stress-testing your system for IPv6-readiness by doing 'rmmod ipv4' :)
(Though I admit it's something I'd *try* if it was available, but certainly

not sufficient for a reason to do it...)

Wait, that's exactly Rusty's point (I think :)
These symbols are exported because they're needed by protocols.

If they weren't available to everyone then it would be difficult
Again, if it's used by decnet then it sounds like it should be

exported because new protocol families may need them.
So based on the network code at least I'm kind of starting to

agree with Rusty now: if a symbol is needed by more than one

in-tree module chances are we want it to be exported for all.
Although I admit I haven't examined your examples elsewhere.
Cheers,

--

Visit Openswan at http://www.openswan.org/

Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>

Home Page: http://gondor.apana.org.au/~herbert/

PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

-

I'd say exporting to a group of modules is the main use case.  E.g. in

scsi there would be symbols exported to transport class modules only

or lots of the vfs_ symbols would be exported only to stackable filesystems

or nfsd.
-

That's my point.  If there's a whole class of modules which can use a symbol,

why are we ruling out external modules?  If that's what you want, why not

have a list of permitted modules compiled into the kernel and allow no

others?
Cheers,

Rusty.

-

The point is to get cleaner interfaces. Anything which is kind of internal

should only be used by closely related in tree modules which can be updated.

Point of is not to be some kind of license enforcer or similar, there

are already other mechanisms for that. Just to get the set of really

public kernel interfaces down to a manageable level.
But I still think exporting only to a single module would be to limiting

for this case even. It would work for the TCP<->ipv6.ko post child,
That would not make the relationship explicit, which would not further

the goal.
-Andi

-

But this doesn't change interfaces at all.  It makes modules fail to load 
Is there evidence that this is a problem for us?  Are there any interfaces 
Why do we care what a "really public"?  We treat them all the same, as

changeable interfaces.  ie.  None of them are "really public".
For example, you put all the udp functions in the "udp" namespace.  But what

have we gained?  What has become easier to maintain?  All those function

start with "udp_": are people having trouble telling what they're for?
If you really want to reduce "public interfaces" then it's much simpler to

mark explicitly what out-of-tree modules can use.  We can have a list of

symbol names in include/linux/public-exports.h.
I just don't see what problems this separation solves.

Rusty.

-

The modules wouldn't be using the internal interfaces in the first

place with name spaces in place. This serves as a documentation

on what is considered internal. And if some obscure module (in or

out of tree) wants to use an internal interface they first have

to send the module maintainer a patch and get some review this way.
I believe that is fairly important in tree too because the

kernel has become so big now that review cannot be the only

enforcement mechanism for this anymore.
Another secondary reason is that there are too many exported interfaces

in general. Several distributions have policies that require to

keep the changes to these exported interfaces minimal and that

is very hard with thousands of exported symbol.  With name spaces

the number of truly publicly exported symbols will hopefully

shrink to a much smaller, more manageable set.
-Andi

-

So, you're saying that there's a problem with in-tree modules using symbols 
If people aren't reviewing, this won't make them review.  I don't think the 
*This* makes sense.  But it's not clear that the burden should be placed on

kernel coders.  You can create a list yourself.  How do I tell the difference

between "truly publicly exported" symbols and others?
If a symbol has more than one in-tree user, it's hard to argue against an

out-of-tree module using the symbol, unless you're arguing against *all*

out-of-tree modules.
Sorry,

Rusty.

-

With millions of LOC the primary maintainers cannot review everything.

It's not that anybody is doing a bad job -- it is just so much code
No of course not -- it is just too much code to let everything

be reviewed by the core subsystem maintainers. But with explicit

marking of internal symbols they would need to look at it because
Out of tree solutions generally do not scale.  Nobody else can 
There are still classes of drivers. e.g. for the SCSI example: SD,SG,SR etc.

are more internal while low level drivers like aic7xxx are clearly external
No, actually namespaces kind of help out of tree modules. Once they only

use interfaces that are really generic driver interfaces and fairly stable

their authors will have much less pain forward porting to newer kernel

version. But currently the authors cannot even know what is an instable

internal interface and what is a generic relatively stable driver level

interface. Namespaces are a mechanism to make this all explicit.
-Andi

-

No, a one-line patch adding the module to the set is all they'd see.  There's 
Then mark those symbols internal and only allow concurrently-built modules to

access them.  That's simpler and requires much less maintenance than your 
So in your head you have a notion of a kernel API, and you're trying to make

that API explicit in the code.
Sorry, but no.

Rusty.

-

Andy, I like your idea.  IMHO, as Rusty said a simple EXPORT_SYMBOL_TO

is better.
And I wonder if it is possible to export to something like  the struct

-

Not sure I follow you. Can you expand? 
-Andi

-

I know little about module internal, so if I'm wrong please just don't

mind, please  point out or just ignore.
Kernel symbols could apply to kernel object model, doesn't it?

I just think that because the device_driver have a mod_name member

(for built-in module), so if something can be done as device driver is

registered.

-

On Thu, Nov 22, 2007 at 03:43:06AM +0100, Andi Kleen wrote:
 > There seems to be rough consensus that the kernel currently has too many

 > exported symbols. A lot of these exports are generally usable utility

 > functions or important driver interfaces; but another large part are functions

 > intended by only one or two very specific modules for a very specific purpose.

 > One example is the TCP code. It has most of its internals exported, but

 > only for use by tcp_ipv6.c (and now a few more by the TCP/IP congestion modules)

 > But it doesn't make sense to include these exported for a specific module

 > functions into a broader "kernel interface".   External modules assume

 > they can use these functions, but they were never intended for that.

 >

 > This patch allows to export symbols only for specific modules by

 > introducing symbol name spaces. A module name space has a white

 > list of modules that are allowed to import symbols for it; all others

 > can't use the symbols.
I really like this patchset.   Definitely a step in the right direction imo.

Looks like some nits there that checkpatch will probably pick up on,

but otherwise, looks very straightforward too.
Kudos.
	Dave
--

http://www.codemonkey.org.uk

-

On Thu, 22 Nov 2007 03:43:06 +0100 (CET)
Hi,
I like this concept in general; I have one minor comment; right now

your namespace argument is like
EXPORT_SYMBOL_NS(foo, some_symbol);
from a language-like pov I kinda wonder if it's nicer to do
EXPORT_SYMBOL_NS("foo", some_symbol);
because foo isn't something in C scope, but more a string-like

identifier...
--

If you want to reach me at my work email, use arjan@linux.intel.com

For development, discussion and tips for power savings,

visit http://www.lesswatts.org

-

That wouldn't work for MODULE_ALLOW() because it appends the namespace

to other identifiers. I don't know of a way in the C processor to get

back from a string to a ## concatenable identifier.
For EXPORT_SYMBOL_NS it would be in theory possible, but making

it asymmetric to MODULE_ALLOW would be ugly imho.
-Andi

-

I defined two namespaces: tcp for TCP internals which are only used by

tcp_ipv6.ko And tcpcong for exports used by the TCP congestion modules
No need to export any TCP internals to anybody else. So express this in a namespace.
I admit I'm not 100% sure tcpcong makes sense -- there might be a legitimate

need to have external out of tree congestion modules. They seem nearly like

drivers, but only nearly. If that was deemed the case it would be possible to

remove tcpcong again to allow everybody to access this.
This implicitely turns all exports into GPL only, but that won't matter

because all modules allowed to import TCP functions are GPLed.
---

 net/ipv4/tcp.c           |   71 +++++++++++++++++++++++++++--------------------

 net/ipv4/tcp_cong.c      |   14 ++++-----

 net/ipv4/tcp_input.c     |   12 +++----

 net/ipv4/tcp_ipv4.c      |   38 ++++++++++++-------------

 net/ipv4/tcp_minisocks.c |   12 +++----

 net/ipv4/tcp_output.c    |   12 +++----

 net/ipv4/tcp_timer.c     |    2 -

 7 files changed, 87 insertions(+), 74 deletions(-)
Index: linux/net/ipv4/tcp.c

===================================================================

--- linux.orig/net/ipv4/tcp.c

+++ linux/net/ipv4/tcp.c

@@ -275,21 +275,21 @@ DEFINE_SNMP_STAT(struct tcp_mib, tcp_sta
 atomic_t tcp_orphan_count = ATOMIC_INIT(0);
-EXPORT_SYMBOL_GPL(tcp_orphan_count);

+EXPORT_SYMBOL_NS(tcp, tcp_orphan_count);
 int sysctl_tcp_mem[3] __read_mostly;

 int sysctl_tcp_wmem[3] __read_mostly;

 int sysctl_tcp_rmem[3] __read_mostly;
-EXPORT_SYMBOL(sysctl_tcp_mem);

-EXPORT_SYMBOL(sysctl_tcp_rmem);

-EXPORT_SYMBOL(sysctl_tcp_wmem);

+EXPORT_SYMBOL_NS(tcp, sysctl_tcp_mem);

+EXPORT_SYMBOL_NS(tcp, sysctl_tcp_rmem);

+EXPORT_SYMBOL_NS(tcp, sysctl_tcp_wmem);
 atomic_t tcp_memory_allocated;	/* Current allocated memory. */

 atomic_t tcp_sockets_allocated;	/* Current number of TCP sockets. */
-EXPORT_SYMBOL(tcp_memory_allocated);

-EXPORT_SYMBOL(tcp_sockets_allocated);

+EXPORT_SYMBOL_NS(tcp, tcp_memory_allocated);...
[ message continues ]

The UDP exports are only used by UDPv6 and UDP lite. They are internal functions

not supposed to be used by anybody else. So turn them into a name space that

only allows those.
---

 net/ipv4/udp.c     |   27 +++++++++++++++------------

 net/ipv4/udplite.c |    6 +++---

 2 files changed, 18 insertions(+), 15 deletions(-)
Index: linux/net/ipv4/udp.c

===================================================================

--- linux.orig/net/ipv4/udp.c

+++ linux/net/ipv4/udp.c

@@ -105,6 +105,9 @@

 #include <net/xfrm.h>

 #include "udp_impl.h"
+MODULE_NAMESPACE_ALLOW(udp, udplite);

+MODULE_NAMESPACE_ALLOW(udp, ipv6);

+

 /*

  *	Snmp MIB for the UDP layer

  */

@@ -1641,18 +1644,18 @@ void udp4_proc_exit(void)

 }

 #endif /* CONFIG_PROC_FS */
-EXPORT_SYMBOL(udp_disconnect);

-EXPORT_SYMBOL(udp_hash);

-EXPORT_SYMBOL(udp_hash_lock);

-EXPORT_SYMBOL(udp_ioctl);

-EXPORT_SYMBOL(udp_get_port);

-EXPORT_SYMBOL(udp_prot);

-EXPORT_SYMBOL(udp_sendmsg);

-EXPORT_SYMBOL(udp_lib_getsockopt);

-EXPORT_SYMBOL(udp_lib_setsockopt);

-EXPORT_SYMBOL(udp_poll);

+EXPORT_SYMBOL_NS(udp, udp_disconnect);

+EXPORT_SYMBOL_NS(udp, udp_hash);

+EXPORT_SYMBOL_NS(udp, udp_hash_lock);

+EXPORT_SYMBOL_NS(udp, udp_ioctl);

+EXPORT_SYMBOL_NS(udp, udp_get_port);

+EXPORT_SYMBOL_NS(udp, udp_prot);

+EXPORT_SYMBOL_NS(udp, udp_sendmsg);

+EXPORT_SYMBOL_NS(udp, udp_lib_getsockopt);

+EXPORT_SYMBOL_NS(udp, udp_lib_setsockopt);

+EXPORT_SYMBOL_NS(udp, udp_poll);
 #ifdef CONFIG_PROC_FS

-EXPORT_SYMBOL(udp_proc_register);

-EXPORT_SYMBOL(udp_proc_unregister);

+EXPORT_SYMBOL_NS(udp, udp_proc_register);

+EXPORT_SYMBOL_NS(udp, udp_proc_unregister);

 #endif

Index: linux/net/ipv4/udplite.c

===================================================================

--- linux.orig/net/ipv4/udplite.c

+++ linux/net/ipv4/udplite.c

@@ -113,6 +113,6 @@ out_register_err:

 	printk(KERN_CRIT "%s: Cannot add UDP-Lite protocol.\n", __FUNCTION__);

 }
-EXPORT_SYMBOL(udplite_hash);

-EXPORT_SYMBOL(udplite_prot);

-EXPORT_SYMBOL(udplit...
[ message continues ]

Shared by IP, IPv6, DCCP, UDPLITE, SCTP. 
The symbols used by tunnel modules weren't put into any name space

because there are quite a lot of them.
---

 net/core/fib_rules.c            |    9 ++++--

 net/ipv4/af_inet.c              |   52 ++++++++++++++++++++++++----------------

 net/ipv4/arp.c                  |    1

 net/ipv4/icmp.c                 |   10 +++----

 net/ipv4/inet_connection_sock.c |   40 +++++++++++++++---------------

 net/ipv4/inet_diag.c            |    4 +--

 net/ipv4/inet_hashtables.c      |    8 +++---

 net/ipv4/inet_timewait_sock.c   |   12 ++++-----

 net/ipv4/ip_input.c             |    2 -

 net/ipv4/ip_output.c            |    7 +++--

 net/ipv4/ip_sockglue.c          |   10 +++----

 11 files changed, 86 insertions(+), 69 deletions(-)
Index: linux/net/ipv4/af_inet.c

===================================================================

--- linux.orig/net/ipv4/af_inet.c

+++ linux/net/ipv4/af_inet.c

@@ -218,7 +218,7 @@ out:

 }
 u32 inet_ehash_secret __read_mostly;

-EXPORT_SYMBOL(inet_ehash_secret);

+EXPORT_SYMBOL_NS(inet, inet_ehash_secret);
 /*

  * inet_ehash_secret must be set exactly once

@@ -235,7 +235,7 @@ void build_ehash_secret(void)

 		inet_ehash_secret = rnd;

 	spin_unlock_bh(&inetsw_lock);

 }

-EXPORT_SYMBOL(build_ehash_secret);

+EXPORT_SYMBOL_NS(inet, build_ehash_secret);
 /*

  *	Create an inet socket.

@@ -1127,7 +1127,7 @@ int inet_sk_rebuild_header(struct sock *

 	return err;

 }
-EXPORT_SYMBOL(inet_sk_rebuild_header);

+EXPORT_SYMBOL_NS(inet,inet_sk_rebuild_header);
 static int inet_gso_send_check(struct sk_buff *skb)

 {

@@ -1235,6 +1235,8 @@ unsigned long snmp_fold_field(void *mib[

 	}

 	return res;

 }

+/* AK: Not in inet namespace because they're a generic facility. Probably

+   should be in another file though. */

 EXPORT_SYMBOL_GPL(snmp_fold_field);
 int snmp_mib_init(void *ptr[2], size_t mibsize, size_t mibalign)

@@ -1499,20 +1501,30 @@ static int __init ipv4_proc_init(void)
 MODULE_ALIAS_NETPRO...
[ message continues ]

This checks the namespaces at build time in modpost
---

 scripts/mod/modpost.c |  344 ++++++++++++++++++++++++++++++++++++++++++++++----

 1 file changed, 317 insertions(+), 27 deletions(-)
Index: linux/scripts/mod/modpost.c

===================================================================

--- linux.orig/scripts/mod/modpost.c

+++ linux/scripts/mod/modpost.c

@@ -1,8 +1,9 @@

-/* Postprocess module symbol versions

+/* Postprocess module symbol versions and do various other module checks.

  *

  * Copyright 2003       Kai Germaschewski

  * Copyright 2002-2004  Rusty Russell, IBM Corporation

  * Copyright 2006       Sam Ravnborg

+ * Copyright 2007	Andi Kleen, SUSE Labs (changes licensed GPLv2 only)

  * Based in part on module-init-tools/depmod.c,file2alias

  *

  * This software may be used and distributed according to the terms

@@ -12,9 +13,13 @@

  */
 #include <ctype.h>

+#include <assert.h>

 #include "modpost.h"

 #include "../../include/linux/license.h"
+#define NS_SEPARATOR '.'

+#define NS_SEPARATOR_STRING "."

+

 /* Are we using CONFIG_MODVERSIONS? */

 int modversions = 0;

 /* Warn about undefined symbols? (do so if we have vmlinux) */

@@ -27,6 +32,9 @@ static int external_module = 0;

 static int vmlinux_section_warnings = 1;

 /* Only warn about unresolved symbols */

 static int warn_unresolved = 0;

+/* Fixing those would cause too many ifdefs -- off by default. */

+static int warn_missing_modules = 0;

+

 /* How a symbol is exported */

 enum export {

 	export_plain,      export_unused,     export_gpl,

@@ -105,19 +113,43 @@ static struct module *find_module(char *

 	return mod;

 }
-static struct module *new_module(char *modname)

+static const char *basename(const char *s)

+{

+	char *p = strrchr(s, '/');

+	if (p)

+		return p + 1;

+	return s;

+}

+

+static struct module *find_module_base(char *modname)

 {

 	struct module *mod;

-	char *p, *s;
-	mod = NOFAIL(malloc(sizeof(*mod)));

-	memset(mod, 0, sizeof(*mod));

-	p = NOFAIL(strdup(mo...
[ message continues ]

Fix wrong format strings in modpost exposed by the previous patch.

Including one missing argument -- some random data was printed instead.
---

 scripts/mod/modpost.c |    7 ++++---

 1 file changed, 4 insertions(+), 3 deletions(-)
Index: linux/scripts/mod/modpost.c

===================================================================

--- linux.orig/scripts/mod/modpost.c

+++ linux/scripts/mod/modpost.c

@@ -388,7 +388,7 @@ static int parse_elf(struct elf_info *in
 	/* Check if file offset is correct */

 	if (hdr->e_shoff > info->size) {

-		fatal("section header offset=%u in file '%s' is bigger then filesize=%lu\n", hdr->e_shoff, filename, info->size);

+		fatal("section header offset=%lu in file '%s' is bigger then filesize=%lu\n", (unsigned long)hdr->e_shoff, filename, info->size);

 		return 0;

 	}
@@ -409,7 +409,7 @@ static int parse_elf(struct elf_info *in

 		const char *secname;
 		if (sechdrs[i].sh_offset > info->size) {

-			fatal("%s is truncated. sechdrs[i].sh_offset=%u > sizeof(*hrd)=%ul\n", filename, (unsigned int)sechdrs[i].sh_offset, sizeof(*hdr));

+			fatal("%s is truncated. sechdrs[i].sh_offset=%lu > sizeof(*hrd)=%lu\n", filename, (unsigned long)sechdrs[i].sh_offset, sizeof(*hdr));

 			return 0;

 		}

 		secname = secstrings + sechdrs[i].sh_name;

@@ -907,7 +907,8 @@ static void warn_sec_mismatch(const char

 		     "before '%s' (at offset -0x%llx)\n",

 		     modname, fromsec, (unsigned long long)r.r_offset,

 		     secname, refsymname,

-		     elf->strtab + after->st_name);

+		     elf->strtab + after->st_name,

+		     (unsigned long long)r.r_offset);

 	} else {

 		warn("%s(%s+0x%llx): Section mismatch: reference to %s:%s\n",

 		     modname, fromsec, (unsigned long long)r.r_offset,

-

Looks good. Can I get a s-o-b then I will apply it.
--

When passing an file name > 1k the stack could be overflowed.

Not really a security issue, but still better plugged.
---

 scripts/mod/modpost.c |    3 ++-

 1 file changed, 2 insertions(+), 1 deletion(-)
Index: linux/scripts/mod/modpost.c

===================================================================

--- linux.orig/scripts/mod/modpost.c

+++ linux/scripts/mod/modpost.c

@@ -1656,7 +1656,6 @@ int main(int argc, char **argv)

 {

 	struct module *mod;

 	struct buffer buf = { };

-	char fname[SZ];

 	char *kernel_read = NULL, *module_read = NULL;

 	char *dump_write = NULL;

 	int opt;

@@ -1709,6 +1708,8 @@ int main(int argc, char **argv)

 	err = 0;
 	for (mod = modules; mod; mod = mod->next) {

+		char fname[strlen(mod->name) + 10];

+

 		if (mod->skip)

 			continue;
-

Looks good. A s-o-b line again please.

Although I am not so happy with the ue of gcc extensions.
--

That's not a gcc extension. It's C99.
-Andi

--

OK.

I have applied all three patches to kbuild.git.
As I did not follow the whole thread about the namespace I did not

take those.

And the first patch touching module.c should go in via akpm I think.

It is outside my core-competence area at least .
	Sam

--

This way gcc can warn for wrong format strings
---

 scripts/mod/modpost.c |    8 +++++---

 1 file changed, 5 insertions(+), 3 deletions(-)
Index: linux/scripts/mod/modpost.c

===================================================================

--- linux.orig/scripts/mod/modpost.c

+++ linux/scripts/mod/modpost.c

@@ -33,7 +33,9 @@ enum export {

 	export_unused_gpl, export_gpl_future, export_unknown

 };
-void fatal(const char *fmt, ...)

+#define PRINTF __attribute__ ((format (printf, 1, 2)))

+

+PRINTF void fatal(const char *fmt, ...)

 {

 	va_list arglist;
@@ -46,7 +48,7 @@ void fatal(const char *fmt, ...)

 	exit(1);

 }
-void warn(const char *fmt, ...)

+PRINTF void warn(const char *fmt, ...)

 {

 	va_list arglist;
@@ -57,7 +59,7 @@ void warn(const char *fmt, ...)

 	va_end(arglist);

 }
-void merror(const char *fmt, ...)

+PRINTF void merror(const char *fmt, ...)

 {

 	va_list arglist;
-

This loks good. Can I get i s-o-b then I will apply it.
--

Even better, switch the code to the standard warn/warnx/err/errx.
Cheers,

Rusty.

--

Sorry must have been left out by mistake.
Signed-off-by: Andi Kleen <ak@suse.de>
for both patches.
-Andi
--