Date: Fri, 30 May 2008 08:34:07 -0700 (PDT) From: Alexander Popov <aopopov@yahoo.com> To: "M. Warner Losh" <imp@bsdimp.com> Cc: freebsd-drivers@FreeBSD.org Subject: Re: Synchronization in drivers (after SMP improvements) Message-ID: <987362.36986.qm@web51404.mail.re2.yahoo.com>
next in thread | raw e-mail | index | archive | help
Hi, Warner,
Thanks, I appreciate your help.
Yes, indeed I am writing a USB device driver. I have done some tests,
and results are interesting. First of all, when I am trying to use default mutex in top-haft of the driver, under heavy load it panics anyway, even though there is no tsleep or alike functions used while holding the mutex. Basically what I do is this:
void driver_open()
{
mtx_lock(&sc->mtx)
// open USB pipes to device, etc.
mtx_unlock(&sc->mtx)
}
void driver_write(...)
{
mtx_lock(&sc->mtx)
// write to device (I do not sleep here!)
mtx_unlock(&sc->mtx)
}
then, one process opens device, and starts repeatedly write to it, constantly acquiring and leaving mutex; then, I start another process, that simply tries to open the same device => if it happens to be while first process is holding the mutex => kernel panics with message "sleeping thread with non-sleepable lock".
When I change default mutex to spin type, it works well, and I was not able to crash it. As Ilya pointed out, locking(9) says that Giant lock must be acquired before any other locks, which might be the reason why my code crashes with default mutex - USB drivers are Giant locked.
It is interesting, because I've seen USB drivers in the default FreeBSD distribution that use default mutexes...
However, when I try to lock spin mutex from interrupt handler, everything blocks and after a while I get panic with a message that "spin mutex has been locked for too long". Could not find explanation for this one yet...
Regards,
Alexander.
----- Original Message ----
From: M. Warner Losh <imp@bsdimp.com>
To: aopopov@yahoo.com
Cc: freebsd-drivers@FreeBSD.org
Sent: Wednesday, May 28, 2008 4:50:51 PM
Subject: Re: Synchronization in drivers (after SMP improvements)
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?987362.36986.qm>