On Sat, 2019-10-05 at 18:13 +0900, Taehee Yoo wrote: > > If we place lockdep keys into "struct net_device", this macro would be a > little bit modified and reused. And driver code shape will not be huge > changed. I think this way is better than this v4 way. > So I will try it. What I was thinking was that if we can do this for every VLAN netdev, why shouldn't we do it for *every* netdev unconditionally? Some code could perhaps even be simplified if this was just a general part of netdev allocation. > > But it seems to me the whole nesting also has to be applied here? > > > > __dev_xmit_skb: > > * qdisc_run_begin() > > * sch_direct_xmit() > > * HARD_TX_LOCK(dev, txq, smp_processor_id()); > > * dev_hard_start_xmit() // say this is VLAN > > * dev_queue_xmit() // on real_dev > > * __dev_xmit_skb // recursion on another netdev > > > > Now if you have VLAN-in-VLAN the whole thing will recurse right? > > > > I have checked on this routine. > Only xmit_lock(HARD_TX_LOCK) could be nested. other > qdisc locks(runinng, busylock) will not be nested. OK, I still didn't check it too closely I guess, or got confused which lock I should look at. > This patch already > handles the _xmit_lock key. so I think there is no problem. Right > But I would like to place four lockdep keys(busylock, address, > running, _xmit_lock) into "struct net_device" because of code complexity. > > Let me know if I misunderstood anything. Nothing to misunderstand - I was just asking/wondering why the qdisc locks were not treated the same way. johannes
