Date: Mon, 13 Jul 2015 20:56:36 +0200 From: Luigi Rizzo <rizzo@iet.unipi.it> To: Konstantin Belousov <kostikbel@gmail.com>, current@freebsd.org Cc: adrian@freebsd.org Subject: Re: protection against module unloading ? Message-ID: <20150713185636.GA28828@onelab2.iet.unipi.it> In-Reply-To: <20150713152912.GL2404@kib.kiev.ua> References: <CA%2BhQ2%2BiNWHzH=L6YC8TQvsWsz3EhMcLTv9VOP5nXtmDWD4TD0g@mail.gmail.com> <20150713124603.GH2404@kib.kiev.ua> <20150713150030.GA25208@onelab2.iet.unipi.it> <20150713152912.GL2404@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Jul 13, 2015 at 06:29:12PM +0300, Konstantin Belousov wrote:
> On Mon, Jul 13, 2015 at 05:00:30PM +0200, Luigi Rizzo wrote:
...
> > 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.
> What contention do you see ? Is it on the device node for a single
> device ? If yes, then any modification of the below proposal would
> not help. I explained this below.
It was adrian that pointed it out to me the huge devmtx contention
with multiple threads doing I/O on netmap file descriptor
(4-8 threads each of them issuing around 200K syscalls/s)
Now i see how even if my idea of per-dev lock was correct
it would not remove contention at all.
One final thing:
> > 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
> This would not work, you cannot protect a lifetime of the object by a lock
> contained in the object.
i thought so but then the current dev_refthread() is already unsafe,
accessing dev->si_flags unprotected
sys/kern/kern_conf.c:
struct cdevsw *
dev_refthread(struct cdev *dev, int *ref)
{
struct cdevsw *csw;
struct cdev_priv *cdp;
mtx_assert(&devmtx, MA_NOTOWNED);
if ((dev->si_flags & SI_ETERNAL) != 0) {
*ref = 0;
return (dev->si_devsw);
}
dev_lock();
csw = dev->si_devsw;
if (csw != NULL) {
cdp = cdev2priv(dev);
if ((cdp->cdp_flags & CDP_SCHED_DTR) == 0)
atomic_add_long(&dev->si_threadcount, 1);
else
csw = NULL;
}
dev_unlock();
*ref = 1;
return (csw);
}
that is particularly bad though, because it prevents from
checking the SI_ETERNAL flag without holding the lock (short
of encoding the flag in the pointer!)
cheers
luigi
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150713185636.GA28828>