On Mon, Jul 13, 2015 at 03:46:03PM +0300, Konstantin Belousov wrote:
> On Mon, Jul 13, 2015 at 02:28:40PM +0200, Luigi Rizzo wrote:
> > Hi,
> > I am trying to understand how to protect efficiently against
> > module removals when a device driver is in use.
> > This issue came up some time ago when trying netmap
> > loaded as a module.
> >
> > --- Summary of the current mode of operation (as i understand it): ---
> >
> > When a device is compiled as a module, all devfs calls in
> > sys/fs/devfs/devfs_vnops.c are protected by setting
> > dev->si_threadcount (within dev*_refthread()) for the duration of the call.
> >
> > This is implemented using devfs_fp_check()/dev_relthread() which
> > in turn, for the module case, updates the dev->si_threadcount under
> > dev_lock() which is a single shared mutex, devmtx .
> >
> > The above is problematic for driver modules that do a lot of I/O
> > using poll, ioctl, read or write, because of the system-wide contention
> > on devmtx
> >
> > To mitigate the contention, statically compiled modules have the
> > SI_ETERNAL attribute that prevents the lock (major contention point)
> > and si_threadcount update.
> >
> > --- Alternative ?
> >
> > I was hoping to make the protection mechanism cheaper by only
> > increasing dev->si_threadcount on devfs_open() without calling
> > dev_relthread() ), and then decreasing si_threadcount on defvs_close() .
> > This way the reference is active for the lifetime of the file descriptor
> > without needing to track individual high-frequency calls.
> >
> > This is probably not enough because there could be mmap handlers,
> > which remain active even after the device is closed.
> This is of course wrong because destroy_dev(9) would be blocked until
> all file descriptors referencing the device are closed. The blocked
> thread which called destroy_dev() is blocked hard, i.e. you cannot
> interrupt it with a signal. The situation is incomprehensible for
> the user issued the kldunload command.
>
> The current si_threadcount mechanism is designed and intended to work on
> the code call granulariry, to allow the device destruction and driver
> unload when there is no code call into the driver.
thanks a lot for the clarification on the intent.
I clearly need to understand more on the architecture of the module unload.
In any case: the global contention on devmtx for I/O syscalls is
really a showstopper for making effective use of modular drivers
so we should really find a way to remove it.
Is there any other way to protect access to dev->si_threadcount ?
Eg how about the following:
- use a (leaf) lock into struct cdev to protect dev->si_threadcount, so that
we could replace dev_lock() with mtx_lock(&dev->foo) in dev_refthread(dev)
dev_relthread(dev) and other places that access si_threadcount
- devvn_refthread() further uses vp->v_rdev, which elsewhere is protected by
VI_LOCK(vp), so devvn_refthread() could use the sequence
VI_LOCK(vp);
dev = vp->v_rdev;
if (dev != NULL) {
mtx_lock(&dev->foo)
if ( ... ) {
atomic_add_long(&dev->si_threadcount, 1);
}
mtx_unlock(&dev->foo);
}
VI_UNLOCK(vp)
(this is almos the same as what we have in devfs_allocv() and devfs_reclaim(),
except that they use dev_lock() instead of the device-specific lock.
cheers
luigi
_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
