init 1 kill khubd on 2.6.11

init 1 kill khubd on 2.6.11

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 382 bytes --]

Hub driver is using SIGKILL to terminate khubd. Unfortunately on a number of 
distributions switching init levels implicitly does "killall -9", killing 
khubd. The only way to restart it is to reload USB subsystem.

Is signal usage in this case really needed? What about replacing it with 
simple flag (i.e. will patch be accepted)?

TIA

-andrey

Please Cc me on reply

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [linux-usb-devel] init 1 kill khubd on 2.6.11

[Alan Stern]

On Sun, 1 May 2005, Andrey Borzenkov wrote:

> Hub driver is using SIGKILL to terminate khubd. Unfortunately on a number of 
> distributions switching init levels implicitly does "killall -9", killing 
> khubd. The only way to restart it is to reload USB subsystem.
> 
> Is signal usage in this case really needed? What about replacing it with 
> simple flag (i.e. will patch be accepted)?

IMO the problem lies in those distributions.  They should not
indiscrimately kill processes when switching init levels.

Alan Stern

Re: [linux-usb-devel] init 1 kill khubd on 2.6.11

[Lee Revell]

On Sun, 2005-05-01 at 17:01 -0400, Alan Stern wrote:
> On Sun, 1 May 2005, Andrey Borzenkov wrote:
> 
> > Hub driver is using SIGKILL to terminate khubd. Unfortunately on a number of 
> > distributions switching init levels implicitly does "killall -9", killing 
> > khubd. The only way to restart it is to reload USB subsystem.
> > 
> > Is signal usage in this case really needed? What about replacing it with 
> > simple flag (i.e. will patch be accepted)?
> 
> IMO the problem lies in those distributions.  They should not
> indiscrimately kill processes when switching init levels.

It's probably not indiscriminate, all the init scripts I have seen only
resort to kill -9 if the process fails to terminate in an orderly
fashion via the /etc/rcX.d/Kfoo script.

Re: [linux-usb-devel] init 1 kill khubd on 2.6.11

[Andrew Morton]

Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Sun, 1 May 2005, Andrey Borzenkov wrote:
> 
> > Hub driver is using SIGKILL to terminate khubd. Unfortunately on a number of 
> > distributions switching init levels implicitly does "killall -9", killing 
> > khubd. The only way to restart it is to reload USB subsystem.
> > 
> > Is signal usage in this case really needed? What about replacing it with 
> > simple flag (i.e. will patch be accepted)?
> 
> IMO the problem lies in those distributions.  They should not
> indiscrimately kill processes when switching init levels.

Nevertheless it's better that kernel internals not be exposed to userspace
actions in this manner, and using signals for in-kernel IPC is crufty, IMO.

It's pretty simple to convert khubd to use the kthread API.  Something like
this (untested):

 drivers/usb/core/hub.c |   40 +++++++++++-----------------------------
 1 files changed, 11 insertions(+), 29 deletions(-)

diff -puN drivers/usb/core/hub.c~hub-use-kthread drivers/usb/core/hub.c
--- 25/drivers/usb/core/hub.c~hub-use-kthread	2005-05-01 15:22:24.634539928 -0700
+++ 25-akpm/drivers/usb/core/hub.c	2005-05-01 15:29:55.739961480 -0700
@@ -26,6 +26,7 @@
 #include <linux/ioctl.h>
 #include <linux/usb.h>
 #include <linux/usbdevice_fs.h>
+#include <linux/kthread.h>
 
 #include <asm/semaphore.h>
 #include <asm/uaccess.h>
@@ -47,8 +48,7 @@ static LIST_HEAD(hub_event_list);	/* Lis
 /* Wakes up khubd */
 static DECLARE_WAIT_QUEUE_HEAD(khubd_wait);
 
-static pid_t khubd_pid = 0;			/* PID of khubd */
-static DECLARE_COMPLETION(khubd_exited);
+static struct task_struct *khubd_task;
 
 /* cycle leds on hubs that aren't blinking for attention */
 static int blinkenlights = 0;
