Re: [PATCH] TTY: pty, fix pty counting

[Jiri Slaby <jslaby@suse.cz>]

On 10/23/2011 11:01 PM, Ilya Zykov wrote:
> New version for commit: 24d406a6bf736f7aebdc8fa0f0ec86e0890c6d24

Although it will work, as ptms are not allowed to be reopen, it doesn't
look correct. We should decrement the count in ->remove, because we are
incrementing in install.

Now, when I understand ptm+devpts layer a bit more, instead of the
current hackish approach introduced by 24d406a6b (TTY: pty, fix pty
counting), I think we may introduce a ->remove hook specific to ptms. In
that one we could decrement the count and don't bother with the
pty_count macros anymore. Right?

BTW you cannot remove ->remove hook of pty layer. It would cause an OOPS
because driver->ttys is not allocated for ptys.

> diff -uprN a/drivers/tty/pty.c b/drivers/tty/pty.c
> --- a/drivers/tty/pty.c	2011-05-19 08:06:34.000000000 +0400
> +++ b/drivers/tty/pty.c	2011-10-23 18:01:20.000000000 +0400
> @@ -36,13 +36,15 @@
>  static struct tty_driver *ptm_driver;
>  static struct tty_driver *pts_driver;
>  #endif
> +static int pty_count;
>  
>  static void pty_close(struct tty_struct *tty, struct file *filp)
>  {
>  	BUG_ON(!tty);
> -	if (tty->driver->subtype == PTY_TYPE_MASTER)
> +	if (tty->driver->subtype == PTY_TYPE_MASTER) {
>  		WARN_ON(tty->count > 1);
> -	else {
> +		pty_count--;
> +	} else {
>  		if (tty->count > 2)
>  			return;
>  	}
> @@ -446,7 +448,6 @@ static inline void legacy_pty_init(void)
>  int pty_limit = NR_UNIX98_PTY_DEFAULT;
>  static int pty_limit_min;
>  static int pty_limit_max = NR_UNIX98_PTY_MAX;
> -static int pty_count;
>  
>  static struct cdev ptmx_cdev;
>  
> @@ -599,15 +600,9 @@ free_mem_out:
>  	return -ENOMEM;
>  }
>  
> -static void pty_unix98_remove(struct tty_driver *driver, struct tty_struct *tty)
> -{
> -	pty_count--;
> -}
> -
>  static const struct tty_operations ptm_unix98_ops = {
>  	.lookup = ptm_unix98_lookup,
>  	.install = pty_unix98_install,
> -	.remove = pty_unix98_remove,
>  	.open = pty_open,
>  	.close = pty_close,
>  	.write = pty_write,
> @@ -624,7 +619,6 @@ static const struct tty_operations ptm_u
>  static const struct tty_operations pty_unix98_ops = {
>  	.lookup = pts_unix98_lookup,
>  	.install = pty_unix98_install,
> -	.remove = pty_unix98_remove,
>  	.open = pty_open,
>  	.close = pty_close,
>  	.write = pty_write,

thanks,
-- 
js
suse labs