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@xxxxxxxxxxxxxxxxxx>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
