Module Refcount & Stuff mini-FAQ

[ Suggestions welcome ]

Golden Rule: If you are calling though a function pointer into a (different) module, you must hold a reference to that module. Otherwise you risk sleeping in the module while it is unloaded.

Q: How do I get a reference to a module?
A: Usually, a successful call to try_module_get(owner). You don't
need to check for owner != NULL, BTW.

Q: When does try_module_get(owner) fail?
A: When the module is not ready to be entered (ie. still in
init_module) or it is being removed. It fails to prevent you entering the module as it is being discarded (init might fail, or it's being removed).

Q: But modules call my register() routine which wants to call back
into one of the function pointers immediately, and so try_module_get() fails!
A: You're being called from the module, so someone already has a
reference (unless there's a bug), so you don't need a try_module_get().

This does mean that if you were to register a structure for *another* module (does anyone do this?) you'd need to have a reference to it.

Q: How do I put the reference back?
A: Using module_put(owner) (owner == NULL is OK).

Q: Do I really need to put try_module_get() before every function ptr
call?
A: If the function does not sleep (any cannot be preempted) ie. is
called in softirq or hardirq context, you can omit this step, since you obviously won't sleep inside the module.

Also, most structs have clear "start" and "stop" functions (eg. mount/umount), so you only need one try_module_get() on start, and module_put() on stop.

Q: Is it safe to call try_module_get() and module_put() from an
interrtupt / softirq?
A: Yes.

Q: My code use "MOD_INC_USE_COUNT". Do I still need to adjust my
module count when someone calls one of my functions?
A: No, you never need to adjust your own module count. There are five
ways a function in your module can get called: firstly, it could be your module_init() function, in which case the module code holds a reference. It could be another module using one of your EXPORT_SYMBOL'ed functions, in which case you cannot be removed since they would have to be removed first. It could be a module which found an EXPORT_SYMBOL'ed function using symbol_get(), in which case they hold a reference count. It could be through a function pointer which your module gave out previously, which is discussed above. Finally, it could be from within your own module, in which case someone must already hold a reference.

Q: My code uses "__MOD_INC_USE_COUNT(reg->owner)", but now I get a
warning at runtime that it is unsafe. What do I need to do?
A: You need to use try_module_get(), and not call into the module if
it fails (act as if it hasn't registered yet). Note that you no longer need to check for NULL yourself, try_module_get() does that.

Q: My code used "GET_USE_COUNT(module)" to get the reference count.
A: Don't do that. If module unloading is disabled, there is no
reference count, and there is never a single value you can assign to.

Q: My code used "try_inc_mod_count(module)" to get the reference
count. Should I change it?
A: No hurry. try_module_get() is exactly the same: the new name
reflects that this is now the only way to get a reference.

Q: How does the code in try_module_get() work?
A: It disables preemption for a moment, checks the live flag, and then
increments a per-cpu counter if the module is live. This is even lighter-weight (in icache and cycles) than using a brlock, but has the same effect. If CONFIG_MODULE_UNLOAD=n, it just becomes a check that the module is live.

Q: How does the module remove code work?
A: It stops the machine by scheduling threads for every other CPU,
then they all disable interrupts. At this stage we know that noone is in try_module_get(), so we can reliably read the counter. If zero, or the rmmod user specified --wait, we set the live flag to false. After this, the reference count should not increase, and each module_put() will wake us up, so we can check the counter again.

Q: Are these changes all so you could implement an in-kernel module
linker?
A: No, they were to prevent load and unload races without altering
every module, nor introducing drastic new requirements.

Q: Doesn't putting linking code into the kernel just add bloat?
A: The total linking code is about 200 generic lines, 100
x86-specific lines. The ia64 linking code is about 500 lines (it's the most complex). Richard Henderson has a great suggestion for simplifying it furthur, which I'm implementing now. "insmod" is now a single portable system call, meaning insmod can be written in about 20 lines of code.

The previous code required to implement the two module loading system call, the module querying system call, and the /proc/ksyms output, required a little more code than the current x86 linker.

Q: Why not just fix the old code?
A: Because having something so intimate with the kernel in userspace
greatly restricts what changes the kernel can make. Moving into the kernel means I have implemented modversions, typesafe extensible module parameters and kallsyms without altering userspace in any way. Future extensions won't have to worry about the modversions problem.

Q: Why not implement two-stage insert / two-stage delete?
A: Because I implemented it first and it sucked. And because this
*is* two-stage insert and two-stage delete, without exposing it to the modules using it, with the added advantage that the second stage is atomic (activation/deactivation is simply changing mod->live, modulo locking implementation magic detailed above). This prevents the race between deactivating the module and finding that someone has starting using it as you are deactivating it.

Hope that helps?
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

I'd suggest "It fails in order to [...]" to avoid the "does work exactly NOT as advertised" ambiguity ;-)

Hmm, I haven't really looked at your new code, but this sounds as if try_module_get gets an exclusive lock. Is this true ? Doesn't seem to make sense to me.

Don't you want to repeat the golden rule at the end, just as a polite reminder ? :-) (Just kidding.)

- Werner

  _________________________________________________________________________
 / Werner Almesberger, Buenos Aires, Argentina         [email protected] /
/_http://www.almesberger.net/____________________________________________/

In message <[email protected]> you write:

Ah, thanks, good catch. I changed it to "This prevents you".

No, it doesn't. The question is badly phrased. How about:

Q: But the modules' init routine calls my register() routine which
wants to call back into one of the function pointers immediately, and so try_module_get() fails! (because the module is not finished initializing yet)
A: You're being called from the module, so someone already has a
reference (unless there's a bug), so you don't need a try_module_get().

This does mean that if you were to register a structure for *another* module (does anyone do this?) you'd need to have a reference to it.

Heh.

Well, if we continue to start modules unisolated, I need to rewrite the FAQ anyway...

Thanks,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Hmm, I wouldn't call this the answer. How about:
- Q: why does it fail ?
- A: because you're initializing
- solution: but since you're calling from a module, and the call goes back to the same module, you don't have to worry

This raises the question: why is this a special case ? The registration function shouldn't have to know all these details. (That's the whole point of try_module_get, isn't it ?)

Wouldn't it be possible to simply allow try_module_get also while the module is initializing ?

Does "unisolated" mean that try_module_get would work ? If yes, you've already solved the problem ;-)

- Werner

  _________________________________________________________________________
 / Werner Almesberger, Buenos Aires, Argentina         [email protected] /
/_http://www.almesberger.net/____________________________________________/

In message <[email protected]> you write:

Yes, this is a fairly rare case: I'm debating it now. For example, scsi calls back into the module which just registered, as does the block layer (to probe for partitions).

At the cost of exposing the module to initialization races.

Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Hmm, what races are there that don't correspond to a bug in some module ?

- Werner

  _________________________________________________________________________
 / Werner Almesberger, Buenos Aires, Argentina         [email protected] /
/_http://www.almesberger.net/____________________________________________/

In message <[email protected]> you write:

Ah, see other thread (weren't you at the kernel summit?). There's currently no way to abort if you've exposed interfaces and then something fails ("don't do that" is great except noone knows that, and it's not always possible or nice)

The old module code used to just ignore it. My code tries to catch it and leaves the module stuck (except if !CONFIG_MODULE_UNLOAD, in which case it can't really detect it).

Since we have a way of isolating modules for the symmetric race on exit, it makes sense to use it on entry (of course, try_inc_mod_count would have to keep violating the rule during the transition).

Hope that helps,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Argh, there are only about a hundred threads on modules ;-)

Yes, but I hadn't paid much attention to modules before, so I only understood about half of what you said, sorry. It was interesting to learn that there were actually so many problems, though :-)

Hmm, if "expose interface" == "publish symbol", why can't you simply defer publishing until after initialization completes ? If "expose interface" == "register something somewhere", then this has to be undone anyway. Or am I overlooking something here ?

Thanks,
- Werner

  _________________________________________________________________________
 / Werner Almesberger, Buenos Aires, Argentina         [email protected] /
/_http://www.almesberger.net/____________________________________________/

In message <[email protected]> you write:

Yes, but between doing and undoing (in the failure path) someone has started using the module. The old modutils would unload it underneath them here. I catch it (if CONFIG_MODULE_UNLOAD, otherwise I can't) and yell "module is now stuck" and leave it hanging.

Given we have a method of isolating a module already, it seems logical to use it to prevent exactly this race. Unfortunately my last attempt assumed noone did this, and broke IDE and SCSI (hence pissing *everyone* off 8).

Cheers,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

But how ? Don't we have only two ways of calling a module, i.e. by symbol, or by callback ? All callbacks that might call a module must be protected with try_module_get, right ? (*)

(*) Actually, if the registration can be revoked, and the
deregistration function does properly synchronize with on-going callbacks, you shouldn't need try_module_get either. E.g. del_timer_sync doesn't need to know about module owners.

So, if you make try_module_get work during initialization, and modules don't publish their symbols before initialization is done, there should be no problem ?

But that's mainly the symbol-unload race, considering that any call through a function pointer requires try_module_get anyway. Without try_module_get, there would also be callback-unload races if callback de-registration doesn't synchronize (e.g. del_timer vs. del_timer_sync).

By the way, it's also not so nice that there can't be callbacks at removal, e.g.

service_unregister(...)

{
	...
	for_pending_requests(req) {
		...
		if (try_module_get(req->owner))
			req->fn(req,REQUEST_CANCELLED);
		else
			printk(KERN_CRIT "we just dropped a request on the "
			  "floor, how nice\n");
		...
	}
	...
}

Calling this from the module removal function would be perfectly safe.

Actually ... can't you allow modules to be called until the cleanup function has returned ?

Two in one strike ain't bad ;-) Maybe we can find something that gets rid of that pesky NFS, too, e.g. by adding

#define while if /* enforce efficient programming practices */

to linux/kernel.h, or such ;-)

- Werner

  _________________________________________________________________________
 / Werner Almesberger, Buenos Aires, Argentina         [email protected] /
/_http://www.almesberger.net/____________________________________________/

In message <[email protected]> you write:

try_module_get() will succeed now during initialization, because the module starts live.

del_timer_sync is actually an oddity: most deregistration functions do not block pending outstanding calls, they reference count and "unhook" at deregistration, and delete when the refcount hits zero (if they do anything at all 8).

(Note also: timers don't need to do try_module_get() since they can't sleep).

Yes, but that's not the way things work currently: the interfaces to reserve and publish are not separated. And whether it's worth separating them simply because of this, is the question.

Yes, and it's already in the list of exceptions: you're being called by the module itself here.

Also, you're assuming that the coder chose not to do the try_module_get() at request submission time.

Finally, there are some cases where a module can miss events while unloading, but that's OK, because it's *guaranteed* to exit at this point, so it must clean everything up in its cleanup routine anyway. I couldn't think of an exception, can you?

Only by splitting cleanup into "cleanup" and "destroy" (either by having cleanup say "OK, I'm not live anymore" halfway though, or having separate hooks).

We have over 1500 modules: not changing the interfaces to them was one of the key goals. If someone decides to later, fine.

Heh... <sigh> Point taken. I already decided to leave this one for the moment, and do something more productive.

Thanks,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

This makes it sound like you're not bringing /proc/ksyms back (or an equivalent to let userspace know where modules are loaded). I hope this isn't the case...

regards
john

Khendon's Law: If the same point is made twice by the same person,
the thread is over.

In message <[email protected]> you write:

I implemented the minimal subset: it's trivial to put back. The important question is why do you want it? Do you only want it when CONFIG_MODULES=y? Do you only want the exported symbols, or all symbols?

If this is for oprofile to figure out where modules are, then an entry in /proc/modules seems more appropriate, yes?

Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Keith Owens pointed out that I need info on where each section is mapped (consider the separate alloc of init/core sections; sure, module_init() is an odd thing to profile but it's nice not to lose samples). I don't care about the symbols themselves, but I believe Keith does.

regards
john

Khendon's Law: If the same point is made twice by the same person,
the thread is over.

Rusty,

What is the status of kksymoops?

I realize that you have a full plate, but [for the short time it existed in mainline] this was a incredibly useful feature for end users providing bug reports to me (and others).

I'm _not_ asking "when", just wondering what the plan is to resuscitate kksymoops.

Jeff

In message <[email protected]> you write:

Looks like someone pushed my patch. Erm, OK, wonder if it works on x86? 8)

My mental TODO list looked something like this:
1) Drop the optimization which checks against addr between _stext and _etext, as this skips __init functions on most archs.
2) Only put in the symbols for functions (currently CONFIG_KALLSYMS=y adds 400k on my laptop: ouch!).
3) Keith asked me to rename it, so as not to get confused with the previous patches and kgdb support). I guess it's too late for this one.
4) Use a simple scheme like the mini-oopser did to reduce the symbol table size furthur (I got it down to 70k IIRC).
5) See if Kai prefers the compile step inside the Makefile rather than in the script.
6) It'd be nice if CONFIG_KALLSYMS=m worked.

If someone wants to champion any or all of these, be my guest, there's plenty to go around 8)

Cheers,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

I think Linus himself did that ;)

Well, this was put in to avoid all kind of garbage in the traces, so it shouldn't just go without replacement. Probably one could even get it correct now, using ->module_init() and ->module_core() (just set them for the core kernel as well).

I'm not to sure about this, I sometimes find it useful to have variables on the stack identified correctly.

Nothing wrong with a follow-up patch, is it?

I'll actually have to look into this. The script is probably fine.

Shouldn't be too hard.

Well, I know talk is cheap. I'll try to find some time to actually look into some of these issues and come up with patches.

--Kai

LOL -- look at me eat my words.

I see it's now in Linus's tree -- thanks much!

Jeff

Hi!

Where is this implemented? I guess I need this for swsusp...

Pavel

Worst form of spam? Adding advertisment signatures ala sourceforge.net.
What goes next? Inserting advertisment *into* email?

In message <[email protected]> you write:

I'm not so sure, but it's worth a look. Look for stop_refcounts() in module.c. BTW, I call this a "bogolock".

Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.