2004-09-10 08:26:23

by Norbert Preining

[permalink] [raw]
Subject: 2.6.9-rc1-mm4, visor.c, Badness in usb_unlink_urb

Hi!

Syncing my PalmOne T|C I get a lot of error messages (see below).
Suprisingly this didn't disturb my palm to be synced.



vmunix: usb 4-2.1: new full speed USB device using address 6
vmunix: visor 4-2.1:1.0: Handspring Visor / Palm OS converter detected
vmunix: usb 4-2.1: Handspring Visor / Palm OS converter now attached to ttyUSB0
vmunix: usb 4-2.1: Handspring Visor / Palm OS converter now attached to ttyUSB1
udev[14677]: configured rule in '/etc/udev/rules.d/00-local.rules' at line 3 applied, added symlink 'pilot'
udev[14675]: creating device node '/dev/ttyUSB0'
udev[14677]: creating device node '/dev/ttyUSB1'
vmunix: usb_unlink_urb() is deprecated for synchronous unlinks. Use usb_kill_urb()
vmunix: Badness in usb_unlink_urb at drivers/usb/core/urb.c:456
vmunix: [<c02616fe>] usb_unlink_urb+0x7e/0x90
vmunix: [<c0287e82>] visor_close+0x22/0xe0
vmunix: [<c028548b>] serial_close+0x9b/0x110
vmunix: [<c01f8fcd>] release_dev+0x62d/0x650
vmunix: [<c01f7eaa>] tty_read+0xda/0x130
vmunix: [<c01f941f>] tty_release+0x1f/0x50
vmunix: [<c014f38e>] __fput+0xfe/0x110
vmunix: [<c014dc0f>] filp_close+0x4f/0x80
vmunix: [<c0115520>] do_page_fault+0x0/0x550
vmunix: [<c0103ff5>] sysenter_past_esp+0x52/0x71
vmunix: usb 4-2.1: USB disconnect, address 6
vmunix: [<c0285150>] destroy_serial+0x0/0x150
vmunix: [<c01ba954>] kref_put+0x34/0xa0
vmunix: [<c0286b98>] usb_serial_disconnect+0x38/0x90
vmunix: [<c026272d>] usb_disable_endpoint+0x3d/0x40
vmunix: [<c0262756>] usb_disable_interface+0x26/0x40
vmunix: [<c025c130>] usb_unbind_interface+0x60/0x70
vmunix: [<c02183e6>] device_release_driver+0x56/0x60
vmunix: [<c0218612>] bus_remove_device+0x52/0x90
vmunix: [<c02175fa>] device_del+0x5a/0x90
vmunix: [<c026280c>] usb_disable_device+0x9c/0xd0
vmunix: [<c025e0f1>] usb_disconnect+0xa1/0x130
vmunix: [<c025f12b>] hub_port_connect_change+0x36b/0x390
vmunix: [<c025f389>] hub_events+0x239/0x360
vmunix: [<c0122241>] free_uid+0x11/0x80
vmunix: [<c025f4e5>] hub_thread+0x35/0x110
vmunix: [<c012c390>] autoremove_wake_function+0x0/0x50
vmunix: [<c0103f1e>] ret_from_fork+0x6/0x14
vmunix: [<c012c390>] autoremove_wake_function+0x0/0x50
vmunix: [<c025f4b0>] hub_thread+0x0/0x110
vmunix: [<c010227d>] kernel_thread_helper+0x5/0x18
vmunix: usb_unlink_urb() is deprecated for synchronous unlinks. Use usb_kill_urb()
kernel: visor ttyUSB0: Handspring Visor / Palm OS converter now disconnected from ttyUSB0
vmunix: Badness in usb_unlink_urb at drivers/usb/core/urb.c:45ad+0x0/0x110
vmunix: [<c010227d>] kernel_thread_helper+0x5/0x18
vmunix: visor 4-2.1:1.0: device disconnected
kernel: usb_unlink_urb() is deprecated for synchronous unlinks. Use usb_kill_urb()
kernel: Badness in usb_unlink_urb at drivers/usb/core/urb.c:456
kernel: [usb_unlink_urb+126/144] usb_unlink_urb+0x7e/0x90
kernel: [port_release+114/176] port_release+0x72/0xb0
kernel: [device_release+83/96] device_release+0x53/0x60
kernel: [kobject_cleanup+142/144] kobject_cleanup+0x8e/0x90
kernel: [kobject_release+0/16] kobject_release+0x0/0x10
kernel: [kref_put+52/160] kref_put+0x34/0xa0
kernel: [device_del+105/144] device_del+0x69/0x90
kernel: [destroy_serial+266/336] destroy_serial+0x10a/0x150
kernel: [hcd_endpoint_disable+219/432] hcd_endpoint_disable+0xdb/0x1b0
kernel: 6
kernel: [usb_unlink_urb+126/144] usb_unlink_urb+0x7e/0x90
kernel: [port_release+99/176] port_release+0x63/0xb0
kernel: [device_release+83/96] device_release+0x53/0x60
kernel: [kobject_cleanup+142/144] kobject_cleanup+0x8e/0x90
kernel: [kobject_release+0/16] kobject_release+0x0/0x10
kernel: [kref_put+52/160] kref_put+0x34/0xa0
kernel: [device_del+105/144] device_del+0x69/0x90
kernel: [destroy_serial+266/336] destroy_serial+0x10a/0x150
kernel: [hcd_endpoint_disable+219/432] hcd_endpoint_disable+0xdb/0x1b0
kernel: [destroy_serial+0/336] destroy_serial+0x0/0x150
kernel: [kref_put+52/160] kref_put+0x34/0xa0
kernel: [usb_serial_disconnect+56/144] usb_serial_disconnect+0x38/0x90
kernel: [usb_disable_endpoint+61/64] usb_disable_endpoint+0x3d/0x40
kernel: [usb_disable_interface+38/64] usb_disable_interface+0x26/0x40
kernel: [usb_unbind_interface+96/112] usb_unbind_interface+0x60/0x70
kernel: [device_release_driver+86/96] device_release_driver+0x56/0x60
kernel: [bus_remove_device+82/144] bus_remove_device+0x52/0x90
kernel: [device_del+90/144] device_del+0x5a/0x90
kernel: [usb_disable_device+156/208] usb_disable_device+0x9c/0xd0
kernel: [usb_disconnect+161/304] usb_disconnect+0xa1/0x130
kernel: [hub_port_connect_change+875/912] hub_port_connect_change+0x36b/0x390
kernel: [hub_events+569/864] hub_events+0x239/0x360
kernel: [free_uid+17/128] free_uid+0x11/0x80
kernel: [hub_thread+53/272] hub_thread+0x35/0x110
kernel: [autoremove_wake_function+0/80] autoremove_wake_function+0x0/0x50
kernel: [ret_from_fork+6/20] ret_from_fork+0x6/0x14
kernel: [autoremove_wake_function+0/80] autoremove_wake_function+0x0/0x50
kernel: [hub_thread+0/272] hub_thread+0x0/0x110
kernel: [kernel_thread_helper+5/24] kernel_thread_helper+0x5/0x18
kernel: visor ttyUSB1: Handspring Visor / Palm OS converter now disconnected from ttyUSB1
kernel: usb_unlink_urb() is deprecated for synchronous unlinks. Use usb_kill_urb()
kernel: Badness in usb_unlink_urb at drivers/usb/core/urb.c:456
kernel: [usb_unlink_urb+126/144] usb_unlink_urb+0x7e/0x90
kernel: [port_release+114/176] port_release+0x72/0xb0
kernel: [device_release+83/96] device_release+0x53/0x60
kernel: [kobject_cleanup+142/144] kobject_cleanup+0x8e/0x90
kernel: [kobject_release+0/16] kobject_release+0x0/0x10
kernel: [kref_put+52/160] kref_put+0x34/0xa0
kernel: [device_del+105/144] device_del+0x69/0x90
kernel: [destroy_serial+266/336] destroy_serial+0x10a/0x150
kernel: [hcd_endpoint_disable+219/432] hcd_endpoint_disable+0xdb/0x1b0
kernel: [destroy_serial+0/336] destroy_serial+0x0/0x150
kernel: [kref_put+52/160] kref_put+0x34/0xa0
kernel: [usb_serial_disconnect+56/144] usb_serial_disconnect+0x38/0x90
kernel: [usb_disable_endpoint+61/64] usb_disable_endpoint+0x3d/0x40
kernel: [usb_disable_interface+38/64] usb_disable_interface+0x26/0x40
kernel: [usb_unbind_interface+96/112] usb_unbind_interface+0x60/0x70
kernel: [device_release_driver+86/96] device_release_driver+0x56/0x60
kernel: [bus_remove_device+82/144] bus_remove_device+0x52/0x90
kernel: [device_del+90/144] device_del+0x5a/0x90
kernel: [usb_disable_device+156/208] usb_disable_device+0x9c/0xd0
kernel: [usb_disconnect+161/304] usb_disconnect+0xa1/0x130
kernel: [hub_port_connect_change+875/912] hub_port_connect_change+0x36b/0x390
kernel: [hub_events+569/864] hub_events+0x239/0x360
kernel: [free_uid+17/128] free_uid+0x11/0x80
kernel: [hub_thread+53/272] hub_thread+0x35/0x110
kernel: [autoremove_wake_function+0/80] autoremove_wake_function+0x0/0x50
kernel: [ret_from_fork+6/20] ret_from_fork+0x6/0x14
kernel: [autoremove_wake_function+0/80] autoremove_wake_function+0x0/0x50
kernel: [hub_thread+0/272] hub_thread+0x0/0x110
kernel: [kernel_thread_helper+5/24] kernel_thread_helper+0x5/0x18
kernel: usb_unlink_urb() is deprecated for synchronous unlinks. Use usb_kill_urb()
kernel: Badness in usb_unlink_urb at drivers/usb/core/urb.c:456
kernel: [usb_unlink_urb+126/144] usb_unlink_urb+0x7e/0x90
kernel: [port_release+99/176] port_release+0x63/0xb0
kernel: [device_release+83/96] device_release+0x53/0x60
kernel: [kobject_cleanup+142/144] kobject_cleanup+0x8e/0x90
kernel: [kobject_release+0/16] kobject_release+0x0/0x10
kernel: [kref_put+52/160] kref_put+0x34/0xa0
kernel: [device_del+105/144] device_del+0x69/0x90
kernel: [destroy_serial+266/336] destroy_serial+0x10a/0x150
kernel: [hcd_endpoint_disable+219/432] hcd_endpoint_disable+0xdb/0x1b0
kernel: [destroy_serial+0/336] destroy_serial+0x0/0x150
kernel: [kref_put+52/160] kref_put+0x34/0xa0
kernel: [usb_serial_disconnect+56/144] usb_serial_disconnect+0x38/0x90
kernel: [usb_disable_endpoint+61/64] usb_disable_endpoint+0x3d/0x40
kernel: [usb_disable_interface+38/64] usb_disable_interface+0x26/0x40
kernel: [usb_unbind_interface+96/112] usb_unbind_interface+0x60/0x70
kernel: [device_release_driver+86/96] device_release_driver+0x56/0x60
kernel: [bus_remove_device+82/144] bus_remove_device+0x52/0x90
kernel: [device_del+90/144] device_del+0x5a/0x90
kernel: [usb_disable_device+156/208] usb_disable_device+0x9c/0xd0
kernel: [usb_disconnect+161/304] usb_disconnect+0xa1/0x130
kernel: [hub_port_connect_change+875/912] hub_port_connect_change+0x36b/0x390
kernel: [hub_events+569/864] hub_events+0x239/0x360
kernel: [free_uid+17/128] free_uid+0x11/0x80
kernel: [hub_thread+53/272] hub_thread+0x35/0x110
kernel: [autoremove_wake_function+0/80] autoremove_wake_function+0x0/0x50
kernel: [ret_from_fork+6/20] ret_from_fork+0x6/0x14
kernel: [autoremove_wake_function+0/80] autoremove_wake_function+0x0/0x50
udev[15601]: removing device node '/dev/ttyUSB0'
udev[15608]: removing device node '/dev/ttyUSB1'

