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


\
 
 \ /
  Last update: 2011-02-24 21:29    [W:0.458 / U:0.152 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site