On Sat, 31 Aug 2019 at 01:29, Jakub Kicinski <jakub.kicin...@netronome.com> wrote: > > On Fri, 30 Aug 2019 13:11:11 +0300, Vladimir Oltean wrote: > > On Fri, 30 Aug 2019 at 04:21, Jakub Kicinski wrote: > > > On Fri, 30 Aug 2019 03:46:20 +0300, Vladimir Oltean wrote: > > > > - Configuring the switch over SPI cannot apparently be done from this > > > > ndo_setup_tc callback because it runs in atomic context. I also have > > > > some downstream patches to offload tc clsact matchall with mirred > > > > action, but in that case it looks like the atomic context restriction > > > > does not apply. > > > > > > This sounds really surprising ndo_setup_tc should always be allowed to > > > sleep. Can the taprio limitation be lifted somehow? > > > > I need to get more familiar with the taprio internal data structures. > > I think you're suggesting to get those updated to a consistent state > > while under spin_lock_bh(qdisc_lock(sch)), then call ndo_setup_tc from > > outside that critical section? > > I'm not 100% sure how taprio handles locking TBH, it just seems naive > that HW callback will not need to sleep, so the kernel should make sure > that callback can sleep. Otherwise we'll end up with 3/4 of drivers > implementing some async work routine... > > Sorry, I know that's quite general and not that helpful. > > > Also, I just noticed that I introduced a bug in taprio_disable_offload > > with my reference counting addition. The qdisc can't just pass stack > > memory to the driver, now that it's allowing it to keep it. So > > definitely the patch needs more refactoring. > > Ah, a slight deja vu, I think someone else has done it in the past :)
Ok, I think I managed to move the ndo_setup_tc callback outside of atomic context and nothing broke so far... Any other comments or should I go ahead and send a first proper patchset? Thanks, -Vladimir
