On Sat, Jun 13, 2020 at 9:03 AM Taehee Yoo <ap420...@gmail.com> wrote:
>
> On Thu, 11 Jun 2020 at 08:21, Cong Wang <xiyou.wangc...@gmail.com> wrote:
> >
>
> Hi Cong :)
>
> > 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:
> > > >
> > > > + lockdep_set_class_and_subclass(&dev->addr_list_lock,
> > > > + &vlan_netdev_addr_lock_key,
> > > > + subclass);
>
> In this patch, lockdep_set_class_and_subclass() is used.
> As far as I know, this function initializes lockdep key and subclass
> value with a given variable.
> A dev->lower_level variable is used as a subclass value in this patch.
> When dev->lower_level value is changed, the subclass value of this
> lockdep key is not changed automatically.
> If this value has to be changed, additional function is needed.
Hmm, but we pass a dynamic subclass to spin_lock_nested().
So I guess I should just remove all the
lockdep_set_class_and_subclass() and leave subclass to 0?
>
> >>> netif_addr_lock_bh(from);
> In this function, internally spin_lock_bh() is used and this function
> might use an 'initialized subclass value' not a current dev->lower_level.
> At this point, I think the lockdep splat might occur.
>
> +static inline void netif_addr_lock_nested(struct net_device *dev)
> +{
> + spin_lock_nested(&dev->addr_list_lock, dev->lower_level);
> +}
> In this patch, you used netif_addr_lock_nested() too.
> These two subclass values could be different.
> But I'm not sure whether using spin_lock_nested with two different
> subclass values are the right way or not.
Yeah, as long as dev->lower_level is different, it should be different
subclass. I assume dev->lower_level is automatically adjusted
whenever the topology changes, like the vlan over bond case above.
Thanks.
