* [patch 21/25] hvc_console: replace schedule_timeout() with msleep()
@ 2004-09-01 20:57 janitor
2004-09-01 22:30 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: janitor @ 2004-09-01 20:57 UTC (permalink / raw)
To: linux-kernel; +Cc: akpm, janitor
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.
Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
Signed-off-by: Maximilian Attems <janitor@sternwelten.at>
---
linux-2.6.9-rc1-bk7-max/drivers/char/hvc_console.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff -puN drivers/char/hvc_console.c~msleep-drivers_char_hvc_console drivers/char/hvc_console.c
--- linux-2.6.9-rc1-bk7/drivers/char/hvc_console.c~msleep-drivers_char_hvc_console 2004-09-01 21:01:32.000000000 +0200
+++ linux-2.6.9-rc1-bk7-max/drivers/char/hvc_console.c 2004-09-01 21:02:32.000000000 +0200
@@ -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 10
static 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);
}
}
_
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch 21/25] hvc_console: replace schedule_timeout() with msleep()
2004-09-01 20:57 [patch 21/25] hvc_console: replace schedule_timeout() with msleep() janitor
@ 2004-09-01 22:30 ` Andrew Morton
2004-09-01 23:08 ` [patch 21/25] hvc_console: replace schedule_timeout() with msleep() maximilian attems
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2004-09-01 22:30 UTC (permalink / raw)
To: janitor; +Cc: linux-kernel
janitor@sternwelten.at wrote:
>
> -#define TIMEOUT ((HZ + 99) / 100)
> +#define TIMEOUT 10
>
> static 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Kernel-janitors] Re: [patch 21/25] hvc_console: replace
2004-09-01 22:30 ` Andrew Morton
@ 2004-09-01 23:08 ` maximilian attems
0 siblings, 0 replies; 6+ messages in thread
From: maximilian attems @ 2004-09-01 23:08 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, kj
[-- Attachment #1: Type: text/plain, Size: 1256 bytes --]
On Wed, 01 Sep 2004, Andrew Morton wrote:
> janitor@sternwelten.at wrote:
> >
> > -#define TIMEOUT ((HZ + 99) / 100)
> > +#define TIMEOUT 10
> >
> > static 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.
could we add for such cases msleep_interruptible()?
patch for that function was sent separately.
> 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.
> 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/
[-- Attachment #2: Type: text/plain, Size: 167 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch 21/25] hvc_console: replace schedule_timeout() with msleep()
@ 2004-09-01 23:08 ` maximilian attems
0 siblings, 0 replies; 6+ messages in thread
From: maximilian attems @ 2004-09-01 23:08 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, kj
On Wed, 01 Sep 2004, Andrew Morton wrote:
> janitor@sternwelten.at wrote:
> >
> > -#define TIMEOUT ((HZ + 99) / 100)
> > +#define TIMEOUT 10
> >
> > static 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.
could we add for such cases msleep_interruptible()?
patch for that function was sent separately.
> 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.
> 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/
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Kernel-janitors] Re: [patch 21/25] hvc_console: replace
2004-09-01 23:08 ` [patch 21/25] hvc_console: replace schedule_timeout() with msleep() maximilian attems
@ 2004-09-01 23:36 ` Andrew Morton
-1 siblings, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2004-09-01 23:36 UTC (permalink / raw)
To: maximilian attems; +Cc: linux-kernel, kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 815 bytes --]
maximilian attems <janitor@sternwelten.at> wrote:
>
> > janitor@sternwelten.at wrote:
> > >
> > > -#define TIMEOUT ((HZ + 99) / 100)
> > > +#define TIMEOUT 10
> > >
> > > static 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.
>
> 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.
[-- Attachment #2: Type: text/plain, Size: 167 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch 21/25] hvc_console: replace schedule_timeout() with msleep()
@ 2004-09-01 23:36 ` Andrew Morton
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2004-09-01 23:36 UTC (permalink / raw)
To: maximilian attems; +Cc: linux-kernel, kernel-janitors
maximilian attems <janitor@sternwelten.at> wrote:
>
> > janitor@sternwelten.at wrote:
> > >
> > > -#define TIMEOUT ((HZ + 99) / 100)
> > > +#define TIMEOUT 10
> > >
> > > static 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.
>
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2004-09-01 23:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-09-01 20:57 [patch 21/25] hvc_console: replace schedule_timeout() with msleep() janitor
2004-09-01 22:30 ` Andrew Morton
2004-09-01 23:08 ` [Kernel-janitors] Re: [patch 21/25] hvc_console: replace maximilian attems
2004-09-01 23:08 ` [patch 21/25] hvc_console: replace schedule_timeout() with msleep() maximilian attems
2004-09-01 23:36 ` [Kernel-janitors] Re: [patch 21/25] hvc_console: replace Andrew Morton
2004-09-01 23:36 ` [patch 21/25] hvc_console: replace schedule_timeout() with msleep() Andrew Morton
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.