Date: Wed, 28 May 2008 08:50:51 -0600 (MDT) From: "M. Warner Losh" <imp@bsdimp.com> To: aopopov@yahoo.com Cc: freebsd-drivers@FreeBSD.org Subject: Re: Synchronization in drivers (after SMP improvements) Message-ID: <20080528.085051.-1253012495.imp@bsdimp.com> In-Reply-To: <730729.32628.qm@web51410.mail.re2.yahoo.com> References: <730729.32628.qm@web51410.mail.re2.yahoo.com>
next in thread | previous in thread | raw e-mail | index | archive | help
In message: <730729.32628.qm@web51410.mail.re2.yahoo.com>
Alexander Popov <aopopov@yahoo.com> writes:
: I'm currently writing a FreeBSD USB driver and have a few questions
: about synchronization between bottom and top half of the
: driver.
By USB driver, I'm assuming you mean "a driver for a usb device that's
plugged into the system" as opposed to "a driver for a usb host
adapter."
: This is the strategy that is followed by most of the (USB) drivers
: currently. However, after the SMP improvements have been
: implemented, the splxxx family of functions has been successfully
: stubbed out, so now these functions do absolutely nothing, which
: (correct me if am wrong) leaves most of the existing USB drivers
: without any synchronization with their bottom halves whatsoever.
Right now, the USB stack is Giant locked, so this stuff works.
: Next, in the scope of SMP project, interrupt handlers have been
: replaced with (lightweight?) threads, which makes it possible to use
: synchronization primitives in the interrupt handlers to synchronize
: them with driver's top half. However, this does not apply to USB
: stack, which, as I understand, still uses Giant lock. So, from this
: perspective, I can not make interrupts in my USB driver MPSAFE.
Well, you could, but it is really hard to do since you have to
understand the current USB code and be able to acquire Giant at the
right times, etc. So theoretically possible, but I don't know of
anybody that's succeed in doing so.
: It has been suggested everywhere that mutexes shall be used to
: protect top and bottom halves of the driver. So I've tried. The
: following is the pseudo-code that I've used:
: int driver_read(...)
: {
: mtx_lock(&sc->mtx);
:
: if (no data is available) => tsleep(..)
:
: mtx_unlock(&sc->mtx);
: }
As others have pointed out, the fundamental problem here is that you
are using tsleep while holding a mutex. This is not allowed because
it leads to bad things happening. Not always, but sometimes as you've
discovered.
: First it worked very well, but under relatively heavy load I started
: getting kernel panic, all the time related to one error: a thread
: holding a non-sleepable lock. Which means that user process has been
: suspended somewhere during execution in the read() function
: (probably awaiting data, but not necessary) and its thread has been
: holding mutex that I've used for synchronization, but that it
: apparently not allowed (look in man page on mutex).
The correct solution here is to just use msleep.
: Then I found that in SMP project an additional flag was added to
: mutex - MTX_SPEEPABLE, which I guess would allow me to have sleeping
: threads in kernel that hold mutexes. But, apparently, this flag has
: been recently removed from kernel...
This isn't what you want...
: Question is: what is now a synchronization model for modern kernels,
: i.e. FreeBSD 7.0? How shall I properly implement synchronization in
: my driver?
I think you can use msleep. Alternatively, you could use
condvars for synchronization. And I'd make the if () statement a
while () statement above to guard against races.
Warner
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080528.085051.-1253012495.imp>