hvc_console: replace schedule_timeout() with msleep()
Subsystems (2)
PATCH 21/25 hvc_console: replace schedule_timeout() with msleep()
I would appreciate any comments from the janitor@sternweltens list. This is one (of many) cases where I made a decision about replacing
set_current_state(TASK_INTERRUPTIBLE);schedule_timeout(some_time);
with
msleep(jiffies_to_msecs(some_time));
msleep() is not exactly the same as the previous code, but I only did this replacement where I thought long delays were *desired*. If this is not the case here, then just disregard this patch.
Thanks,
Nish
Description: Uses msleep() instead of schedule_timeout() to guarantee the task delays at least the desired time amount.
---
linux-2.6.9-rc1-bk7-max/drivers/char/hvc_console.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
drivers/char/hvc_console.c renamed from drivers/char/hvc_console.c~msleep-drivers_char_hvc_console +3 -3
@@ -26,6 +26,7 @@#include <linux/tty.h> #include <linux/tty_flip.h> #include <linux/sched.h> +#include <linux/delay.h> #include <linux/kbd_kern.h> #include <asm/uaccess.h> #include <linux/spinlock.h>
@@ -40,7 +41,7 @@ extern int hvc_put_chars(int index, cons#define MAX_NR_HVC_CONSOLES 4 -#define TIMEOUT ((HZ + 99) / 100) +#define TIMEOUT 10static struct tty_driver *hvc_driver;static int hvc_offset;
@@ -276,8 +277,7 @@ int khvcd(void *unused)for (i =0 ; i <MAX_NR_HVC_CONSOLES ; ++i)hvc_poll (i); } -set_current_state (TASK_INTERRUPTIBLE ); -schedule_timeout (TIMEOUT ); +msleep (TIMEOUT ); } }
_
drivers/char/hvc_console.c int khvcd(void *unused) · line 280
Quoted 13 lines · [email protected] wrote:
drivers/char/hvc_console.c renamed from drivers/char/hvc_console.c~msleep-drivers_char_hvc_console +2
-3-#define TIMEOUT ((HZ + 99) / 100) +#define TIMEOUT 10static struct tty_driver *hvc_driver;static int hvc_offset;@@ -276,8 +277,7 @@ int khvcd(void *unused)for (i =0 ; i <MAX_NR_HVC_CONSOLES ; ++i)hvc_poll (i); } -set_current_state (TASK_INTERRUPTIBLE ); -schedule_timeout (TIMEOUT ); +msleep (TIMEOUT );
This one is wrong: we need to sleep in interruptible state here, otherwise this kernel thread will contribute to the system load average.
Several other of your msleep conversion patches actually fix bugs. You've found drivers which want to sleep for a fixed period, but they do that with TASK_INTERRUPTIBLE. If someone sends the calling process a signal, these drivers will end up not sleeping at all and may fail.
I'll going through these patches and shall apply the ones which look right. Please consider them all to have been handled, thanks.
Quoted 5 lines · Andrew Morton
Quoted 4 lines · [email protected] wrote:
drivers/char/hvc_console.c renamed from drivers/char/hvc_console.c~msleep-drivers_char_hvc_console +1
-1-#define TIMEOUT ((HZ + 99) / 100) +#define TIMEOUT 10
Quoted 12 lines
drivers/char/hvc_console.c int khvcd(void *unused) · line 280
Quoted 9 lines
static struct
tty_driver*hvc_driver;
static inthvc_offset;drivers/char/hvc_console.c renamed from drivers/char/hvc_console.c~msleep-drivers_char_hvc_console +1
-2@@ -276,8 +277,7 @@ int khvcd(void *unused)for (i =0 ; i <MAX_NR_HVC_CONSOLES ; ++i)hvc_poll (i); } -set_current_state (TASK_INTERRUPTIBLE ); -schedule_timeout (TIMEOUT ); +msleep (TIMEOUT );This one is wrong: we need to sleep in interruptible state here, otherwise this kernel thread will contribute to the system load average.
could we add for such cases msleep_interruptible()? patch for that function was sent separately.
Quoted 4 lines
Several other of your msleep conversion patches actually fix bugs. You've found drivers which want to sleep for a fixed period, but they do that with
TASK_INTERRUPTIBLE. If someone sends the calling process a signal, these drivers will end up not sleeping at all and may fail.
i'll instantly queue some more msleep conversions up.
Quoted 2 lines
I'll going through these patches and shall apply the ones which look right. Please consider them all to have been handled, thanks.
thanks cool :)
--
maks
kernel janitor http://janitor.kernelnewbies.org/
_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 19 lines · maximilian attems
Quoted 6 lines
Quoted 4 lines · [email protected] wrote:
drivers/char/hvc_console.c renamed from drivers/char/hvc_console.c~msleep-drivers_char_hvc_console +1
-1-#define TIMEOUT ((HZ + 99) / 100) +#define TIMEOUT 10Quoted 12 lines
drivers/char/hvc_console.c int khvcd(void *unused) · line 280
Quoted 9 lines
static struct
tty_driver*hvc_driver;
static inthvc_offset;drivers/char/hvc_console.c renamed from drivers/char/hvc_console.c~msleep-drivers_char_hvc_console +1
-2@@ -276,8 +277,7 @@ int khvcd(void *unused)for (i =0 ; i <MAX_NR_HVC_CONSOLES ; ++i)hvc_poll (i); } -set_current_state (TASK_INTERRUPTIBLE ); -schedule_timeout (TIMEOUT ); +msleep (TIMEOUT );This one is wrong: we need to sleep in interruptible state here, otherwise this kernel thread will contribute to the system load average.
Quoted 1 line
could we add for such cases
msleep_interruptible()?
Sure.
Note that you're raising patches against some ancient kernel and that this particular function has changed.
_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors