Messages in this thread | | | Date | Wed, 7 Dec 2005 14:23:11 -0200 | From | Eduardo Pereira Habkost <> | Subject | Re: [linux-usb-devel] Re: [PATCH 00/10] usb-serial: Switches from spin lock to atomic_t. |
| |
On Wed, Dec 07, 2005 at 05:02:33PM +0100, Arjan van de Ven wrote: > > > > No they're not. Both are just about equally expensive cpu wise, > > > sometimes the atomic_t ones are a bit more expensive (like on parisc > > > architecture). But on x86 in either case it's a locked cycle, which is > > > just expensive no matter which side you flip the coin... > > > > But if a lock is used exclusively to protect a int variable, an atomic_t > > seems to be more appropriate to me. Isn't it? > > sounds like it... > > > Please, if you could, review the patches with this in mind: we aren't > > changing any behaviour neither creating any weird lock scheme, we are > > only doing two things: > > ... however you are NOT changing the behavior, which is EXACTLY my > point; the current "lock emulation" behavior is wrong, all you're doing > is replacing how you do the wrong thing ;)
But now doing the Right Thing will be easy, as the wrong code isn't duplicated all around anymore: it is only in one place. ;)
We have just done a small refactoring, trying to keep behaviour. I haven't analysed deeply the current code to check if the "lock emulation" could be replaced by a better approach. But at a first look, it didn't look wrong to me. I am open to suggestions on how to replace the write_urb_busy checking by something better.
So, at least we agree that using atomic_t is better than the current approach, right? So, do you agree that, _if_ we chose to keep the write_urb_busy "pseudo-locking", we could at least remove the code duplication for that and use an atomic_t instead of spin_lock+int?
> > It's like having a bike with square wheels, and replacing a flat tire > with one with air in, as opposed to replacing it with a round wheel... >
I am open to suggestions on how to build a round wheel in this case. :)
-- Eduardo [unhandled content-type:application/pgp-signature] | |