On Mon, 28 May 2007 15:29:50 +0200 (CEST)
Jiri Slaby <[EMAIL PROTECTED]> wrote:
> rtc, use wait_event_interruptible
>
> Signed-off-by: Jiri Slaby <[EMAIL PROTECTED]>
>
> ---
> commit 62bd843054a7b2b2a3e3003a6b57b6359f199837
> tree 7c6912d4ed42581ca46d384ab2467bfb9d9d9dbe
> parent 2b7813d0f34703e0a4b18593a3186bdc7e719c06
> author Jiri Slaby <[EMAIL PROTECTED]> Sat, 26 May 2007 21:58:54 +0200
> committer Jiri Slaby <[EMAIL PROTECTED]> Sat, 26 May 2007 21:58:54 +0200
>
> drivers/char/rtc.c | 40 +++++++++++++++-------------------------
> 1 files changed, 15 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/char/rtc.c b/drivers/char/rtc.c
> index 20380a2..787b520 100644
> --- a/drivers/char/rtc.c
> +++ b/drivers/char/rtc.c
> @@ -353,33 +353,26 @@ static ssize_t rtc_read(struct file *file, char __user
> *buf,
> if (count != sizeof(unsigned int) && count != sizeof(unsigned long))
> return -EINVAL;
>
> - add_wait_queue(&rtc_wait, &wait);
> -
> - do {
> - /* First make it right. Then make it fast. Putting this whole
> - * block within the parentheses of a while would be too
> - * confusing. And no, xchg() is not the answer. */
> -
> - __set_current_state(TASK_INTERRUPTIBLE);
> -
> - spin_lock_irq (&rtc_lock);
> - data = rtc_irq_data;
> - rtc_irq_data = 0;
> - spin_unlock_irq (&rtc_lock);
> -
> - if (data != 0)
> - break;
> + spin_lock_irq (&rtc_lock);
> + data = rtc_irq_data;
> + rtc_irq_data = 0;
> + spin_unlock_irq (&rtc_lock);
>
> + if (data == 0) {
> if (file->f_flags & O_NONBLOCK) {
> retval = -EAGAIN;
> goto out;
> }
> - if (signal_pending(current)) {
> - retval = -ERESTARTSYS;
> + retval = wait_event_interruptible(rtc_wait, ({
> + spin_lock_irq (&rtc_lock);
> + data = rtc_irq_data;
> + rtc_irq_data = 0;
> + spin_unlock_irq (&rtc_lock);
> + data;
> + }));
> + if (retval)
> goto out;
> - }
> - schedule();
> - } while (1);
> + }
>
> if (count == sizeof(unsigned int))
> retval = put_user(data, (unsigned int __user *)buf) ?:
> sizeof(int);
> @@ -387,10 +380,7 @@ static ssize_t rtc_read(struct file *file, char __user
> *buf,
> retval = put_user(data, (unsigned long __user *)buf) ?:
> sizeof(long);
> if (!retval)
> retval = count;
> - out:
> - __set_current_state(TASK_RUNNING);
> - remove_wait_queue(&rtc_wait, &wait);
> -
> +out:
> return retval;
> #endif
erm, I'm not sure that really improved the code.
Note that wait_event_interruptible() evaluates the condition twice, so
that's now three copies we have in there. A little helper function would
make this all much better.
Also, feel free to sneak in a s/spin_lock_irq (/spin_lock_irq(/ cleanup
when making changes like this.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
