Re: [PATCH tty-next] n_tty: Fix termios_rwsem lockdep false positive

[Sergey Senozhatsky <sergey.senozhatsky@gmail.com>]

On (08/12/13 08:55), Peter Hurley wrote:
> >>> [..]
> >>>  drivers/tty/n_tty.c | 25 +++++++++++--------------
> >>>  1 file changed, 11 insertions(+), 14 deletions(-)
> >>>
> >
> >I hate to do this, but isn't it actually my patch posted here
> >https://lkml.org/lkml/2013/8/1/510
> >
> >which was tagged as `wrong'?
> 
> Sergey,
> 
> My apologies; I was mistaken regarding this problem being a lockdep
> regression (although it's still a false positive from lockdep). Once
> I had worked around some issues with the nouveau driver, I was able to
> reproduce the lockdep report on 3.10.
>
no problem.

> I included Artem's lockdep report in the changelog because I received
> that first, on 30 July.
>
> My patch below is not the same as your patch of 1 Aug. This patch
> preserves the protected access of termios.c_cc[VMIN] and termios.c_cc[VTIME]
> (via the MIN_CHAR() and TIME_CHAR() macros).

fair enough. v3 was protecting VMIN/VTIME (my bad, I noticed this a bit later),
but I didn't submit it since v2 did not get positive response.

> If you'd prefer, I could add to changelog:
> 
>    Patch based on original posted here https://lkml.org/lkml/2013/8/1/510
>    by Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

if you don't mind, that would be great.

thanks a lot,
	-ss

> Regards,
> Peter Hurley
> 
> 
> >>>diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> >>>index dd8ae0c..c9a9ddd 100644
> >>>--- a/drivers/tty/n_tty.c
> >>>+++ b/drivers/tty/n_tty.c
> >>>@@ -2122,6 +2122,17 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
> >>>  	if (c < 0)
> >>>  		return c;
> >>>
> >>>+	/*
> >>>+	 *	Internal serialization of reads.
> >>>+	 */
> >>>+	if (file->f_flags & O_NONBLOCK) {
> >>>+		if (!mutex_trylock(&ldata->atomic_read_lock))
> >>>+			return -EAGAIN;
> >>>+	} else {
> >>>+		if (mutex_lock_interruptible(&ldata->atomic_read_lock))
> >>>+			return -ERESTARTSYS;
> >>>+	}
> >>>+
> >>>  	down_read(&tty->termios_rwsem);
> >>>
> >>>  	minimum = time = 0;
> >>>@@ -2141,20 +2152,6 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
> >>>  		}
> >>>  	}
> >>>
> >>>-	/*
> >>>-	 *	Internal serialization of reads.
> >>>-	 */
> >>>-	if (file->f_flags & O_NONBLOCK) {
> >>>-		if (!mutex_trylock(&ldata->atomic_read_lock)) {
> >>>-			up_read(&tty->termios_rwsem);
> >>>-			return -EAGAIN;
> >>>-		}
> >>>-	} else {
> >>>-		if (mutex_lock_interruptible(&ldata->atomic_read_lock)) {
> >>>-			up_read(&tty->termios_rwsem);
> >>>-			return -ERESTARTSYS;
> >>>-		}
> >>>-	}
> >>>  	packet = tty->packet;
> >>>
> >>>  	add_wait_queue(&tty->read_wait, &wait);
> >>>--
> >>>1.8.1.2
> >>>
> >>
> >>--
> >>Regards,
> >>     Artem
> >>
>