@@ -2807,23 +2807,16 @@ loop:
 
 static int hub_thread(void *__unused)
 {
-	/*
-	 * This thread doesn't need any user-level access,
-	 * so get rid of all our resources
-	 */
-
-	daemonize("khubd");
-	allow_signal(SIGKILL);
-
-	/* Send me a signal to get me die (for debugging) */
 	do {
 		hub_events();
-		wait_event_interruptible(khubd_wait, !list_empty(&hub_event_list)); 
+		wait_event_interruptible(khubd_wait,
+				!list_empty(&hub_event_list) ||
+				kthread_should_stop());
 		try_to_freeze(PF_FREEZE);
-	} while (!signal_pending(current));
+	} while (!kthread_should_stop() || !list_empty(&hub_event_list));
 
-	pr_debug ("%s: khubd exiting\n", usbcore_name);
-	complete_and_exit(&khubd_exited, 0);
+	pr_debug("%s: khubd exiting\n", usbcore_name);
+	return 0;
 }
 
 static struct usb_device_id hub_id_table [] = {
@@ -2849,20 +2842,15 @@ static struct usb_driver hub_driver = {
 
 int usb_hub_init(void)
 {
-	pid_t pid;
-
 	if (usb_register(&hub_driver) < 0) {
 		printk(KERN_ERR "%s: can't register hub driver\n",
 			usbcore_name);
 		return -1;
 	}
 
-	pid = kernel_thread(hub_thread, NULL, CLONE_KERNEL);
-	if (pid >= 0) {
-		khubd_pid = pid;
-
+	khubd_task = kthread_run(hub_thread, NULL, "khubd");
+	if (!IS_ERR(khubd_task))
 		return 0;
-	}
 
 	/* Fall through if kernel_thread failed */
 	usb_deregister(&hub_driver);
@@ -2873,12 +2861,7 @@ int usb_hub_init(void)
 
 void usb_hub_cleanup(void)
 {
-	int ret;
-
-	/* Kill the thread */
-	ret = kill_proc(khubd_pid, SIGKILL, 1);
-
-	wait_for_completion(&khubd_exited);
+	kthread_stop(khubd_task);
 
 	/*
 	 * Hub resources are freed for us by usb_deregister. It calls
@@ -2890,7 +2873,6 @@ void usb_hub_cleanup(void)
 	usb_deregister(&hub_driver);
 } /* usb_hub_cleanup() */
 
-
 static int config_descriptors_changed(struct usb_device *udev)
 {
 	unsigned			index;
_

Re: [linux-usb-devel] init 1 kill khubd on 2.6.11

[Nish Aravamudan]

On 5/1/05, Andrew Morton <akpm@osdl.org> wrote:
> Alan Stern <stern@rowland.harvard.edu> wrote:
> >
> > On Sun, 1 May 2005, Andrey Borzenkov wrote:
> >
> > > Hub driver is using SIGKILL to terminate khubd. Unfortunately on a number of
> > > distributions switching init levels implicitly does "killall -9", killing
> > > khubd. The only way to restart it is to reload USB subsystem.
> > >
> > > Is signal usage in this case really needed? What about replacing it with
> > > simple flag (i.e. will patch be accepted)?
> >
> > IMO the problem lies in those distributions.  They should not
> > indiscrimately kill processes when switching init levels.
> 
> Nevertheless it's better that kernel internals not be exposed to userspace
> actions in this manner, and using signals for in-kernel IPC is crufty, IMO.
> 
> It's pretty simple to convert khubd to use the kthread API.  Something like
> this (untested):
> 
>  drivers/usb/core/hub.c |   40 +++++++++++-----------------------------
>  1 files changed, 11 insertions(+), 29 deletions(-)
> 
> diff -puN drivers/usb/core/hub.c~hub-use-kthread drivers/usb/core/hub.c
> --- 25/drivers/usb/core/hub.c~hub-use-kthread   2005-05-01 15:22:24.634539928 -0700
> +++ 25-akpm/drivers/usb/core/hub.c      2005-05-01 15:29:55.739961480 -0700

<snip>

>  static int hub_thread(void *__unused)

<snip>

> -       /* Send me a signal to get me die (for debugging) */
>         do {
>                 hub_events();
> -               wait_event_interruptible(khubd_wait, !list_empty(&hub_event_list));
> +               wait_event_interruptible(khubd_wait,
> +                               !list_empty(&hub_event_list) ||
> +                               kthread_should_stop());
>                 try_to_freeze(PF_FREEZE);
> -       } while (!signal_pending(current));
> +       } while (!kthread_should_stop() || !list_empty(&hub_event_list));

Shouldn't this simply be a wait_event(), instead of
wait_event_interruptible()? Then the do-while() can be gotten rid of,
as the only reason it is there currently, I guess, is to ignore
signals?

Also, the while's conditional should be (!kthread_should_stop() ||
list_empty(&hub_event_list) to match the negation of wait_event's?
(wait_event() expects the condition to stop on, while while() expects
the condition to continue on)

Thanks,
Nish

Re: [linux-usb-devel] init 1 kill khubd on 2.6.11

[Andrew Morton]

Nish Aravamudan <nish.aravamudan@gmail.com> wrote:
>
> > -       /* Send me a signal to get me die (for debugging) */
> >         do {
> >                 hub_events();
> > -               wait_event_interruptible(khubd_wait, !list_empty(&hub_event_list));
> > +               wait_event_interruptible(khubd_wait,
> > +                               !list_empty(&hub_event_list) ||
> > +                               kthread_should_stop());
> >                 try_to_freeze(PF_FREEZE);
> > -       } while (!signal_pending(current));
> > +       } while (!kthread_should_stop() || !list_empty(&hub_event_list));
> 
> Shouldn't this simply be a wait_event(), instead of
> wait_event_interruptible()?

That would cause uninterruptible sleep, which contributes to load average.

> Then the do-while() can be gotten rid of,
> as the only reason it is there currently, I guess, is to ignore
> signals?

Nope, the do-while is a basic part of the daemon's operation: keep doing
stuff until either there's no stuff to do or until we're told to exit.

> Also, the while's conditional should be (!kthread_should_stop() ||
> list_empty(&hub_event_list) to match the negation of wait_event's?
> (wait_event() expects the condition to stop on, while while() expects
> the condition to continue on)

Nope, the wait_event_interruptible test says

  "sleep unless the list is not empty or I am being asked to exit"

the while termination test says

  "loop until the list is empty and I am being asked to stop".

I think.  I had to scratch my head for a while over that code ;)

Re: [linux-usb-devel] init 1 kill khubd on 2.6.11

[Nish Aravamudan]

On 5/1/05, Andrew Morton <akpm@osdl.org> wrote:
> Nish Aravamudan <nish.aravamudan@gmail.com> wrote:
> >
> > > -       /* Send me a signal to get me die (for debugging) */
> > >         do {
> > >                 hub_events();
> > > -               wait_event_interruptible(khubd_wait, !list_empty(&hub_event_list));
> > > +               wait_event_interruptible(khubd_wait,
> > > +                               !list_empty(&hub_event_list) ||
> > > +                               kthread_should_stop());
> > >                 try_to_freeze(PF_FREEZE);
> > > -       } while (!signal_pending(current));
> > > +       } while (!kthread_should_stop() || !list_empty(&hub_event_list));
> >
> > Shouldn't this simply be a wait_event(), instead of
> > wait_event_interruptible()?
> 
> That would cause uninterruptible sleep, which contributes to load average.

True, and this is the argument I face(d) with a lot of the msleep()
changes I made. I guess I would like a comment for this case, where
we're using wait_event_interruptible(), but actually are ignoring the
signals that might make us return early.
 
> > Then the do-while() can be gotten rid of,
> > as the only reason it is there currently, I guess, is to ignore
> > signals?
> 
> Nope, the do-while is a basic part of the daemon's operation: keep doing
> stuff until either there's no stuff to do or until we're told to exit.

I see that now, thanks.

> > Also, the while's conditional should be (!kthread_should_stop() ||
> > list_empty(&hub_event_list) to match the negation of wait_event's?
> > (wait_event() expects the condition to stop on, while while() expects
> > the condition to continue on)
> 
> Nope, the wait_event_interruptible test says
> 
>   "sleep unless the list is not empty or I am being asked to exit"
> 
> the while termination test says
> 
>   "loop until the list is empty and I am being asked to stop".
> 
> I think.  I had to scratch my head for a while over that code ;)

