On Mon, Mar 25, 2002 at 01:52:27AM +0100, Martin Diehl wrote:
> On Fri, 22 Mar 2002, Jean Tourrilhes wrote:
>
> > P.S. Martin : please tell me about the USB patch.
>
> Hi Jean,
>
> first goes my apology for the long delay since we discussed these usb
> serialization issues. My feeling was this deserves some more thinking
> and investigation and I was (aehm, I'm) too overloaded - you know :)
No apologies needed, same applies to me and almost everybody
around here.
> Ok, just did some testing with 2.5.7 with your ir257_usb_disconnect_atomic
> applied. Despite trying all my best to break it, I didn't manage.
;-)
> What
> I've done was some play with disconnect during heavy ftp-xfer at 4Mbps.
> But only unplugging by hand. Of course, one would get better coverage if
> one would use some automated tool doing forced disconnect via usbfs for
> example. But since this would be another project and unplugging 1000 times
> by hand isn't fun, the resulting claim is "didn't _manage_ to break it".
> Testbox was UP, buth with SMP kernel. So most of the races we were
> thinking about could not even happen there.
>
> Did some additional walk through the driver. Your solution with the
> spinlock is somewhat different from what I was thinking about and what is
> used for a number of usb-drivers (see usb-skeleton f.e.).
I pride myself in beeing an original. Actually, I think my
solution is somewhat simpler and more efficient. And I like simple.
> But anyway,
> AFAICS it looks good to me. These are the remaining comments from me:
>
> - IMHO you missed one place in netdev->ioctl, where the test of
> self->present is not protected by the spinlock.
Correct. I was not sure which way to go about this one.
> - we were discussing the option to simply drop tx packets on lock
> contention. You were pointing out the packet might have PF set.
> If it is a rare case, this wouldn't be a problem IMHO - the packet
> might get lost in the air as well. We just loose up to 500ms before
> the stack recovers (hope so). AFAICS the worst thing that could
> happen when dropping packets is if it is a speed-changing packet.
> I'm unsure how the stack behaves if we loose this, but I agree it
> would be much prefered not to loose this particular one.
> Seems your solution doesn't have this problem.
Tx for the network layer will be strictly serialised, because
of the netif_stop_queue(). You can get lock contention only if you
access the driver directly.
I refuse to drop the packet, because the probability of having
a pf bit in it is greater than 50%. The better solution would be to
"return 1" which would force the network layer to requeue the packet
and deliver it at a later date.
> - what concerns me somewhat is the potentially long time we hold the
> spinlock with irq disabled during xmit. Yes, mtt is disabled by default,
> but what if a dongle needs IUC_NO_TURN and peer mtt 5msec? OTOH, we have
> similar problems for almost all irda-drivers. But I think if we could
> use a semaphore or similar to serialize with disconnect, it would be
> possible to disable irq's only after the mtt has passed.
Other IrDA driver works very differently with regards to mtt,
and only the USB hardware is braindamaged.
Yes, I agree that this is the most ugly part of the driver.
> In conclusion, I think this patch is really fine to apply, probably with
> the spinlock extended in netdev->ioctl.
Good.
> Martin
>
> PS: Besides the above mentioned tests using usb-ohci, I've also tried
> with usb-uhci and uhci with particular focus on the problems you've
> reported to linux-usb-devel. See my separate reply there.
;-)
Jean
_______________________________________________
Linux-IrDA mailing list - [EMAIL PROTECTED]
http://www.pasta.cs.UiT.No/mailman/listinfo/linux-irda
