lkml.org 
[lkml]   [2012]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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


    \
     
     \ /
      Last update: 2012-05-17 20:41    [W:0.028 / U:1.056 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site