You're right again -- sorry for the noise, I must have been reading it
wrong. Rewriting it as !(kthread_should_stop() &&
list_empty(&hub_event_list)) helped me :)

Thanks!
Nish

Re: [linux-usb-devel] init 1 kill khubd on 2.6.11

[Dmitry Torokhov]

On Sunday 01 May 2005 17:46, Nish Aravamudan wrote:
> On 5/1/05, Andrew Morton <akpm@osdl.org> wrote:
> > Alan Stern <stern@rowland.harvard.edu> wrote:
> > >
> > > On Sun, 1 May 2005, Andrey Borzenkov wrote:
> > >
> > > > Hub driver is using SIGKILL to terminate khubd. Unfortunately on a number of
> > > > distributions switching init levels implicitly does "killall -9", killing
> > > > khubd. The only way to restart it is to reload USB subsystem.
> > > >
> > > > Is signal usage in this case really needed? What about replacing it with
> > > > simple flag (i.e. will patch be accepted)?
> > >
> > > IMO the problem lies in those distributions.  They should not
> > > indiscrimately kill processes when switching init levels.
> > 
> > Nevertheless it's better that kernel internals not be exposed to userspace
> > actions in this manner, and using signals for in-kernel IPC is crufty, IMO.
> > 
> > It's pretty simple to convert khubd to use the kthread API.  Something like
> > this (untested):
> > 
> >  drivers/usb/core/hub.c |   40 +++++++++++-----------------------------
> >  1 files changed, 11 insertions(+), 29 deletions(-)
> > 
> > diff -puN drivers/usb/core/hub.c~hub-use-kthread drivers/usb/core/hub.c
> > --- 25/drivers/usb/core/hub.c~hub-use-kthread   2005-05-01 15:22:24.634539928 -0700
> > +++ 25-akpm/drivers/usb/core/hub.c      2005-05-01 15:29:55.739961480 -0700
> 
> <snip>
> 
> >  static int hub_thread(void *__unused)
> 
> <snip>
> 
> > -       /* Send me a signal to get me die (for debugging) */
> >         do {
> >                 hub_events();
> > -               wait_event_interruptible(khubd_wait, !list_empty(&hub_event_list));
> > +               wait_event_interruptible(khubd_wait,
> > +                               !list_empty(&hub_event_list) ||
> > +                               kthread_should_stop());
> >                 try_to_freeze(PF_FREEZE);
> > -       } while (!signal_pending(current));
> > +       } while (!kthread_should_stop() || !list_empty(&hub_event_list));
> 
> Shouldn't this simply be a wait_event(), instead of
> wait_event_interruptible()? Then the do-while() can be gotten rid of,
> as the only reason it is there currently, I guess, is to ignore
> signals?

