Messages in this thread |  | | | Date | Thu, 17 May 2012 11:28:30 -0700 | | From | Greg Kroah-Hartman <> | | Subject | Re: [PATCH] tty: tty_mutex: fix lockdep warning in tty_lock_pair |
| |
On Thu, May 17, 2012 at 01:58:16PM +0800, Ming Lei wrote: > Commit d29f3ef39be4eec0362b985305fc526d9be318cf(tty_lock: > Localise the lock) introduces tty_lock_pair, in which > may cause lockdep warning because two locks with same lock > class are to be acquired one after another. > > This patch uses mutex_lock_nest_lock annotation to avoid > the warning. > > Cc: Alan Cox <alan@linux.intel.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Peter Zijlstra <peterz@infradead.org> > Signed-off-by: Ming Lei <ming.lei@canonical.com> > --- > drivers/tty/tty_mutex.c | 21 ++++++++++++++++++--- > 1 file changed, 18 insertions(+), 3 deletions(-) > > diff --git a/drivers/tty/tty_mutex.c b/drivers/tty/tty_mutex.c > index 69adc80..079f9d7 100644 > --- a/drivers/tty/tty_mutex.c > +++ b/drivers/tty/tty_mutex.c > @@ -10,6 +10,18 @@ > * Getting the big tty mutex. > */ > > +static void __lockfunc tty_lock_nest_lock(struct tty_struct *tty, > + struct tty_struct *tty2)
Duplicating tty_lock() just for this one issue seems wrong and prone to error, don't you think?
> +{ > + if (tty->magic != TTY_MAGIC) { > + printk(KERN_ERR "L Bad %p\n", tty); > + WARN_ON(1); > + return; > + } > + tty_kref_get(tty); > + mutex_lock_nest_lock(&tty->legacy_mutex, &tty2->legacy_mutex); > +} > + > void __lockfunc tty_lock(struct tty_struct *tty) > { > if (tty->magic != TTY_MAGIC) { > @@ -43,11 +55,14 @@ void __lockfunc tty_lock_pair(struct tty_struct *tty, > { > if (tty < tty2) { > tty_lock(tty); > - tty_lock(tty2); > + tty_lock_nest_lock(tty2, tty); > } else { > - if (tty2 && tty2 != tty) > + if (tty2 && tty2 != tty) { > tty_lock(tty2); > - tty_lock(tty); > + tty_lock_nest_lock(tty, tty2);
This is wonky, and confusing, don't you think?
I don't like it, surely there's a better way to solve this?
thanks,
greg k-h
|  |