Re: [RFC PATCH 3/3] net/ncsi: Configure VLAN tag filter

[Samuel Mendoza-Jonas <sam@mendozajonas.com>]

On Thu, 2017-08-10 at 17:55 +0930, Joel Stanley wrote:
> On Wed, Aug 9, 2017 at 6:24 PM, Samuel Mendoza-Jonas
> <sam@mendozajonas.com> wrote:
> > +static int ncsi_kick_channels(struct ncsi_dev_priv *ndp)
> > +{
> > +       struct ncsi_dev *nd = &ndp->ndev;
> > +       struct ncsi_channel *nc;
> > +       struct ncsi_package *np;
> > +       unsigned long flags;
> > +       unsigned int n = 0;
> > +
> > +       NCSI_FOR_EACH_PACKAGE(ndp, np) {
> > +               NCSI_FOR_EACH_CHANNEL(np, nc) {
> > +                       spin_lock_irqsave(&nc->lock, flags);
> > +
> > +                       /*
> > +                        * Channels may be busy - mark dirty instead of kicking if
> > +                        * a) not ACTIVE (configured)
> > +                        * b) in the channel_queue (to be configured)
> > +                        * c) it's ndev is in the config state
> > +                        */
> > +                       if (nc->state != NCSI_CHANNEL_ACTIVE) {
> > +                               if ((ndp->ndev.state & 0xff00) == ncsi_dev_state_config ||
> > +                                               !list_empty(&nc->link)) {
> > +                                       netdev_info(nd->dev, "nc %p marked dirty\n", nc);
> > +                                       nc->reconfigure_needed = true;
> > +                               }
> > +                               spin_unlock_irqrestore(&nc->lock, flags);
> > +                               continue;
> > +                       }
> > +
> > +                       spin_unlock_irqrestore(&nc->lock, flags);
> > +
> > +                       ncsi_stop_channel_monitor(nc);
> > +                       spin_lock_irqsave(&nc->lock, flags);
> > +                       nc->state = NCSI_CHANNEL_INVISIBLE;
> > +                       spin_unlock_irqrestore(&nc->lock, flags);
> > +
> > +                       spin_lock_irqsave(&ndp->lock, flags);
> > +                       nc->state = NCSI_CHANNEL_INACTIVE;
> > +                       list_add_tail_rcu(&nc->link, &ndp->channel_queue);
> > +                       spin_unlock_irqrestore(&ndp->lock, flags);
> > +
> > +                       spin_unlock_irqrestore(&nc->lock, flags);
> 
> Oops, this is unlocking nc->lock a second time.

Oops indeed! Good spot.

> 
> > +
> > +                       netdev_info(nd->dev, "Kicked channel %p\n", nc);
> > +                       n++;
> > +               }
> > +       }