Re: [PATCH v3 01/23] tty: Add diagnostic for halted line discipline

[Jiri Slaby]

On 02/05/2013 09:20 PM, Peter Hurley wrote:
> --- a/drivers/tty/tty_ldisc.c
> +++ b/drivers/tty/tty_ldisc.c
> @@ -375,6 +375,7 @@ static inline void tty_ldisc_put(struct tty_ldisc *ld)
>  
>  void tty_ldisc_enable(struct tty_struct *tty)
>  {
> +	clear_bit(TTY_LDISC_HALTED, &tty->flags);
>  	set_bit(TTY_LDISC, &tty->flags);
>  	clear_bit(TTY_LDISC_CHANGING, &tty->flags);
>  	wake_up(&tty_ldisc_wait);
> @@ -498,26 +499,6 @@ static void tty_ldisc_restore(struct tty_struct *tty, struct tty_ldisc *old)
>  }
>  
>  /**
> - *	tty_ldisc_halt		-	shut down the line discipline
> - *	@tty: tty device
> - *
> - *	Shut down the line discipline and work queue for this tty device.
> - *	The TTY_LDISC flag being cleared ensures no further references can
> - *	be obtained while the delayed work queue halt ensures that no more
> - *	data is fed to the ldisc.
> - *
> - *	You need to do a 'flush_scheduled_work()' (outside the ldisc_mutex)
> - *	in order to make sure any currently executing ldisc work is also
> - *	flushed.
> - */
> -
> -static int tty_ldisc_halt(struct tty_struct *tty)
> -{
> -	clear_bit(TTY_LDISC, &tty->flags);
> -	return cancel_work_sync(&tty->port->buf.work);
> -}
> -
> -/**
>   *	tty_ldisc_flush_works	-	flush all works of a tty
>   *	@tty: tty device to flush works for
>   *
> @@ -547,6 +528,29 @@ static int tty_ldisc_wait_idle(struct tty_struct *tty, long timeout)
>  }
>  
>  /**
> + *	tty_ldisc_halt		-	shut down the line discipline
> + *	@tty: tty device
> + *
> + *	Shut down the line discipline and work queue for this tty device.
> + *	The TTY_LDISC flag being cleared ensures no further references can
> + *	be obtained while the delayed work queue halt ensures that no more
> + *	data is fed to the ldisc.
> + *
> + *	You need to do a 'flush_scheduled_work()' (outside the ldisc_mutex)
> + *	in order to make sure any currently executing ldisc work is also
> + *	flushed.
> + */
> +
> +static int tty_ldisc_halt(struct tty_struct *tty)
> +{
> +	int scheduled;
> +	clear_bit(TTY_LDISC, &tty->flags);
> +	scheduled = cancel_work_sync(&tty->port->buf.work);
> +	set_bit(TTY_LDISC_HALTED, &tty->flags);
> +	return scheduled;
> +}

Why did you move with that function? It makes the patch unnecessarily
bigger (and harder for a review).

-- 
js
suse labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/