Messages in this thread | | | Date | Thu, 24 Feb 2011 21:27:28 +0100 (CET) | From | Thomas Gleixner <> | Subject | Re: [PATCH 09/10] n_tracerouter ldisc driver. |
| |
On Thu, 24 Feb 2011, J Freyensee wrote: > > > + > > > + mutex_lock(&routelock); > > > + if (tr_data->opencalled == 0) { > > > + > > > + tr_data->kref_tty = tty_kref_get(tty); > > > + if (tr_data->kref_tty == NULL) > > > + retval = -EFAULT; > > > + else { > > > > Please use braces for the if as well. It's just irritating not to have > > them before the else. > > checkpatch.pl did not care about the lack of braces so that is why this > got missed; I can add it.
It's not a strict requirement, but add it and look at the result. Then judge yourself.
> > +static void n_tracerouter_close(struct tty_struct *tty) > > > +{ > > > + struct tracerouter_data *tptr = tty->disc_data; > > > + > > > + WARN_ON(tptr->kref_tty != tr_data->kref_tty); > > > + tty_driver_flush_buffer(tty); > > > > That code probably never run with lockdep as you would get a potential > > deadlock warning. See n_tracerouter_open(). > > > > Though the deadlock cannot happen as you are protected by > > tr_data->opencalled it does not make it more correct. > > > > What do you suggest to make it correct for upstream?
Keep the lock ordering consistent. Either call that under the lock always or do not.
Thanks,
tglx
| |