You need "_interruptible" so your thread can enter refrigerator. Without it
you won't be able to suspend...

-- 
Dmitry

Re: [linux-usb-devel] init 1 kill khubd on 2.6.11

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 8764 bytes --]

On Monday 02 May 2005 02:30, Andrew Morton wrote:
> Alan Stern <stern@rowland.harvard.edu> wrote:
> > On Sun, 1 May 2005, Andrey Borzenkov wrote:
> > > Hub driver is using SIGKILL to terminate khubd. Unfortunately on a
> > > number of distributions switching init levels implicitly does "killall
> > > -9", killing khubd. The only way to restart it is to reload USB
> > > subsystem.
> > >
> > > Is signal usage in this case really needed? What about replacing it
> > > with simple flag (i.e. will patch be accepted)?
> >
> > IMO the problem lies in those distributions.  They should not
> > indiscrimately kill processes when switching init levels.
>
> Nevertheless it's better that kernel internals not be exposed to userspace
> actions in this manner, and using signals for in-kernel IPC is crufty, IMO.
>
> It's pretty simple to convert khubd to use the kthread API.  Something like
> this (untested):
>


Something strange is going on with this patch.

insmod usbcore; insmod uhci-hcd works as expected, finds out all devices, 
triggers hotplug etc. But

{pts/2}% sudo insmod ./usbcore.ko
{pts/2}% sudo mount -t usbfs -o devmode=0664,devgid=43 none /proc/bus/usb
{pts/2}% sudo modprobe usb-interface

results in

