Hi!
> Vojtech, thank you for the explanation. That's much better than the
> one-line message from Pavel which just said there was a problem.
>
> Randy -- please reject _all_ of Pavel's patches. At best, they don't fix
> anything. At worst, they introduce new problems, hide other problems, and
> slow me down by forcing me to diff and merge his changes and mine.
Sorry. Notice that I did not send "big" patch to randy.
> Pavel - look, I hate to sound like this, but would you please stop messing
> around in places you don't fully understand? I'll be the first to admit
> that your comment about the spinlock_irqsave as a good catch. But,
> without even waiting for a reply from me, you issued multiple patches
> which didn't fix anything. In fact your attempt to remove duplicated code
> breaks things because of the status-less nature of CB transport. Of
Could you explain? Moving 3 copies of same code onto subroutine just
cant break anything.
> course, you didn't know that. You also didn't know that the reduction of
> common code is on my TODO list. But it's well behind getting functional
> bugs (like the lack of reset functions) done. And every time you bypass
> me (the official maintainer of usb-storage -- check the MAINTAINERS file),
> you slow the process down. That may be fine during most times, but with
> the race for 2.4 on, it's unacceptable.
Ok. I'll stop mailing directly to randy. I did not know storage is
under so fast development.
> I'm always open to bug reports. If you want to submit a suggested
> pseudo-code or even a real patch, go ahead. But you can't go sending
> patches to the list with a phrase like "I hope I did not break anything",
> especially in cases where you have. And, if you do find something wrong,
> a message with more than one line saying something "is completely broken
> in usb-storage", and then have a second line which inaccurately describes
> the problem. You claim, "You may never ever block while you are holding
> spinlock" which is not the case. Correctly, as Vojtech points out, it's
> improper for me to hold the _irqsave spinlock. Tho, oddly enough,
> it
Vojtech is wrong at this point. Schedule() can take second or so, and
if you hold spinlock for a second, other cpu is wasting second! That's
why you may not hold spinlock over schedule().
And besides, you do not need spinlocks at all, since _disconnect() is
called from process context. (Put if (in_interrupt()) panic("???")
inside _disconnect() if you do not believe me.) Therefore normal locks
(up and down) are completely okay, and you do not need to protect
->pusb_dev (or how it is called) entry at all -- just check it before
each use.
That is what my patch did. [I thought patch is better than
description. Looks like I was wrong.]
> seems to work for me on my system. But Vojtech is correct, and I will fix
> this tomorrow morning.
Of course it works for you. It worked quite well even with no locking
at all. (Do you have SMP machine?)
Pavel
--
The best software in life is free (not shareware)! Pavel
GCM d? s-: !g p?:+ au- a--@ w+ v- C++@ UL+++ L++ N++ E++ W--- M- Y- R+
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
