Re: BUG: scheduling while atomic: ifconfig/0x00000002/4170

Johannes Berg
2007-09-07T13:26:18Z

[-- Attachment #1: Type: text/plain, Size: 1624 bytes --]

On Thu, 2007-09-06 at 08:46 -0700, Paul E. McKenney wrote:

> Looks good to me from an RCU viewpoint. I cannot claim familiarity with
> this code. I therefore especially like the indications of where RTNL
> is held and not!!!

:)

> Some questions below based on a quick scan. And a global question:
> should the comments about RTNL being held be replaced by ASSERT_RTNL()?

I don't like ASSERT_RTNL() much because it actually tries to lock it.
I'd be much happer if it was WARN_ON(!mutex_locked(&rtnl_mutex)) or
something equivalent.

In any case, I have an updated patch I'll be sending soon, and it
requires a new list walking primitive I'll also send.

> > - write_lock_bh(&local->sub_if_lock);
> > + /* we're under RTNL so all this is fine */
> > if (unlikely(local->reg_state == IEEE80211_DEV_UNREGISTERED)) {
> > - write_unlock_bh(&local->sub_if_lock);
> > __ieee80211_if_del(local, sdata);
> > return -ENODEV;
> > }
> > - list_add(&sdata->list, &local->sub_if_list);
> > + list_add_tail_rcu(&sdata->list, &local->interfaces);
>
> The _rcu is required because this list isn't protected by RTNL?

Yes, not all walkers of the list are protected by the RTNL.

> > @@ -226,22 +225,22 @@ void ieee80211_if_reinit(struct net_devi
> > /* Remove all virtual interfaces that use this BSS
> > * as their sdata->bss */
> > struct ieee80211_sub_if_data *tsdata, *n;
> > - LIST_HEAD(tmp_list);
> >
> > - write_lock_bh(&local->sub_if_lock);
>
> This code is also protected by RTNL?

Yes.

> > ASSERT_RTNL();
>
> I -like- this!!! ;-)

:)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]