usbcore: registered new driver usbfs
usbcore: registered new driver hub
USB Universal Host Controller Interface driver v2.2
ACPI: PCI interrupt 0000:00:1f.2[D] -> GSI 5 (level, low) -> IRQ 5
uhci_hcd 0000:00:1f.2: UHCI Host Controller
PCI: Setting latency timer of device 0000:00:1f.2 to 64
uhci_hcd 0000:00:1f.2: irq 5, io base 0xa400
uhci_hcd 0000:00:1f.2: new USB bus registered, assigned bus number 1
hub 1-0:1.0: USB hub found
hub 1-0:1.0: 2 ports detected
ACPI: PCI interrupt 0000:00:1f.4[C] -> GSI 9 (level, low) -> IRQ 9
uhci_hcd 0000:00:1f.4: UHCI Host Controller
PCI: Setting latency timer of device 0000:00:1f.4 to 64
uhci_hcd 0000:00:1f.4: irq 9, io base 0xa000
usb 1-1: new low speed USB device using uhci_hcd and address 2
uhci_hcd 0000:00:1f.2: Unlink after no-IRQ?  Controller is probably using the 
wrong IRQ.
usb 1-1: khubd timed out on ep0out
usb 1-1: khubd timed out on ep0out
usb 1-1: device not accepting address 2, error -110
usb 1-1: new low speed USB device using uhci_hcd and address 3
usb 1-1: khubd timed out on ep0out
usb 1-1: khubd timed out on ep0out
usb 1-1: device not accepting address 3, error -110
usb 1-1: new low speed USB device using uhci_hcd and address 4
usb 1-1: khubd timed out on ep0in
usb 1-1: khubd timed out on ep0out
usb 1-1: khubd timed out on ep0out
usb 1-1: device not accepting address 4, error -110
usb 1-1: new low speed USB device using uhci_hcd and address 5
usb 1-1: khubd timed out on ep0in
usb 1-1: khubd timed out on ep0out
usb 1-1: khubd timed out on ep0out
usb 1-1: device not accepting address 5, error -110
usb 1-2: new full speed USB device using uhci_hcd and address 6
usb 1-2: khubd timed out on ep0out
usb 1-2: khubd timed out on ep0out
usb 1-2: device not accepting address 6, error -110
usb 1-2: new full speed USB device using uhci_hcd and address 7
usb 1-2: khubd timed out on ep0out
usb 1-2: khubd timed out on ep0out
usb 1-2: device not accepting address 7, error -110
usb 1-2: new full speed USB device using uhci_hcd and address 8
usb 1-2: khubd timed out on ep0in
usb 1-2: khubd timed out on ep0out
usb 1-2: khubd timed out on ep0out
usb 1-2: device not accepting address 8, error -110
usb 1-2: new full speed USB device using uhci_hcd and address 9
usb 1-2: khubd timed out on ep0in
usb 1-2: khubd timed out on ep0out
usb 1-2: khubd timed out on ep0out
usb 1-2: device not accepting address 9, error -110
uhci_hcd 0000:00:1f.4: new USB bus registered, assigned bus number 2
hub 2-0:1.0: USB hub found
hub 2-0:1.0: 2 ports detected
usb 2-2: new full speed USB device using uhci_hcd and address 2
hub 2-2:1.0: USB hub found
hub 2-2:1.0: 4 ports detected

I do not understand what difference mounting usbfs makes. For reference here 
is result of loading the same usbcore without mounting usbfs:

{pts/2}% sudo insmod ./usbcore-new.ko
{pts/2}% sudo modprobe uhci-hcd

usbcore: registered new driver usbfs
usbcore: registered new driver hub
USB Universal Host Controller Interface driver v2.2
ACPI: PCI interrupt 0000:00:1f.2[D] -> GSI 5 (level, low) -> IRQ 5
uhci_hcd 0000:00:1f.2: UHCI Host Controller
PCI: Setting latency timer of device 0000:00:1f.2 to 64
uhci_hcd 0000:00:1f.2: irq 5, io base 0xa400
uhci_hcd 0000:00:1f.2: new USB bus registered, assigned bus number 1
hub 1-0:1.0: USB hub found
hub 1-0:1.0: 2 ports detected
ACPI: PCI interrupt 0000:00:1f.4[C] -> GSI 9 (level, low) -> IRQ 9
uhci_hcd 0000:00:1f.4: UHCI Host Controller
PCI: Setting latency timer of device 0000:00:1f.4 to 64
uhci_hcd 0000:00:1f.4: irq 9, io base 0xa000
uhci_hcd 0000:00:1f.4: new USB bus registered, assigned bus number 2
hub 2-0:1.0: USB hub found
hub 2-0:1.0: 2 ports detected
usb 1-1: new low speed USB device using uhci_hcd and address 2
usb 1-2: new full speed USB device using uhci_hcd and address 3
usb 2-2: new full speed USB device using uhci_hcd and address 2
hub 2-2:1.0: USB hub found
hub 2-2:1.0: 4 ports detected
drivers/usb/class/usblp.c: usblp0: USB Bidirectional printer dev 3 if 0 alt 0 
proto 2 vid 0x03F0 pid 0x1904
usbcore: registered new driver usblp
drivers/usb/class/usblp.c: v0.13: USB Printer Device Class driver
usbcore: registered new driver hiddev
usbhid: probe of 1-1:1.0 failed with error -5
usbcore: registered new driver usbhid
drivers/usb/input/hid-core.c: v2.0:USB HID core driver
usbcore: registered new driver xpad
drivers/usb/input/xpad-core.c: driver for Xbox controllers with mouse 
emulation v0.1.4