Best wishes

Norbert

-------------------------------------------------------------------------------
Norbert Preining <preining AT logic DOT at> Technische Universit?t Wien
gpg DSA: 0x09C5B094 fp: 14DF 2E6C 0307 BE6D AD76 A9C0 D2BF 4AA3 09C5 B094
-------------------------------------------------------------------------------
YORK (vb.)
To shift the position of the shoulder straps on a heavy bag or
rucksack in a vain attempt to make it seem lighter. Hence : to laugh
falsely and heartily at an unfunny remark. 'Jasmine yorked politely,
loathing him to the depths of her being' - Virginia Woolf.
--- Douglas Adams, The Meaning of Liff


2004-09-10 09:52:38

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 0/2] Re: 2.6.9-rc1-mm4, visor.c, Badness in usb_unlink_urb

On Friday 10 September 2004 10:26, Norbert Preining wrote:
> Hi!
>
<snip>
Hi Norbert, Greg,
yesterday we had the same problem so here you go.


> Best wishes
>
> Norbert
>
> ---------------------------------------------------------------------------
>---- Norbert Preining <preining AT logic DOT at> Technische
> Universit?t Wien gpg DSA: 0x09C5B094 fp: 14DF 2E6C 0307 BE6D AD76
> A9C0 D2BF 4AA3 09C5 B094
> ---------------------------------------------------------------------------
>---- YORK (vb.)
> To shift the position of the shoulder straps on a heavy bag or
> rucksack in a vain attempt to make it seem lighter. Hence : to laugh
> falsely and heartily at an unfunny remark. 'Jasmine yorked politely,
> loathing him to the depths of her being' - Virginia Woolf.
> --- Douglas Adams, The Meaning of Liff
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2004-09-10 09:52:37

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 1/2] Re: 2.6.9-rc1-mm4, visor.c, Badness in usb_unlink_urb

