Messages in this thread | | | Date | Tue, 09 Sep 2003 10:14:31 -0300 | From | Felipe W Damasio <> | Subject | Re: [PATCH] Fix SMP support on 3c527 net driver, take 2 |
| |
Hi Richard,
Richard Procter wrote: > I've had a good look over your revised patch, and it looks fine to me. I > didn't manage to get an MCA kernel booting to test it, but I'm not sure if > it would have added a lot, especially as I don't have an SMP MCA machine.
That's closer the a proper test box than me, since I don't even have an that NIC ;)
> That said, over the weekend I realised that the need to unroll wait_event > was a consequence of using the same queue to perform two quite distinct > functions: serialising the issuing of commands, and waiting for the card > to complete command execution. This forces us to use a private variable to > indicate which situation has occured. That's ok on UP, but requires us to > jump through hoops to use it safely on SMP with spinlocks.
True, but since the patch uses a per-device lock, aren't we safe (and relatively scaleable) on SMP too?
Using wait_event as "prepare_for_wait" and all that IMHO is needed because wait_event calls schedule, and we can't do that with our device lock held..hence the prepare_to_wait/spin_unlock/schedule stuff on mc32::wait_pending.
I don't know if using 2 different locks is worth it, since we may starve mc_reload on SMP...but since the ammount of code dropped, it's worth testing to see if we scale better than the inline wait_pending stuff.
> I've rewritten things using completions (== semaphores?), and it's both > cleaner and (unexpectedly) smaller (see example below). I'm in the process > of convincing myself it all works; should have something out there by the > end of the week.
Great!
Please let me know when you have something...I'm really enjoying helping to fix this bug. :)
> If there's a merge deadline coming up, please feel free to submit the > patch, otherwise I'd like to hold off for a couple of days and see where > we stand then.
There isn't.
Maybe we won't make 2.6.0-test5, but there's no need to rush things. Let's get this right.
Kind Regards,
Felipe
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |