On Thu, 01 Nov 2007 11:06:24 +0200, Vitaliy Ivanov <[EMAIL PROTECTED]> wrote:
> > Do you still want to go ahead with a 2.4 backport?
>
> Yep. Will do it asap. In latest 2.4 patch we had just a locks issue.
> So do you think I should modify 2.4 patch with all modifications we done in
> 2.6 versio
On Thu, 2007-11-01 at 00:01, Pete Zaitcev wrote:
> Sorry about that. I'll try to be more explicit in the future.
OK, got it. Thanks.
> > I just tried the latest patch and all seems to be good.
> > BTW, slab corruption issue that I saw on the original driver we started
> > fixing on is not an is
Two main issues fixed here are:
- An improper use of in-struct lock to protect an open count
- Use of urb status for -EINPROGRESS
Also, along the way:
- Change usb_unlink_urb to usb_kill_urb. Apparently there's no need
to use usb_unlink_urb whatsoever in this driver, and the old use of
us
On Wed, 31 Oct 2007 13:54:54 +0200, Vitaliy Ivanov <[EMAIL PROTECTED]> wrote:
> On Tue, 2007-10-30 at 23:54, Pete Zaitcev wrote:
>
> > One other small thing. I see you dropped the dev->mtx bracket that
> > I added around the initialization and submission. Notice that the
> > dev->udev is zeroed un
On Tue, 2007-10-30 at 23:54, Pete Zaitcev wrote:
> One other small thing. I see you dropped the dev->mtx bracket that
> I added around the initialization and submission. Notice that the
> dev->udev is zeroed under dev->mtx, not the static lock. This is because
> it has to be seen in read and write
On Tue, 30 Oct 2007 15:09:54 +0200, Vitaliy Ivanov <[EMAIL PROTECTED]> wrote:
> As about read_urb_finished probably it's OK. But we shouldn't decrease
> open_count in the case of error as then we return normal exit value.
Oh, thanks. I fixed that.
One other small thing. I see you dropped the dev
On Tue, 2007-10-30 at 06:24, Pete Zaitcev wrote:
> However, this looks wrong:
>
> > +dev->interrupt_in_endpoint->bInterval);
> > + dev->read_urb_finished = 0;
> > + retval = usb_submit_urb(dev->interrupt_in_urb, GFP_KERNEL);
> > + /* we ignore failure */
> > + /* end o
On Mon, 29 Oct 2007 20:04:57 +0200, Vitaliy Ivanov <[EMAIL PROTECTED]> wrote:
> Finally had a time on my weekend to perform tests and fix Pete's patch a
> little. Now it seems to be correct.
Great!
> 1. One device per user space application. Old driver allowed many users
> application to work
Finally had a time on my weekend to perform tests and fix Pete's patch a
little. Now it seems to be correct.
Added a few changes:
1. One device per user space application. Old driver allowed many users
application to work with same device which can lead to IO mess.
2. GFP_KERNEL is used instead
Greg,
On 10/25/07, Greg KH <[EMAIL PROTECTED]> wrote:
> Hm, I think I'll wait for Pete and you to agree and send me a final
> version before applying anything here :)
I'm testing the final patch Pete created with all the notes.
Will let you know if there are issues with it. If not it can be
appli
On Thu, 25 Oct 2007 14:03:48 +0200, Oliver Neukum <[EMAIL PROTECTED]> wrote:
> Am Donnerstag 25 Oktober 2007 schrieb Pete Zaitcev:
> > + if (signal_pending(current)) {
> > dbg(1," %s : interrupted", __FUNCTION__);
> > +
BTW, the patch in my previous message had one buglet - an extra
remove_wait_queue. It was in an error case though, so it should be
ok to test. But just in case, here's a fixed one.
-- Pete
diff --git a/drivers/usb/misc/adutux.c b/drivers/usb/misc/adutux.c
index c567aa7..6914c0b 100644
--- a/drive
On Wed, 24 Oct 2007 17:09:47 +0300, Vitaliy Ivanov <[EMAIL PROTECTED]> wrote:
> > static void adu_abort_transfers(struct adu_device *dev)
> > {
> ...
> > + mutex_lock(&dev->mtx);
> ...
> > + mutex_unlock(&dev->mtx);
>
> > }
>
> Don't you think it's needed? You call adu_abort_transfers fr
On Wed, Oct 24, 2007 at 04:49:12PM +0200, Oliver Neukum wrote:
> Am Mittwoch 24 Oktober 2007 schrieb Pete Zaitcev:
> > Oliver, thanks for the inftdata catch. I also fixed the sleep_on.
>
> But you are leaving a function while still on a waitqueue left on the stack.
> Here's a patch on top of yours
Am Mittwoch 24 Oktober 2007 schrieb Pete Zaitcev:
> Oliver, thanks for the inftdata catch. I also fixed the sleep_on.
But you are leaving a function while still on a waitqueue left on the stack.
Here's a patch on top of yours.
Regards
Oliver
--- work/drivers/usb/mis
Pete,
On Wed, 2007-10-24 at 04:53, Pete Zaitcev wrote:
1)
> +/*
> + * The locking scheme is a vanilla 3-lock:
> + * adu_device.buflock: A spinlock, covers what IRQs touch.
> + * adutux_mutex: A Static lock to cover open_count. It would also
> cover
> + * any globa
Am Dienstag 23 Oktober 2007 schrieb Pete Zaitcev:
> On Tue, 23 Oct 2007 11:38:37 +0200, Oliver Neukum <[EMAIL PROTECTED]> wrote:
>
> > > + /* XXX Anchor these instead */
> > > + spin_lock_irqsave(&dev->buflock, flags);
> > > + if (!dev->read_urb_finished) {
> > > + spin_unlock_irqr
Two main issues fixed here are:
- An improper use of in-struct lock to protect an open count
- Use of urb status for -EINPROGRESS
Also, along the way:
- Change usb_unlink_urb to usb_kill_urb. Apparently there's no need
to use usb_unlink_urb whatsoever in this driver, and the old use of
us
On Tue, 23 Oct 2007 11:38:37 +0200, Oliver Neukum <[EMAIL PROTECTED]> wrote:
> > + /* XXX Anchor these instead */
> > + spin_lock_irqsave(&dev->buflock, flags);
> > + if (!dev->read_urb_finished) {
> > + spin_unlock_irqrestore(&dev->buflock, flags);
> > + usb_kill_urb(dev
Am Dienstag 23 Oktober 2007 schrieb Pete Zaitcev:
> Two main issues fixed here are:
> - An improper use of in-struct lock to protect an open count
> - Use of urb status for -EINPROGRESS
> + /* XXX Anchor these instead */
> + spin_lock_irqsave(&dev->buflock, flags);
> + if (!dev->read
Two main issues fixed here are:
- An improper use of in-struct lock to protect an open count
- Use of urb status for -EINPROGRESS
Also, along the way:
- Change usb_unlink_urb to usb_kill_urb. Apparently there's no need
to use usb_unlink_urb whatsoever in this driver, and the old use of
us
21 matches
Mail list logo
