On Thu, Dec 14, 2000 at 12:47:51PM -0800, John Baldwin wrote:
> In other news, while wandering through the cardbus code, I discovered that
> pccbb softc's have an internal mutex much to my surprise, and that they weren't
> quite being used properly AFAICT. In the pccb0 kthread, they were
> acquired/released without Giant. However, in the rest of the pccbb driver,
> they are acquired/released with Giant. This leads to lock order violations
> which could potentially deadlock the machine at some point. Also, the pccbb0
> kthread holds the mutex across the entire card insertion and removal events,
> which is not quite right. Mutexes should only be held for short periods of
> time. As such, I've futzed around with the mutex operations in the pccbb
> driver some. It may not be completely correct, but I think it is an
> improvement. The patch for this can be found at
> http://www.FreeBSD.org/~jhb/patches/pccbb.patch.
[I just now recalled your mentioning this a while back, so here's my much
delayed response]
Originally I had write the code with spl's, and at one point decided to
convert them to mutex after finding spl's don't do anything. However, I
don't claim to know a whole lot about mutexes or FreeBSD's implementation
of it. Perhaps someone can point me to the Great Definitive Source?
The first thing I'm unclear about is the deal with Giant. My understanding
is that Giant is something one would acquire if he wanted to be really
exclusive, or something like that. John - you mentioned I was
acquiring/releasing with/without Giant - but I don't see how I'm doing
things differently in and out of the kthread. The only place I mentioned
Giant was right before kthread_exit, and that's necessary least
kthread_exit calls panic() on me. On your patch, you acquire Giant at the
start of the kthread - wouldn't that hold onto the lock until the thread
exits, and make other things attempting to lock Giant block?
As for the 'holding onto mutex for a long time' problem, I'm not sure if
your solution is best. I originally put in the locks firstly because
that's how NetBSD had it, and secondly so that card removal interrupts
while card is still attaching wouldn't be lost. Your patched version
allows for wakeup() to be called while insertion/removal is being
processed. I've glanced through the code and didn't see anything that
requires mutual exclusivity between the kthread the the interrupt handler,
so perhaps a better solution would be to simply not use mutex at all?
Comments?
--
(o_ 1-2-1-2-1-2-1-2-1-2-1-2-1-2-1-2-1-2-1-2-1-2-1-2-1-2-1-2-1-2 _o)
\\\_\ Jonathan Chen [EMAIL PROTECTED] /_///
<____) No electrons were harmed during production of this message (____>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
