lkml.org 
[lkml]   [2013]   [Feb]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 04/23] tty: Refactor wait for ldisc refs out of tty_ldisc_hangup()
On 02/05/2013 09:20 PM, Peter Hurley wrote:
> Refactor tty_ldisc_hangup() to extract standalone function,
> tty_ldisc_hangup_wait_idle(), to wait for ldisc references
> to be released.
>
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> ---
> drivers/tty/tty_ldisc.c | 54 ++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 36 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
> index 904bf75..70d2b86 100644
> --- a/drivers/tty/tty_ldisc.c
> +++ b/drivers/tty/tty_ldisc.c
> @@ -551,6 +551,41 @@ static int tty_ldisc_halt(struct tty_struct *tty)
> }
>
> /**
> + * tty_ldisc_hangup_wait_idle - wait for the ldisc to become idle
> + * @tty: tty to wait for
> + *
> + * Wait for the line discipline to become idle. The discipline must
> + * have been halted for this to guarantee it remains idle.
> + *
> + * Caller must hold legacy and ->ldisc_mutex.
> + */
> +static bool tty_ldisc_hangup_wait_idle(struct tty_struct *tty)
> +{
> + while (tty->ldisc) { /* Not yet closed */
> + if (atomic_read(&tty->ldisc->users) != 1) {
> + char cur_n[TASK_COMM_LEN], tty_n[64];
> + long timeout = 3 * HZ;
> + tty_unlock(tty);
> +
> + while (tty_ldisc_wait_idle(tty, timeout) == -EBUSY) {
> + timeout = MAX_SCHEDULE_TIMEOUT;
> + printk_ratelimited(KERN_WARNING
> + "%s: waiting (%s) for %s took too long, but we keep waiting...\n",
> + __func__, get_task_comm(cur_n, current),
> + tty_name(tty, tty_n));
> + }
> + /* must reacquire both locks and preserve lock order */
> + mutex_unlock(&tty->ldisc_mutex);
> + tty_lock(tty);
> + mutex_lock(&tty->ldisc_mutex);
> + continue;
> + }
> + break;

That continue and break at the end of the loop look weird.

What about:
while (tty->ldisc) {
if (atomic_read(&tty->ldisc->users) == 1)
break;
THE_REST_OF_THE_BODY_WITHOUT_CONTINUE
}

Or even:
while (tty->ldisc && atomic_read(&tty->ldisc->users) != 1) {
THE_REST_OF_THE_BODY_WITHOUT_CONTINUE
}

For me, this does not look so hard to understand as it is w/ cont+brk now.

And now, when we have a separate function, the declarations would look
nicer to be at the beginning of the function.

> + }
> + return !!(tty->ldisc);

Just a nit, the parenthesis are not needed.

--
js
suse labs


\
 
 \ /
  Last update: 2013-02-07 17:01    [W:0.656 / U:0.800 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site