Message-ID: <3954262D.60BDEF41@uow.edu.au>
Date:   Sat, 24 Jun 2000 13:08:29 +1000
From:   Andrew Morton <andrewm@uow.edu.au>
MIME-Version: 1.0
To:     Rusty Russell <rusty@linuxcare.com.au>
CC:     Keith Owens <kaos@ocs.com.au>,
        Philipp Rumpf <prumpf@puffin.external.hp.com>,
        Alan Cox <alan@lxorguk.ukuu.org.uk>,
        "netdev@oss.sgi.com" <netdev@oss.sgi.com>
Subject: Re: modular net drivers
References: <20000623164805.AA5BB8154@halfway>
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Sender: owner-netdev@oss.sgi.com
Precedence: bulk

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
_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).


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
a kmalloc() within a netdriver's
set_multicast_list/do_ioctl/get_stats/etc method.


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().


Finally, the net drivers seem to be the biggest problem at this time,
and I think all their methods are called via ioctl on a socket.

For 2.4 why the hell don't we just take the unlock_kernel() out of
sock_ioctl()?