On Wed, Jun 10, 2020 at 7:48 AM Taehee Yoo <ap420...@gmail.com> wrote: > > On Tue, 9 Jun 2020 at 06:53, Cong Wang <xiyou.wangc...@gmail.com> wrote: > > > > Hi Cong, > Thank you for this work! > > > The dynamic key update for addr_list_lock still causes troubles, > > for example the following race condition still exists: > > > > CPU 0: CPU 1: > > (RCU read lock) (RTNL lock) > > dev_mc_seq_show() netdev_update_lockdep_key() > > -> lockdep_unregister_key() > > -> netif_addr_lock_bh() > > > > because lockdep doesn't provide an API to update it atomically. > > Therefore, we have to move it back to static keys and use subclass > > for nest locking like before. > > > > I'm sorry for the late reply. > I agree that using subclass mechanism to avoid too many lockdep keys.
Avoiding too many lockdep keys is not the real goal of my patch, its main purpose is to fix a race condition shown above. Just FYI. > But the subclass mechanism is also not updated its subclass key > automatically. So, if upper/lower relationship is changed, > interface would have incorrect subclass key. > It eventually results in lockdep warning. So dev->lower_level is not updated accordingly? I just blindly trust dev->lower_level, as you use it in other places too. > And, I think this patch doesn't contain bonding and team module part. > So, an additional patch is needed. Hmm, dev->lower_level is generic, so is addr_list_lock. Again, I just assume you already update dev->lower_level each time the topology changes. I added some printk() to verify it too for my simple bond over bond case. So, I can't immediately see what is wrong with dev->lower_level here. Do you mind to be more specific? Or I misunderstand your point? Thanks!
