Hi Max, On Mon, Jul 25, 2022 at 3:09 PM Max Staudt <max@xxxxxxxxx> wrote: > > On Mon, 25 Jul 2022 08:40:24 +0200 > Dario Binacchi <dario.binacchi@xxxxxxxxxxxxxxxxxxxx> wrote: > > > > > @@ -883,72 +786,50 @@ static int slcan_open(struct tty_struct *tty) > > > > if (!tty->ops->write) > > > > return -EOPNOTSUPP; > > > > > > > > - /* RTnetlink lock is misused here to serialize concurrent > > > > - * opens of slcan channels. There are better ways, but it is > > > > - * the simplest one. > > > > - */ > > > > - rtnl_lock(); > > > > + dev = alloc_candev(sizeof(*sl), 1); > > > > + if (!dev) > > > > + return -ENFILE; > > > > > > > > - /* Collect hanged up channels. */ > > > > - slc_sync(); > > > > + sl = netdev_priv(dev); > > > > > > > > - sl = tty->disc_data; > > > > + /* Configure TTY interface */ > > > > + tty->receive_room = 65536; /* We don't flow control */ > > > > + sl->rcount = 0; > > > > + sl->xleft = 0; > > > > > > I suggest moving the zeroing to slc_open() - i.e. to the netdev open > > > function. As a bonus, you can then remove the same two assignments from > > > slc_close() (see above). They are only used when netif_running(), with > > > appropiate guards already in place as far as I can see. > > > > I think it is better to keep the code as it is, since at the entry of > > the netdev > > open function, netif_running already returns true (it is set to true by the > > calling function) and therefore it would be less safe to reset the > > rcount and xleft > > fields. > > Wow, great catch! > > I wonder why __LINK_STATE_START is set before ->ndo_open() is called...? > > > Since the drivers are similar, I've checked can327. It is unaffected, > because the counters are additionally guarded by a spinlock. Same in > slcan, where netdev_close() takes the spinlock to reset the counters. > > So you *could* move them to netdev_open() *if* they are always guarded > by the slcan lock. > > Or, leave it as it is, as it seems to be correct. Your choice :) If possible I prefer not to use spin_lock. So I prefer to keep the code as is. Thanks and regards, Dario > > > Thank you! > > Max -- Dario Binacchi Embedded Linux Developer dario.binacchi@xxxxxxxxxxxxxxxxxxxx __________________________________ Amarula Solutions SRL Via Le Canevare 30, 31100 Treviso, Veneto, IT T. +39 042 243 5310 info@xxxxxxxxxxxxxxxxxxxx www.amarulasolutions.com