remove the deprecated call to usb_unlink_urb.

Signed-off-by: Borislav Petkov <[email protected]>

--- linux-2.6.9-rc1-mm/drivers/usb/serial/visor.c.orig 2004-09-10
11:35:11.000000000 +0200
+++ linux-2.6.9-rc1-mm/drivers/usb/serial/visor.c 2004-09-10
11:35:55.000000000 +0200
@@ -446,9 +446,9 @@ static void visor_close (struct usb_seri
dbg("%s - port %d", __FUNCTION__, port->number);

/* shutdown our urbs */
- usb_unlink_urb (port->read_urb);
+ usb_kill_urb (port->read_urb);
if (port->interrupt_in_urb)
- usb_unlink_urb (port->interrupt_in_urb);
+ usb_kill_urb (port->interrupt_in_urb);

/* Try to send shutdown message, if the device is gone, this will just fail.
*/
transfer_buffer = kmalloc (0x12, GFP_KERNEL);

2004-09-10 09:57:35

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 2/2] Re: 2.6.9-rc1-mm4, sub-serial.c, Badness in usb_unlink_urb

Remove the deprecated call to usb_unlink_urb in usb-serial.c

Signed-off-by: Borislav Petkov <[email protected]>

--- linux-2.6.9-rc1-mm/drivers/usb/serial/usb-serial.c.orig 2004-09-10
11:41:16.000000000 +0200
+++ linux-2.6.9-rc1-mm/drivers/usb/serial/usb-serial.c 2004-09-10
11:41:44.000000000 +0200
@@ -819,15 +819,15 @@ static void port_release(struct device *

dbg ("%s - %s", __FUNCTION__, dev->bus_id);
if (port->read_urb) {
- usb_unlink_urb(port->read_urb);
+ usb_kill_urb(port->read_urb);
usb_free_urb(port->read_urb);
}
if (port->write_urb) {
- usb_unlink_urb(port->write_urb);
+ usb_kill_urb(port->write_urb);
usb_free_urb(port->write_urb);
}
if (port->interrupt_in_urb) {
- usb_unlink_urb(port->interrupt_in_urb);
+ usb_kill_urb(port->interrupt_in_urb);
usb_free_urb(port->interrupt_in_urb);
}
kfree(port->bulk_in_buffer);

2004-09-10 14:13:19

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 1/2] Re: 2.6.9-rc1-mm4, visor.c, Badness in usb_unlink_urb

