From: Felipe W Damasio <felipewd@terra.com.br>
To: Richard Procter <rnp@paradise.net.nz>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] Fix SMP support on 3c527 net driver, take 2
Date: Tue, 09 Sep 2003 10:14:31 -0300 [thread overview]
Message-ID: <3F5DD237.4040909@terra.com.br> (raw)
In-Reply-To: <Pine.LNX.4.21.0309091029230.252-100000@ps2.local>
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
prev parent reply other threads:[~2003-09-09 13:12 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2003-09-02 20:24 [PATCH] Fix SMP support on 3c527 net driver, take 2 Felipe W Damasio
2003-09-03 13:42 ` Felipe W Damasio
2003-09-04 6:33 ` Richard Procter
2003-09-04 12:52 ` Felipe W Damasio
2003-09-04 13:22 ` Alan Cox
2003-09-04 13:28 ` Felipe W Damasio
2003-09-08 19:58 ` Felipe W Damasio
2003-09-08 23:49 ` Richard Procter
2003-09-09 13:14 ` Felipe W Damasio [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3F5DD237.4040909@terra.com.br \
--to=felipewd@terra.com.br \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=linux-kernel@vger.kernel.org \
--cc=rnp@paradise.net.nz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.