Re: [linux-usb-devel] Re: [PATCH 00/10] usb-serial: Switches from spin lock to atomic_t.

[Eduardo Pereira Habkost <ehabkost@mandriva.com>]

[-- Attachment #1: Type: text/plain, Size: 1884 bytes --]

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

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]