On Fri, Sep 10, 2004 at 11:48:37AM +0200, Borislav Petkov wrote:
> remove the deprecated call to usb_unlink_urb.
>
> Signed-off-by: Borislav Petkov <[email protected]>
>
> --- linux-2.6.9-rc1-mm/drivers/usb/serial/visor.c.orig 2004-09-10
> 11:35:11.000000000 +0200

Ick, your patch was line wrapped, and all of the tabs stripped out of it
:(

Anyway, I converted all of the files in drivers/usb/serial/* that needed
it to usb_kill_urb() yesterday, and checked them into my bk-usb tree and
will show up in the next -mm release.

But if you want to tackle the other drivers in the tree, please do so :)

thanks,

greg k-h

2004-09-10 15:56:14

by Greg KH

[permalink] [raw]
Subject: Re: 2.6.9-rc1-mm4, visor.c, Badness in usb_unlink_urb

On Fri, Sep 10, 2004 at 10:26:01AM +0200, Norbert Preining wrote:
> Hi!
>
> Syncing my PalmOne T|C I get a lot of error messages (see below).
> Suprisingly this didn't disturb my palm to be synced.

Fixed and will show up in the next -mm released.

thanks,

greg k-h

2004-09-10 18:24:39

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.6.9-rc1-mm4, visor.c, Badness in usb_unlink_urb

Hi Greg,
I'll be happy to tackle the other drivers. Question: Does usb_unlink_urb have
to be replaced only for synchronious unlinking and if so, is the wrapping
function responsible for checking the URB_ASYNC_UNLINK transfer flag? Here's
a patch:

Signed-off-by: Borislav Petkov <[email protected]>

--- linux-2.6.9-rc1-mm/drivers/usb/class/audio.c.orig 2004-09-10 20:05:17.000000000 +0200
+++ linux-2.6.9-rc1-mm/drivers/usb/class/audio.c 2004-09-10 20:23:12.000000000 +0200
@@ -635,13 +635,13 @@ static void usbin_stop(struct usb_audiod
spin_unlock_irqrestore(&as->lock, flags);
if (notkilled && signal_pending(current)) {
if (i & FLG_URB0RUNNING)
- usb_unlink_urb(u->durb[0].urb);
+ usb_kill_urb(u->durb[0].urb);
if (i & FLG_URB1RUNNING)
- usb_unlink_urb(u->durb[1].urb);
+ usb_kill_urb(u->durb[1].urb);
if (i & FLG_SYNC0RUNNING)
- usb_unlink_urb(u->surb[0].urb);
+ usb_kill_urb(u->surb[0].urb);
if (i & FLG_SYNC1RUNNING)
- usb_unlink_urb(u->surb[1].urb);
+ usb_kill_urb(u->surb[1].urb);
notkilled = 0;
}
}
@@ -1114,13 +1114,13 @@ static void usbout_stop(struct usb_audio
spin_unlock_irqrestore(&as->lock, flags);
if (notkilled && signal_pending(current)) {
if (i & FLG_URB0RUNNING)
- usb_unlink_urb(u->durb[0].urb);
+ usb_kill_urb(u->durb[0].urb);
if (i & FLG_URB1RUNNING)
- usb_unlink_urb(u->durb[1].urb);
+ usb_kill_urb(u->durb[1].urb);
if (i & FLG_SYNC0RUNNING)
- usb_unlink_urb(u->surb[0].urb);
+ usb_kill_urb(u->surb[0].urb);
if (i & FLG_SYNC1RUNNING)
- usb_unlink_urb(u->surb[1].urb);
+ usb_kill_urb(u->surb[1].urb);
notkilled = 0;
}
}

2004-09-14 07:55:27

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.6.9-rc1-mm4, visor.c, Badness in usb_unlink_urb

On Fri, Sep 10, 2004 at 08:24:31PM +0200, Borislav Petkov wrote:
> Hi Greg,
> I'll be happy to tackle the other drivers. Question: Does usb_unlink_urb have
> to be replaced only for synchronious unlinking

Yes.

> and if so, is the wrapping function responsible for checking the
> URB_ASYNC_UNLINK transfer flag? Here's a patch:

The driver should know how to shut down the urb.

> Signed-off-by: Borislav Petkov <[email protected]>
>
> --- linux-2.6.9-rc1-mm/drivers/usb/class/audio.c.orig 2004-09-10 20:05:17.000000000 +0200
> +++ linux-2.6.9-rc1-mm/drivers/usb/class/audio.c 2004-09-10 20:23:12.000000000 +0200
> @@ -635,13 +635,13 @@ static void usbin_stop(struct usb_audiod
> spin_unlock_irqrestore(&as->lock, flags);
> if (notkilled && signal_pending(current)) {
> if (i & FLG_URB0RUNNING)
> - usb_unlink_urb(u->durb[0].urb);
> + usb_kill_urb(u->durb[0].urb);

Ick, the tabs are stripped out :(

thanks,

greg k-h