| Message ID | 1290642894-4577-1-git-send-email-jslaby@suse.cz |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
Sorry I haven't used this driver so can't fully test it. Looks straightforward and compile tested ok. On Thu, Nov 25, 2010 at 10:54 AM, Jiri Slaby <jslaby@suse.cz> wrote: > We register lapb when tty is created, but unregister it only when the > device is UP. So move the lapb_unregister to x25_asy_close_tty after > the device is down. > > Signed-off-by: Jiri Slaby <jslaby@suse.cz> > Reported-by: Sergey Lapin <slapin@ossfans.org> > Cc: Andrew Hendry <andrew.hendry@gmail.com> > --- > drivers/net/wan/x25_asy.c | 11 ++++++----- > 1 files changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/wan/x25_asy.c b/drivers/net/wan/x25_asy.c > index 66cda25..24297b2 100644 > --- a/drivers/net/wan/x25_asy.c > +++ b/drivers/net/wan/x25_asy.c > @@ -498,7 +498,6 @@ norbuff: > static int x25_asy_close(struct net_device *dev) > { > struct x25_asy *sl = netdev_priv(dev); > - int err; > > spin_lock(&sl->lock); > if (sl->tty) > @@ -507,10 +506,6 @@ static int x25_asy_close(struct net_device *dev) > netif_stop_queue(dev); > sl->rcount = 0; > sl->xleft = 0; > - err = lapb_unregister(dev); > - if (err != LAPB_OK) > - printk(KERN_ERR "x25_asy_close: lapb_unregister error -%d\n", > - err); > spin_unlock(&sl->lock); > return 0; > } > @@ -595,6 +590,7 @@ static int x25_asy_open_tty(struct tty_struct *tty) > static void x25_asy_close_tty(struct tty_struct *tty) > { > struct x25_asy *sl = tty->disc_data; > + int err; > > /* First make sure we're connected. */ > if (!sl || sl->magic != X25_ASY_MAGIC) > @@ -605,6 +601,11 @@ static void x25_asy_close_tty(struct tty_struct *tty) > dev_close(sl->dev); > rtnl_unlock(); > > + err = lapb_unregister(sl->dev); > + if (err != LAPB_OK) > + printk(KERN_ERR "x25_asy_close: lapb_unregister error -%d\n", > + err); > + > tty->disc_data = NULL; > sl->tty = NULL; > x25_asy_free(sl); > -- > 1.7.3.1 > > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/25/2010 12:37 PM, Andrew Hendry wrote: > Sorry I haven't used this driver so can't fully test it. Looks > straightforward and compile tested ok. > > On Thu, Nov 25, 2010 at 10:54 AM, Jiri Slaby <jslaby@suse.cz> wrote: >> We register lapb when tty is created, but unregister it only when the >> device is UP. So move the lapb_unregister to x25_asy_close_tty after >> the device is down. I forgot to mention what it causes. The commit message should add: The old behaviour causes ldisc switching to fail each second attempt, because we noted for us that the device is unused, so we use it the second time, but labp layer still have it registered, so it fails obviously. thanks,
On Thu, Nov 25, 2010 at 12:54:54AM +0100, Jiri Slaby wrote: > We register lapb when tty is created, but unregister it only when the > device is UP. So move the lapb_unregister to x25_asy_close_tty after > the device is down. > > Signed-off-by: Jiri Slaby <jslaby@suse.cz> > Reported-by: Sergey Lapin <slapin@ossfans.org> > Cc: Andrew Hendry <andrew.hendry@gmail.com> Tested-by: Sergey Lapin <slapin@ossfans.org> Tested-by: Mikhail Ulyanov <ulyanov.mikhail@gmail.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Looks like some more X.25 users. Not sure on your usage, but I should point out: https://patchwork.kernel.org/patch/332981/ X.25 protocol needs to have the big kernel lock removed so it can stay around in the future. I have been working through them, but there are still some of the more complex ones remaining. The test setups I am using are: Sockets<->X25<->x25loop<->X25<->Sockets Sockets<->X25<->xotd<->network<->xotd<->X25<->Sockets and I know some users have: Sockets<->X25<->xotd<->network<->CiscoXOT<->devices. Where x25loop is a userspace tun device which shuffles the LCIs, does some basic call handling and loops the calls back into kernel X25. xotd is a basic implementation of the X25 over TCP RFC. Are you using something like this? Sockets<->X25<->lapb<->x25_async<->..... If you are using X25 I would be interested to see how your setup goes with the X25 BKL cleanup work, and if anyone wanted to help with the remaining BKLs :) On Fri, Nov 26, 2010 at 9:56 AM, Sergey Lapin <slapin@ossfans.org> wrote: > On Thu, Nov 25, 2010 at 12:54:54AM +0100, Jiri Slaby wrote: >> We register lapb when tty is created, but unregister it only when the >> device is UP. So move the lapb_unregister to x25_asy_close_tty after >> the device is down. >> >> Signed-off-by: Jiri Slaby <jslaby@suse.cz> >> Reported-by: Sergey Lapin <slapin@ossfans.org> >> Cc: Andrew Hendry <andrew.hendry@gmail.com> > Tested-by: Sergey Lapin <slapin@ossfans.org> > Tested-by: Mikhail Ulyanov <ulyanov.mikhail@gmail.com> > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Jiri Slaby <jslaby@suse.cz> Date: Thu, 25 Nov 2010 00:54:54 +0100 > We register lapb when tty is created, but unregister it only when the > device is UP. So move the lapb_unregister to x25_asy_close_tty after > the device is down. > > Signed-off-by: Jiri Slaby <jslaby@suse.cz> > Reported-by: Sergey Lapin <slapin@ossfans.org> > Cc: Andrew Hendry <andrew.hendry@gmail.com> Applied with commit message addition you mentioned Jiri. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/wan/x25_asy.c b/drivers/net/wan/x25_asy.c index 66cda25..24297b2 100644 --- a/drivers/net/wan/x25_asy.c +++ b/drivers/net/wan/x25_asy.c @@ -498,7 +498,6 @@ norbuff: static int x25_asy_close(struct net_device *dev) { struct x25_asy *sl = netdev_priv(dev); - int err; spin_lock(&sl->lock); if (sl->tty) @@ -507,10 +506,6 @@ static int x25_asy_close(struct net_device *dev) netif_stop_queue(dev); sl->rcount = 0; sl->xleft = 0; - err = lapb_unregister(dev); - if (err != LAPB_OK) - printk(KERN_ERR "x25_asy_close: lapb_unregister error -%d\n", - err); spin_unlock(&sl->lock); return 0; } @@ -595,6 +590,7 @@ static int x25_asy_open_tty(struct tty_struct *tty) static void x25_asy_close_tty(struct tty_struct *tty) { struct x25_asy *sl = tty->disc_data; + int err; /* First make sure we're connected. */ if (!sl || sl->magic != X25_ASY_MAGIC) @@ -605,6 +601,11 @@ static void x25_asy_close_tty(struct tty_struct *tty) dev_close(sl->dev); rtnl_unlock(); + err = lapb_unregister(sl->dev); + if (err != LAPB_OK) + printk(KERN_ERR "x25_asy_close: lapb_unregister error -%d\n", + err); + tty->disc_data = NULL; sl->tty = NULL; x25_asy_free(sl);
We register lapb when tty is created, but unregister it only when the device is UP. So move the lapb_unregister to x25_asy_close_tty after the device is down. Signed-off-by: Jiri Slaby <jslaby@suse.cz> Reported-by: Sergey Lapin <slapin@ossfans.org> Cc: Andrew Hendry <andrew.hendry@gmail.com> --- drivers/net/wan/x25_asy.c | 11 ++++++----- 1 files changed, 6 insertions(+), 5 deletions(-)