Re: [RFC] change format of LSM hooks

From: Rusty Russell (
Date: Thu Oct 17 2002 - 21:04:15 EST

In message <E182EJl-0004Rd-00@starship> you write:
> > The current spinlock is horrible.
> Is it? You must be thinking about much more intensive use of the spinlock as
> with per-op calls as opposed to per-attach (mount). I'd planned to make the
> spinlocks per-module, but your per-cpu code looks just fine.

Actually, after I thought about it harder, I was inspired by your
point that the race can be prevented with a spinlock (as currently
done). I spent last night writing a new solution to this (I'm testing
it now), which works as follows:

/* We only need protection against local interrupts. */
#define local_inc(x) atomic_inc(x)
#define local_dec(x) atomic_dec(x)

static inline int try_module_get(struct module *module)
        int ret = 1;

        if (module) {
                unsigned int cpu = get_cpu();
                if (likely(module->live))
                        ret = 0;
        return ret;

static inline void module_put(struct module *module)
        if (module) {
                unsigned int cpu = get_cpu();
                /* Maybe they're waiting for us to drop reference? */
                if (unlikely(!module->live))

Now, these are both short, sweet, and cache-local. The unload stage
becomes harder. We stop all the reference counts by scheduling a
thread on every cpu, then having them all do a local_irq_disable.
This means we know that the refcounts won't change, and we can safely
sum them. If they're zero (or the non-block flag isn't set), we set
"module->live" to false and release the CPUs.

Now, in the sleeping case, I sleep uninterruptible, but drop the
module mutex first, so other module stuff can happen at the same time.

This completely prevents the spurious unload race.

> > I don't know of any code which does this now, but it is at least a
> > theoretical problem.
> To resolve this, start the module in can-increment state, do the module
> initialization, register the notifiers, and finally register the interface.
> In other words, the module never needs to be in can't-increment state at
> initialization. (The module writer must ensure they have the correct,
> raceless initial state of whatever the notifiers are notifying about, which
> strikes me as a little tricky in itself.)

Yes, this basically means two-state init for modules, which I shy
clear of in general. But if any modules care, they can do this
themselves, though, by setting " = 1;" to activate
the notifiers during their init routine.

Basically, with the other one solved, I'm happy to place this on the
shoulders of any module which really cares.

> > IMHO, the benifits of having it in-kernel outweigh the slight extra
> > size.
> I'll cast my vote for your in-kernel linker, in the mistaken belief that
> democracy has anything to do with the question. Does this make progress
> towards eliminating one of create_module or init_module?

Yes, query_module, create_module and /proc/ksyms all gone.

> > > > ...The second is the "die-mother-fucker-die"
> I'd use this feature in filesystem development, regardless of the risks,
> since I regularly do worse things to the kernel anyway. But don't you think
> the rmmod parameter should be different for the ask-nicely vs the
> shoot-in-the-head flavor? How about -f -f or -F for the latter?

Yes, currently:
        rusty@mingo:~$ /sbin/rmmod
        Usage: /sbin/rmmod [--wait|-f] <modulename>
          The --wait option begins a module removal even if it is used
          and will stop new users from accessing the module (so it
          should eventually fall to zero).
          The -f option forces a module unload, and may crash your
          machine. This requires the Forced Module Removal option
          when the kernel was compiled.


  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

This archive was generated by hypermail 2b29 : Wed Oct 23 2002 - 22:00:39 EST