Please consider applying. This is my first wait-queue related patch, so comments
are very welcome.
Use wait_event_timeout() in place of custom wait-queue code. The
code is not changed in any way (I don't think), but is cleaned up quite a bit
(will get expanded to almost identical code).
Signed-off-by: Nishanth Aravamudan <[email protected]>
Signed-off-by: Domen Puncer <[email protected]>
---
kj-domen/drivers/char/hvsi.c | 41 ++++-------------------------------------
1 files changed, 4 insertions(+), 37 deletions(-)
diff -puN drivers/char/hvsi.c~wait_event_timeout-drivers_char_hvsi drivers/char/hvsi.c
--- kj/drivers/char/hvsi.c~wait_event_timeout-drivers_char_hvsi 2005-03-05 16:11:27.000000000 +0100
+++ kj-domen/drivers/char/hvsi.c 2005-03-05 16:11:27.000000000 +0100
@@ -44,6 +44,7 @@
#include <linux/sysrq.h>
#include <linux/tty.h>
#include <linux/tty_flip.h>
+#include <linux/wait.h>
#include <asm/hvcall.h>
#include <asm/hvconsole.h>
#include <asm/prom.h>
@@ -631,27 +632,10 @@ static int __init poll_for_state(struct
/* wait for irq handler to change our state */
static int wait_for_state(struct hvsi_struct *hp, int state)
{
- unsigned long end_jiffies = jiffies + HVSI_TIMEOUT;
- unsigned long timeout;
int ret = 0;
- DECLARE_WAITQUEUE(myself, current);
- set_current_state(TASK_INTERRUPTIBLE);
- add_wait_queue(&hp->stateq, &myself);
-
- for (;;) {
- set_current_state(TASK_INTERRUPTIBLE);
- if (hp->state == state)
- break;
- timeout = end_jiffies - jiffies;
- if (time_after(jiffies, end_jiffies)) {
- ret = -EIO;
- break;
- }
- schedule_timeout(timeout);
- }
- remove_wait_queue(&hp->stateq, &myself);
- set_current_state(TASK_RUNNING);
+ if(!wait_event_timeout(hp->stateq, (hp->state == state), jiffies + HVSI_TIMEOUT))
+ ret = -EIO;
return ret;
}
@@ -868,24 +852,7 @@ static int hvsi_open(struct tty_struct *
/* wait for hvsi_write_worker to empty hp->outbuf */
static void hvsi_flush_output(struct hvsi_struct *hp)
{
- unsigned long end_jiffies = jiffies + HVSI_TIMEOUT;
- unsigned long timeout;
-
- DECLARE_WAITQUEUE(myself, current);
- set_current_state(TASK_UNINTERRUPTIBLE);
- add_wait_queue(&hp->emptyq, &myself);
-
- for (;;) {
- set_current_state(TASK_UNINTERRUPTIBLE);
- if (hp->n_outbuf <= 0)
- break;
- timeout = end_jiffies - jiffies;
- if (time_after(jiffies, end_jiffies))
- break;
- schedule_timeout(timeout);
- }
- remove_wait_queue(&hp->emptyq, &myself);
- set_current_state(TASK_RUNNING);
+ wait_event_timeout(hp->emptyq, (hp->n_outbuf <= 0), jiffies + HVSI_TIMEOUT);
/* 'writer' could still be pending if it didn't see n_outbuf = 0 yet */
cancel_delayed_work(&hp->writer);
_
[email protected] wrote:
>
> Please consider applying. This is my first wait-queue related patch, so comments
> are very welcome.
>
> Use wait_event_timeout() in place of custom wait-queue code. The
> code is not changed in any way (I don't think), but is cleaned up quite a bit
> (will get expanded to almost identical code).
>
> ...
> diff -puN drivers/char/hvsi.c~wait_event_timeout-drivers_char_hvsi drivers/char/hvsi.c
> --- kj/drivers/char/hvsi.c~wait_event_timeout-drivers_char_hvsi 2005-03-05 16:11:27.000000000 +0100
> +++ kj-domen/drivers/char/hvsi.c 2005-03-05 16:11:27.000000000 +0100
> @@ -44,6 +44,7 @@
> #include <linux/sysrq.h>
> #include <linux/tty.h>
> #include <linux/tty_flip.h>
> +#include <linux/wait.h>
> #include <asm/hvcall.h>
> #include <asm/hvconsole.h>
> #include <asm/prom.h>
> @@ -631,27 +632,10 @@ static int __init poll_for_state(struct
> /* wait for irq handler to change our state */
> static int wait_for_state(struct hvsi_struct *hp, int state)
> {
> - unsigned long end_jiffies = jiffies + HVSI_TIMEOUT;
> - unsigned long timeout;
> int ret = 0;
>
> - DECLARE_WAITQUEUE(myself, current);
> - set_current_state(TASK_INTERRUPTIBLE);
> - add_wait_queue(&hp->stateq, &myself);
> -
> - for (;;) {
> - set_current_state(TASK_INTERRUPTIBLE);
> - if (hp->state == state)
> - break;
> - timeout = end_jiffies - jiffies;
> - if (time_after(jiffies, end_jiffies)) {
> - ret = -EIO;
> - break;
> - }
> - schedule_timeout(timeout);
> - }
> - remove_wait_queue(&hp->stateq, &myself);
> - set_current_state(TASK_RUNNING);
> + if(!wait_event_timeout(hp->stateq, (hp->state == state), jiffies + HVSI_TIMEOUT))
> + ret = -EIO;
wait_event_timeout()'s `timeout' arg is number-of-milliseconds-to-wait,
not an absolute time, yes?
Also, it's conventional to put a space between the `if' and the `('.
It's nice to squeeze the code into an 80-column xterm, too.
On Sun, 6 Mar 2005 19:32:36 -0800, Andrew Morton <[email protected]> wrote:
> [email protected] wrote:
> >
> > Please consider applying. This is my first wait-queue related patch, so comments
> > are very welcome.
> >
> > Use wait_event_timeout() in place of custom wait-queue code. The
> > code is not changed in any way (I don't think), but is cleaned up quite a bit
> > (will get expanded to almost identical code).
> >
> > ...
<snip>
> > + if(!wait_event_timeout(hp->stateq, (hp->state == state), jiffies + HVSI_TIMEOUT))
> > + ret = -EIO;
>
> wait_event_timeout()'s `timeout' arg is number-of-milliseconds-to-wait,
> not an absolute time, yes?
D'oh. I will fix this and any other timeout version that are being
pushed to you tomorrow. Sadly, wait_event*() do not take milliseconds,
but jiffies. I am hoping to push some patches to change that, but it
may have to wait a while.
> Also, it's conventional to put a space between the `if' and the `('.
>
> It's nice to squeeze the code into an 80-column xterm, too.
Will fix these both as well.
Thanks,
Nish
On Sun, Mar 06, 2005 at 07:32:36PM -0800, Andrew Morton wrote:
> [email protected] wrote:
> >
> > Please consider applying. This is my first wait-queue related patch, so comments
> > are very welcome.
> >
> > Use wait_event_timeout() in place of custom wait-queue code. The
> > code is not changed in any way (I don't think), but is cleaned up quite a bit
> > (will get expanded to almost identical code).
> >
> > ...
> > diff -puN drivers/char/hvsi.c~wait_event_timeout-drivers_char_hvsi drivers/char/hvsi.c
> > --- kj/drivers/char/hvsi.c~wait_event_timeout-drivers_char_hvsi 2005-03-05 16:11:27.000000000 +0100
> > +++ kj-domen/drivers/char/hvsi.c 2005-03-05 16:11:27.000000000 +0100
<snip>
> > + if(!wait_event_timeout(hp->stateq, (hp->state == state), jiffies + HVSI_TIMEOUT))
> > + ret = -EIO;
>
> wait_event_timeout()'s `timeout' arg is number-of-milliseconds-to-wait,
> not an absolute time, yes?
>
> Also, it's conventional to put a space between the `if' and the `('.
>
> It's nice to squeeze the code into an 80-column xterm, too.
All fixed in the below patch. Would you prefer an incremental version?
Thanks,
Nish
Description: Use wait_event_timeout() in place of custom wait-queue
code. The code is not changed in any way (I don't think), but is cleaned
up quite a bit (will get expanded to almost identical code).
Signed-off-by: Nishanth Aravamudan <[email protected]>
--- 2.6.11-v/drivers/char/hvsi.c 2005-03-01 23:38:13.000000000 -0800
+++ 2.6.11/drivers/char/hvsi.c 2005-03-07 16:14:51.000000000 -0800
@@ -631,27 +631,10 @@ static int __init poll_for_state(struct
/* wait for irq handler to change our state */
static int wait_for_state(struct hvsi_struct *hp, int state)
{
- unsigned long end_jiffies = jiffies + HVSI_TIMEOUT;
- unsigned long timeout;
int ret = 0;
- DECLARE_WAITQUEUE(myself, current);
- set_current_state(TASK_INTERRUPTIBLE);
- add_wait_queue(&hp->stateq, &myself);
-
- for (;;) {
- set_current_state(TASK_INTERRUPTIBLE);
- if (hp->state == state)
- break;
- timeout = end_jiffies - jiffies;
- if (time_after(jiffies, end_jiffies)) {
- ret = -EIO;
- break;
- }
- schedule_timeout(timeout);
- }
- remove_wait_queue(&hp->stateq, &myself);
- set_current_state(TASK_RUNNING);
+ if (!wait_event_timeout(hp->stateq, (hp->state == state), HVSI_TIMEOUT))
+ ret = -EIO;
return ret;
}
@@ -868,24 +851,7 @@ static int hvsi_open(struct tty_struct *
/* wait for hvsi_write_worker to empty hp->outbuf */
static void hvsi_flush_output(struct hvsi_struct *hp)
{
- unsigned long end_jiffies = jiffies + HVSI_TIMEOUT;
- unsigned long timeout;
-
- DECLARE_WAITQUEUE(myself, current);
- set_current_state(TASK_UNINTERRUPTIBLE);
- add_wait_queue(&hp->emptyq, &myself);
-
- for (;;) {
- set_current_state(TASK_UNINTERRUPTIBLE);
- if (hp->n_outbuf <= 0)
- break;
- timeout = end_jiffies - jiffies;
- if (time_after(jiffies, end_jiffies))
- break;
- schedule_timeout(timeout);
- }
- remove_wait_queue(&hp->emptyq, &myself);
- set_current_state(TASK_RUNNING);
+ wait_event_timeout(hp->emptyq, (hp->n_outbuf <= 0), HVSI_TIMEOUT);
/* 'writer' could still be pending if it didn't see n_outbuf = 0 yet */
cancel_delayed_work(&hp->writer);