-andrey

>  drivers/usb/core/hub.c |   40 +++++++++++-----------------------------
>  1 files changed, 11 insertions(+), 29 deletions(-)
>
> diff -puN drivers/usb/core/hub.c~hub-use-kthread drivers/usb/core/hub.c
> --- 25/drivers/usb/core/hub.c~hub-use-kthread	2005-05-01 15:22:24.634539928
> -0700 +++ 25-akpm/drivers/usb/core/hub.c	2005-05-01 15:29:55.739961480
> -0700 @@ -26,6 +26,7 @@
>  #include <linux/ioctl.h>
>  #include <linux/usb.h>
>  #include <linux/usbdevice_fs.h>
> +#include <linux/kthread.h>
>
>  #include <asm/semaphore.h>
>  #include <asm/uaccess.h>
> @@ -47,8 +48,7 @@ static LIST_HEAD(hub_event_list);	/* Lis
>  /* Wakes up khubd */
>  static DECLARE_WAIT_QUEUE_HEAD(khubd_wait);
>
> -static pid_t khubd_pid = 0;			/* PID of khubd */
> -static DECLARE_COMPLETION(khubd_exited);
> +static struct task_struct *khubd_task;
>
>  /* cycle leds on hubs that aren't blinking for attention */
>  static int blinkenlights = 0;
> @@ -2807,23 +2807,16 @@ loop:
>
>  static int hub_thread(void *__unused)
>  {
> -	/*
> -	 * This thread doesn't need any user-level access,
> -	 * so get rid of all our resources
> -	 */
> -
> -	daemonize("khubd");
> -	allow_signal(SIGKILL);
> -
> -	/* Send me a signal to get me die (for debugging) */
>  	do {
>  		hub_events();
> -		wait_event_interruptible(khubd_wait, !list_empty(&hub_event_list));
> +		wait_event_interruptible(khubd_wait,
> +				!list_empty(&hub_event_list) ||
> +				kthread_should_stop());
>  		try_to_freeze(PF_FREEZE);
> -	} while (!signal_pending(current));
> +	} while (!kthread_should_stop() || !list_empty(&hub_event_list));
>
> -	pr_debug ("%s: khubd exiting\n", usbcore_name);
> -	complete_and_exit(&khubd_exited, 0);
> +	pr_debug("%s: khubd exiting\n", usbcore_name);
> +	return 0;
>  }
>
>  static struct usb_device_id hub_id_table [] = {
> @@ -2849,20 +2842,15 @@ static struct usb_driver hub_driver = {
>
>  int usb_hub_init(void)
>  {
> -	pid_t pid;
> -
>  	if (usb_register(&hub_driver) < 0) {
>  		printk(KERN_ERR "%s: can't register hub driver\n",
>  			usbcore_name);
>  		return -1;
>  	}
>
> -	pid = kernel_thread(hub_thread, NULL, CLONE_KERNEL);
> -	if (pid >= 0) {
> -		khubd_pid = pid;
> -
> +	khubd_task = kthread_run(hub_thread, NULL, "khubd");
> +	if (!IS_ERR(khubd_task))
>  		return 0;
> -	}
>
>  	/* Fall through if kernel_thread failed */
>  	usb_deregister(&hub_driver);
> @@ -2873,12 +2861,7 @@ int usb_hub_init(void)
>
>  void usb_hub_cleanup(void)
>  {
> -	int ret;
> -
> -	/* Kill the thread */
> -	ret = kill_proc(khubd_pid, SIGKILL, 1);
> -
> -	wait_for_completion(&khubd_exited);
> +	kthread_stop(khubd_task);
>
>  	/*
>  	 * Hub resources are freed for us by usb_deregister. It calls
> @@ -2890,7 +2873,6 @@ void usb_hub_cleanup(void)
>  	usb_deregister(&hub_driver);
>  } /* usb_hub_cleanup() */
>
> -
>  static int config_descriptors_changed(struct usb_device *udev)
>  {
>  	unsigned			index;
> _

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [linux-usb-devel] init 1 kill khubd on 2.6.11

[Andrew Morton]

Andrey Borzenkov <arvidjaar@mail.ru> wrote:
>
>  > It's pretty simple to convert khubd to use the kthread API.  Something like
>  > this (untested):
>  >
> 
> 
>  Something strange is going on with this patch.
> 
>  insmod usbcore; insmod uhci-hcd works as expected, finds out all devices, 
>  triggers hotplug etc. But
> 
>  {pts/2}% sudo insmod ./usbcore.ko
>  {pts/2}% sudo mount -t usbfs -o devmode=0664,devgid=43 none /proc/bus/usb
>  {pts/2}% sudo modprobe usb-interface
> 
>  results in
> 
> ...
>  uhci_hcd 0000:00:1f.2: Unlink after no-IRQ?  Controller is probably using the 
>  wrong IRQ.
>  usb 1-1: khubd timed out on ep0out

Does this only happen when the convert-khubd-to-kevent patch is applied?

Re: [linux-usb-devel] init 1 kill khubd on 2.6.11

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 1235 bytes --]

On Monday 02 May 2005 13:30, Andrew Morton wrote:
> Andrey Borzenkov <arvidjaar@mail.ru> wrote:
> >  > It's pretty simple to convert khubd to use the kthread API.  Something
> >  > like this (untested):
> >
> >  Something strange is going on with this patch.
> >
> >  insmod usbcore; insmod uhci-hcd works as expected, finds out all
> > devices, triggers hotplug etc. But
> >
> >  {pts/2}% sudo insmod ./usbcore.ko
> >  {pts/2}% sudo mount -t usbfs -o devmode=0664,devgid=43 none
> > /proc/bus/usb {pts/2}% sudo modprobe usb-interface
> >
> >  results in
> >
> > ...
> >  uhci_hcd 0000:00:1f.2: Unlink after no-IRQ?  Controller is probably
> > using the wrong IRQ.
> >  usb 1-1: khubd timed out on ep0out
>
> Does this only happen when the convert-khubd-to-kevent patch is applied?

(Do you mean patch posted in this thread?)

Now I must admit it does happen without patch too. Sometimes it goes through 
but most of the time it results in those timeouts.

So I confirm that patch posted in this thread fixes original problem (khubd 
killed by SIGKILL).

W.r.t. to timeouts - I appreciate hints where to start debugging. (I am 
downloading vanilla kernel + -mm to give it a try).

regards

-andrey

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [linux-usb-devel] init 1 kill khubd on 2.6.11

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 1142 bytes --]

On Monday 02 May 2005 16:18, Andrey Borzenkov wrote:
> On Monday 02 May 2005 13:30, Andrew Morton wrote:
> > Andrey Borzenkov <arvidjaar@mail.ru> wrote:
> > >  > It's pretty simple to convert khubd to use the kthread API. 
> > >  > Something like this (untested):
> > >
> > >  Something strange is going on with this patch.
> > >
> > >  insmod usbcore; insmod uhci-hcd works as expected, finds out all
> > > devices, triggers hotplug etc. But
> > >
> > >  {pts/2}% sudo insmod ./usbcore.ko
> > >  {pts/2}% sudo mount -t usbfs -o devmode=0664,devgid=43 none
> > > /proc/bus/usb {pts/2}% sudo modprobe usb-interface
> > >
> > >  results in
> > >
> > > ...
> > >  uhci_hcd 0000:00:1f.2: Unlink after no-IRQ?  Controller is probably
> > > using the wrong IRQ.
> > >  usb 1-1: khubd timed out on ep0out
> >
> > Does this only happen when the convert-khubd-to-kevent patch is applied?
...
> (I am
> downloading vanilla kernel + -mm to give it a try).
>

I cannot reproduce it on 2.6.12-rc3[-mm2], with or without patch in this 
thread. It looks like whatever it was it was fixed in the meantime.

regards

-andrey

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]