On 22.02.2012 16:19, Stephane Grosjean wrote:

Le 22/02/2012 08:30, Oliver Hartkopp a écrit :

quoted

On 22.02.2012 08:05, Oliver Hartkopp wrote:


Just printing this status and continue with business as usual
(netif_wake_queue!) is probably not the right approach 8-)

Please check if it makes sense to check (urb->status != 0) earlier in this
callback function.

Hi Oliver,

I took time to have a look to how things are done in other (net+usb) drivers
and it's not easy to conclude to anything: some are testing that status (but
with some other errno values) while others are always waking up the netdev
queues... So, I tried to do a "mixed-of", cleaned up the code and tested it in
(something like) your conditions, that is:

PC1:
can0 -> usb-pro ch1 -+
can1 -> usb-pro ch2  |
can2 -> usb ---------+
                     | 1Mpbs
PC2:                 |
can0 -> pcmcia ch1 --+

PC1:
cangen -g0 -i can0
cangen -g0 -i can2
candump any

I did the two following tests:

- removing the peak_usb driver while sending/reading data:

[ 4354.534576] peak_usb 1-1:1.0: can0: attached to PCAN-USB Pro channel 0
(device 1898240)
[ 4354.535574] peak_usb 1-1:1.0: can1: attached to PCAN-USB Pro channel 1
(device 2)
[ 4360.388020] usb 4-1: new full speed USB device number 7 using uhci_hcd
[ 4360.588035] peak_usb 4-1:1.0: PEAK-System PCAN-USB adapter hwrev 28 serial
FFFFFFFF (1 channel)
[ 4360.592034] peak_usb 4-1:1.0: can2: attached to PCAN-USB channel 0 (device
255)
[ 4363.753037] peak_usb 1-1:1.0: can0: setting ccbt=0x00140006
[ 4363.770637] peak_usb 1-1:1.0: can1: setting ccbt=0x00140006
[ 4363.787775] peak_usb 4-1:1.0: can2: setting BTR0=0x00 BTR1=0x14
[ 4385.878968] usbcore: deregistering interface driver peak_usb
[ 4385.920049] peak_usb 4-1:1.0: can2 removed
[ 4385.948036] peak_usb 1-1:1.0: can1 removed
[ 4385.964042] peak_usb 1-1:1.0: can0 removed
[ 4385.964076] peak_usb: PCAN-USB interfaces driver unloaded

=> ok

Hm - i didn't test that so far.

But nice it works :-)

- unplug the usb then usb-pro adapters while sending/reading data:

[ 5043.008127] peak_usb 4-1:1.0: PEAK-System PCAN-USB adapter hwrev 28 serial
FFFFFFFF (1 channel)
[ 5043.012133] peak_usb 4-1:1.0: can0: attached to PCAN-USB channel 0 (device
255)
[ 5048.029973] peak_usb 1-1:1.0: PEAK-System PCAN-USB Pro hwrev 0 serial
FFFFFFFF.00000001 (2 channels)
[ 5048.030596] peak_usb 1-1:1.0: can1: attached to PCAN-USB Pro channel 0
(device 1898240)
[ 5048.031717] peak_usb 1-1:1.0: can2: attached to PCAN-USB Pro channel 1
(device 2)
[ 5053.795570] peak_usb 4-1:1.0: can0: setting BTR0=0x00 BTR1=0x14
[ 5053.825418] peak_usb 1-1:1.0: can1: setting ccbt=0x00140006
[ 5053.844272] peak_usb 1-1:1.0: can2: setting ccbt=0x00140006
[ 5081.440039] usb 4-1: USB disconnect, device number 8
[ 5081.468028] peak_usb 4-1:1.0: can0 removed
[ 5096.856274] usb 1-1: USB disconnect, device number 16
[ 5096.876038] peak_usb 1-1:1.0: can2 removed
[ 5096.896051] peak_usb 1-1:1.0: can1 removed

Doesn't fit to your described setup ... anyway ...

(then rmmod):

[ 6489.510730] usbcore: deregistering interface driver peak_usb
[ 6489.510756] peak_usb: PCAN-USB interfaces driver unloaded

=> Ok

(I definitively prefer my logs to yours ;-)

i can imagine that :-)

Pls try the following:

PC1:
can0 -> usb-pro ch1 ----+
can1 -> usb-pro ch2 -+  |
can2 -> usb ---------+  |
                     |  | 1Mpbs
PC2:                 |  |
can0 -> pcmcia ch1 --+  |
can1 -> pcmcia ch2 -----+

PC1:
cangen -g0 -i can0
cangen -g0 -i can1
candump any

PC2:
cangen -g0 -i can0
cangen -g0 -i can1
candump any

and then pull the usb-pro from PC1 ...

That's really unfriendly 8-)

In my opinion, the Kernel hangs you got don't come from the netif_wake_queue()
wrong usage, but these tests lead to run some failure cases in the driver with
potential bad memory usage... So I did a pass on (I hope) all of these cases
too. Anyway, this should be better now.

So, if you don't have any other comments or any other (not very friendly)
tests, I'm ready to send a new version of the peak_usb.

Please check this use-case above. And when it works with your fixed driver you
can probalby check the 'old' v5 version to see if you can follow my bug report
on your setup.

Best regards,
Oliver