netdev
[Top] [All Lists]

Re: modular net drivers

To: Andrew Morton <andrewm@xxxxxxxxxx>
Subject: Re: modular net drivers
From: Philipp Rumpf <prumpf@xxxxxxxx>
Date: Sat, 24 Jun 2000 08:01:06 -0700
Cc: Rusty Russell <rusty@xxxxxxxxxxxxxxxx>, Keith Owens <kaos@xxxxxxxxxx>, Alan Cox <alan@xxxxxxxxxxxxxxxxxxx>, "netdev@xxxxxxxxxxx" <netdev@xxxxxxxxxxx>
In-reply-to: <3954262D.60BDEF41@uow.edu.au>
References: <20000623164805.AA5BB8154@halfway> <3954262D.60BDEF41@uow.edu.au>
Sender: owner-netdev@xxxxxxxxxxx
On Sat, Jun 24, 2000 at 01:08:29PM +1000, Andrew Morton wrote:
> Rusty Russell wrote:
> > 
> > In message <20000622174858.304CE8154@halfway> I wrote:
> > > OK.  Here is how it would work:
> > 
> > Alternate solution to avoid module problems: Phil Rumpf and I came up
> > with basically identical answers.  It assumes that MOD_INC_USE_COUNT
> > is always called in user context, and involves no changes to module
> > code.
> > 
> > 1) static volatile int freeze[NR_CPUS];
> 
> Yup.
> 
> I think this can be generalised and pushed out to userland more.  A new
> system call:
> 
> int sys_stop_cpu(int yep)
> 
> sys_stop_cpu(1) Causes the current CPU to enter a busy loop, with local
> interrupts disabled.  The return value is the number of CPUs which are

Do we need to disable local interrupts ?  interrupt handlers are safe
because disable_irq is synchronous, not sure about softirqs.

> _not_ captured by sys_stop_cpu.  If the current CPU is the last CPU,
> sys_stop_cpu() will return 1 immediately.
> 
> 
> sys_stop_cpu(0) will unfreeze _all_ CPUs (I think this is a little
> racy...)
> 
> 
> 
> So the idea is that a privileged app can loop doing
> clone()/sys_stop_cpu(1) until all CPUs have stopped.  Then the
> privileged app can unload modules (or do anything else which requires
> total serialisation).

What's the point of exporting int sys_stop_cpu(int yep) ?  I can't really
see it being useful for anything but module load/unload, and I'm sure in
2.5 we will see people needing other APIs - disable_cpu(int cpu)-like.

> The weakness in this (and in your proposal, Rusty) is the case where
> some module code does a schedule() when the module reference count is
> zero.  I'm not aware of any which can do this, but all it would take is

You're still not allowed to be stupid, no.  What makes this slightly less
problematic is schedule() with a zero reference count is a bug, even on
UP.

Note that schedule() in the cleanup_module function isn't a bug, as long
as you can guarantee the module count won't be incremented again.

> Two more things:
> 
> You had:
> 
>         if (atomic_read(&mod->uc.use) == 0)
>                 mod->cleanup();
> 
> This could be changed to
> 
>         if (atomic_read(&mod->uc.use) == 0) {
>               atomic_inc(&mod->uc.use);
>                 mod->cleanup();
>               atomic_dec(&mod->uc.use);
>       }
> 
> to avoid bizarre reentrancy happenings if the module destructor somehow
> calls schedule().

I think we should mark a module that's being unloaded anyway.  incrementing
the use count would be nice because it would mea schedule() with a zero
use count is always a bug.


        Philipp Rumpf

<Prev in Thread] Current Thread [Next in Thread>