hardcoded HZ in hub.c
Hello!
hub.c in 2.4.0-test10 and above contains hardcoded HZ value, which is wrong. Here is the patch:
usb/hub.c renamed from usb/hub.c.orig +1 -1
@@ -813,7 +813,7 @@ ret =kill_proc (khubd_pid,SIGTERM ,1 );if (!ret) {/* Wait 10 seconds */ -int count =10 *100 ; +int count =10 *HZ ;while (khubd_running && --count) { current->state =TASK_INTERRUPTIBLE ;
Bye,
Oleg
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
Quoted 7 lines · Oleg Drokin
hub.cin 2.4.0-test10 and above contains hardcoded HZ value, which is wrong. Here is the patch:
usb/hub.c line 819
Quoted 8 lines
usb/hub.c renamed from usb/hub.c.orig +1
-1@@ -813,7 +813,7 @@ ret =kill_proc (khubd_pid,SIGTERM ,1 );if (!ret) {/* Wait 10 seconds */ -int count =10 *100 ; +int count =10 *HZ ;while (khubd_running && --count) { current->state =TASK_INTERRUPTIBLE ;
We applied a slightly different patch which is would not remove the pages out from under the thread, using semaphores instead.
This patch isn't needed anymore. Thanks anyway.
JE
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
Quoted 4 lines · [email protected] said:
We applied a slightly different patch which is would not remove the
pages out from under the thread, using semaphores instead.This patch isn't needed anymore. Thanks anyway.
Actually, the best fix is probably to get rid of the thread entirely and use schedule_task to run usb_hub_events() instead.
--
dwmw2
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
Quoted 9 lines · David Woodhouse
Quoted 4 lines · [email protected] said:
We applied a slightly different patch which is would not remove the
pages out from under the thread, using semaphores instead.This patch isn't needed anymore. Thanks anyway.
Actually, the best fix is probably to get rid of the thread entirely and use
schedule_taskto runusb_hub_events()instead.
That that possible? usb_hub_events can block for a long time. That is why the kernel thread was needed. I'm not familiar with schedule_task enough to know if it can be used.
JE
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
Quoted 3 lines · Johannes Erdfelt
That that possible?
usb_hub_eventscan block for a long time. That is why the kernel thread was needed. I'm not familiar withschedule_taskenough to know if it can be used.
Ah. How long? At first glance, it didn't look to me as if it would sleep for long at all.
Signature · 7 lines
dwmw2 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] Please read the FAQ at http://www.tux.org/lkml/
Quoted 8 lines · David Woodhouse
Quoted 3 lines · Johannes Erdfelt
That that possible?
usb_hub_eventscan block for a long time. That is why the kernel thread was needed. I'm not familiar withschedule_taskenough to know if it can be used.Ah. How long? At first glance, it didn't look to me as if it would sleep for long at all.
Multiple seconds in the worst case.
JE
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
Quoted 1 line · [email protected] said:
Multiple seconds in the worst case.
Well, I think the PCMCIA socket drivers would be happy with that. Depends what akpm also added to the list of tasks, and whether Linus actually puts that patch into test12.
Probably best to leave it for now and think about it in 2.5.
--
dwmw2
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
Quoted 6 lines · David Woodhouse
Quoted 1 line · [email protected] said:
Multiple seconds in the worst case.
Well, I think the PCMCIA socket drivers would be happy with that. Depends what akpm also added to the list of tasks,
Nothing which sleeps for very long - mainly serial drivers which queue a call to tty_hangup(), which immediately queues _another_ tq_scheduler call to do_tty_hangup (Why? Heaven knows).
Quoted 2 lines
and whether Linus actually puts
that patch into test12.
tq_scheduler is a bug. You can't sleep, you can't call copy_*_user, you can't call kmalloc non-atomically. But we do do these things. Really the only reason for using tq_scheduler is so you can acquire the tasklist_lock or the files_lock. schedule_task() is fine for that.
tq_scheduler must die.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
Quoted 3 lines · [email protected] said:
Nothing which sleeps for very long - mainly serial drivers which
queue a call totty_hangup(), which immediately queues _another_tq_schedulercall todo_tty_hangup(Why? Heaven knows).
Not so much worried about that. More about how sensitive they would be to something _else_ causing the eventd thread to sleep for 'multiple seconds' before getting round to doing what they asked.
I really don't want to have to start using multiple eventd threads before 2.5, if at all. So I don't want to add the USB hub stuff unless the other queued tasks will be happy with that.
--
dwmw2
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
Quoted 9 lines · David Woodhouse
Quoted 3 lines · [email protected] said:
Nothing which sleeps for very long - mainly serial drivers which
queue a call totty_hangup(), which immediately queues _another_tq_schedulercall todo_tty_hangup(Why? Heaven knows).Not so much worried about that. More about how sensitive they would be to something _else_ causing the eventd thread to sleep for 'multiple seconds' before getting round to doing what they asked.
Ah. No, I don't think it would be polite to cause TTY hangup processing to be deferred for this long. I'd suggest that the policy be "scheduled tasks can't sleep". I guess kmalloc(GFP_KERNEL) is acceptable because the system is already running like a dog if this sleeps.
Quoted 3 lines
I really don't want to have to start using multiple eventd threads before 2.5, if at all. So I don't want to add the USB hub stuff unless the other queued tasks will be happy with that.
Some of these requirements could also be satisfied with a combination of a timer_list and a tq_struct. When the timer fires, feed the tq_struct into schedule_task.
Easy, except for the problem of killing the damn things off reliably. That would require a bit of infrastructure. But it's the right thing for netdriver media polling functions, for example.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
Quoted 5 lines · [email protected] said:
Ah. No, I don't think it would be polite to cause TTY hangup
processing to be deferred for this long. I'd suggest that the policy be "scheduled tasks can't sleep". I guesskmalloc(GFP_KERNEL)is acceptable because the system is already running like a dog if this sleeps.
Oi! I specifically added that so I could queue tasks which can sleep.
Put a time limit on it if you must, but not one which prohibits the existing usage by the PCMCIA socket drivers.
I'm beginning to think that I should have argued with Linus¹ when he told me to make it a generic thing rather than calling it pcmcia_queue_task()
¹ "Blasphemy! He said it again!"
--
dwmw2
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/