Hi Nikolay, On Wed, Feb 10, 2021 at 12:31:43PM +0200, Nikolay Aleksandrov wrote: > Hi Vladimir, > Let's take a step back for a moment and discuss the bridge unlock/lock sequences > that come with this set. I'd really like to avoid those as they're a recipe > for future problems. The only good way to achieve that currently is to keep > the PRE_FLAGS call and do that in unsleepable context but move the FLAGS call > after the flags have been changed (if they have changed obviously). That would > make the code read much easier since we'll have all our lock/unlock sequences > in the same code blocks and won't play games to get sleepable context. > Please let's think and work in that direction, rather than having: > + spin_lock_bh(&p->br->lock); > + if (err) { > + netdev_err(p->dev, "%s\n", extack._msg); > + return err; > } > + > > which immediately looks like a bug even though after some code checking we can > verify it's ok. WDYT? > > I plan to get rid of most of the br->lock since it's been abused for a very long > time because it's essentially STP lock, but people have started using it for other > things and I plan to fix that when I get more time. This won't make the sysfs codepath any nicer, will it?