2009-07-24 18:45:32

by Reinette Chatre

[permalink] [raw]
Subject: new lockdep warning in 2.6.31-rc3-wl

I saw this right after association. This is with the latest
wireless-testing.

[ 65.053475] =======================================================
[ 65.053562] [ INFO: possible circular locking dependency detected ]
[ 65.053608] 2.6.31-rc3-wl #13
[ 65.053649] -------------------------------------------------------
[ 65.053694] phy0/3223 is trying to acquire lock:
[ 65.053738] (cfg80211_mutex){+.+.+.}, at: [<ffffffffa01c9a72>] regulatory_hint_11d+0x32/0x430 [cfg80211]
[ 65.053887]
[ 65.053888] but task is already holding lock:
[ 65.053968] (&ifmgd->mtx){+.+.+.}, at: [<ffffffffa0220f78>] ieee80211_sta_work+0x118/0x11c0 [mac80211]
[ 65.054120]
[ 65.054120] which lock already depends on the new lock.
[ 65.054121]
[ 65.054239]
[ 65.054240] the existing dependency chain (in reverse order) is:
[ 65.054322]
[ 65.054323] -> #3 (&ifmgd->mtx){+.+.+.}:
[ 65.054483] [<ffffffff8106f994>] __lock_acquire+0x1164/0x1b50
[ 65.054558] [<ffffffff81070453>] lock_acquire+0xd3/0x100
[ 65.054630] [<ffffffff81323e65>] mutex_lock_nested+0x45/0x320
[ 65.054705] [<ffffffffa021ed35>] ieee80211_mgd_auth+0xf5/0x1f0 [mac80211]
[ 65.054798] [<ffffffffa0225623>] ieee80211_auth+0x13/0x20 [mac80211]
[ 65.054880] [<ffffffffa01d5ac1>] __cfg80211_mlme_auth+0x1b1/0x290 [cfg80211]
[ 65.054971] [<ffffffffa01d7d6b>] cfg80211_conn_do_work+0xdb/0x1b0 [cfg80211]
[ 65.055061] [<ffffffffa01d8217>] __cfg80211_connect+0x3d7/0x4c0 [cfg80211]
[ 65.055150] [<ffffffffa01db9ed>] cfg80211_mgd_wext_connect+0xcd/0x180 [cfg80211]
[ 65.055240] [<ffffffffa01dbc2f>] cfg80211_mgd_wext_siwap+0x18f/0x210 [cfg80211]
[ 65.055329] [<ffffffffa021698c>] ieee80211_ioctl_siwap+0x1c/0x70 [mac80211]
[ 65.055421] [<ffffffff8130ebc3>] ioctl_standard_call+0x63/0x460
[ 65.055495] [<ffffffff8130e7bb>] wext_handle_ioctl+0x16b/0x240
[ 65.055568] [<ffffffff8128cd02>] dev_ioctl+0x3f2/0x5f0
[ 65.055642] [<ffffffff81277059>] sock_ioctl+0x89/0x290
[ 65.055715] [<ffffffff810fe201>] vfs_ioctl+0x31/0xa0
[ 65.055787] [<ffffffff810fe38a>] do_vfs_ioctl+0x8a/0x5c0
[ 65.055859] [<ffffffff810fe959>] sys_ioctl+0x99/0xa0
[ 65.055931] [<ffffffff8100bd6b>] system_call_fastpath+0x16/0x1b
[ 65.056006] [<ffffffffffffffff>] 0xffffffffffffffff
[ 65.056080]
[ 65.056080] -> #2 (&wdev->mtx){+.+.+.}:
[ 65.056240] [<ffffffff8106f994>] __lock_acquire+0x1164/0x1b50
[ 65.056314] [<ffffffff81070453>] lock_acquire+0xd3/0x100
[ 65.056386] [<ffffffff81323e65>] mutex_lock_nested+0x45/0x320
[ 65.056459] [<ffffffffa01c5224>] cfg80211_netdev_notifier_call+0x1a4/0x380 [cfg80211]
[ 65.056550] [<ffffffff81328855>] notifier_call_chain+0x65/0xa0
[ 65.056623] [<ffffffff810605f1>] raw_notifier_call_chain+0x11/0x20
[ 65.056697] [<ffffffff8128bbea>] dev_open+0x10a/0x120
[ 65.056769] [<ffffffff8128af4d>] dev_change_flags+0x9d/0x1e0
[ 65.056842] [<ffffffff81294bb3>] do_setlink+0x2b3/0x450
[ 65.056844] [<ffffffff81294f25>] rtnl_setlink+0x115/0x160
[ 65.056844] [<ffffffff8129605e>] rtnetlink_rcv_msg+0x18e/0x240
[ 65.056844] [<ffffffff812a8379>] netlink_rcv_skb+0x89/0xb0
[ 65.056844] [<ffffffff81295eb9>] rtnetlink_rcv+0x29/0x40
[ 65.056844] [<ffffffff812a7d12>] netlink_unicast+0x2e2/0x2f0
[ 65.056844] [<ffffffff812a7f41>] netlink_sendmsg+0x221/0x330
[ 65.056844] [<ffffffff81277f07>] sock_sendmsg+0x127/0x140
[ 65.056844] [<ffffffff81278065>] sys_sendmsg+0x145/0x280
[ 65.056844] [<ffffffff8100bd6b>] system_call_fastpath+0x16/0x1b
[ 65.056844] [<ffffffffffffffff>] 0xffffffffffffffff
[ 65.056844]
[ 65.056844] -> #1 (&rdev->mtx){+.+.+.}:
[ 65.056844] [<ffffffff8106f994>] __lock_acquire+0x1164/0x1b50
[ 65.056844] [<ffffffff81070453>] lock_acquire+0xd3/0x100
[ 65.056844] [<ffffffff81323e65>] mutex_lock_nested+0x45/0x320
[ 65.056844] [<ffffffffa01c6421>] cfg80211_get_dev_from_ifindex+0x61/0xa0 [cfg80211]
[ 65.056844] [<ffffffffa01ca3c0>] cfg80211_wext_giwscan+0x50/0x1210 [cfg80211]
[ 65.056844] [<ffffffff8130ed54>] ioctl_standard_call+0x1f4/0x460
[ 65.056844] [<ffffffff8130e7bb>] wext_handle_ioctl+0x16b/0x240
[ 65.056844] [<ffffffff8128cd02>] dev_ioctl+0x3f2/0x5f0
[ 65.056844] [<ffffffff81277059>] sock_ioctl+0x89/0x290
[ 65.056844] [<ffffffff810fe201>] vfs_ioctl+0x31/0xa0
[ 65.056844] [<ffffffff810fe38a>] do_vfs_ioctl+0x8a/0x5c0
[ 65.056844] [<ffffffff810fe959>] sys_ioctl+0x99/0xa0
[ 65.056844] [<ffffffff8100bd6b>] system_call_fastpath+0x16/0x1b
[ 65.056844] [<ffffffffffffffff>] 0xffffffffffffffff
[ 65.056844]
[ 65.056844] -> #0 (cfg80211_mutex){+.+.+.}:
[ 65.056844] [<ffffffff8106fae3>] __lock_acquire+0x12b3/0x1b50
[ 65.056844] [<ffffffff81070453>] lock_acquire+0xd3/0x100
[ 65.056844] [<ffffffff81323e65>] mutex_lock_nested+0x45/0x320
[ 65.056844] [<ffffffffa01c9a72>] regulatory_hint_11d+0x32/0x430 [cfg80211]
[ 65.056844] [<ffffffffa0220a8d>] ieee80211_rx_mgmt_beacon+0x2cd/0x4d0 [mac80211]
[ 65.056844] [<ffffffffa0221049>] ieee80211_sta_work+0x1e9/0x11c0 [mac80211]
[ 65.056844] [<ffffffff81055f80>] worker_thread+0x1f0/0x340
[ 65.056844] [<ffffffff8105b28e>] kthread+0x9e/0xb0
[ 65.056844] [<ffffffff8100ce7a>] child_rip+0xa/0x20
[ 65.056844] [<ffffffffffffffff>] 0xffffffffffffffff
[ 65.056844]
[ 65.056844] other info that might help us debug this:
[ 65.056844]
[ 65.056844] 3 locks held by phy0/3223:
[ 65.056844] #0: ((wiphy_name(local->hw.wiphy))){+.+.+.}, at: [<ffffffff81055f2d>] worker_thread+0x19d/0x340
[ 65.056844] #1: (&ifmgd->work){+.+.+.}, at: [<ffffffff81055f2d>] worker_thread+0x19d/0x340
[ 65.056844] #2: (&ifmgd->mtx){+.+.+.}, at: [<ffffffffa0220f78>] ieee80211_sta_work+0x118/0x11c0 [mac80211]
[ 65.056844]
[ 65.056844] stack backtrace:
[ 65.056844] Pid: 3223, comm: phy0 Not tainted 2.6.31-rc3-wl #13
[ 65.056844] Call Trace:
[ 65.056844] [<ffffffff8106e28c>] print_circular_bug_tail+0xdc/0xe0
[ 65.056844] [<ffffffff8106fae3>] __lock_acquire+0x12b3/0x1b50
[ 65.056844] [<ffffffff8106d5ed>] ? mark_held_locks+0x6d/0x90
[ 65.056844] [<ffffffff81070453>] lock_acquire+0xd3/0x100
[ 65.056844] [<ffffffffa01c9a72>] ? regulatory_hint_11d+0x32/0x430 [cfg80211]
[ 65.056844] [<ffffffff81323e65>] mutex_lock_nested+0x45/0x320
[ 65.056844] [<ffffffffa01c9a72>] ? regulatory_hint_11d+0x32/0x430 [cfg80211]
[ 65.056844] [<ffffffffa01cc03c>] ? cfg80211_inform_bss_frame+0x19c/0x1a0 [cfg80211]
[ 65.056844] [<ffffffffa01c9a72>] regulatory_hint_11d+0x32/0x430 [cfg80211]
[ 65.056844] [<ffffffffa021a35c>] ? ieee80211_rx_bss_put+0xc/0x10 [mac80211]
[ 65.056844] [<ffffffffa0220753>] ? ieee80211_rx_bss_info+0xb3/0x120 [mac80211]
[ 65.056844] [<ffffffffa0220a8d>] ieee80211_rx_mgmt_beacon+0x2cd/0x4d0 [mac80211]
[ 65.056844] [<ffffffffa0221049>] ieee80211_sta_work+0x1e9/0x11c0 [mac80211]
[ 65.056844] [<ffffffff810adb60>] ? probe_workqueue_execution+0x40/0xa0
[ 65.056844] [<ffffffffa0220e60>] ? ieee80211_sta_work+0x0/0x11c0 [mac80211]
[ 65.056844] [<ffffffff81055f80>] worker_thread+0x1f0/0x340
[ 65.056844] [<ffffffff81055f2d>] ? worker_thread+0x19d/0x340
[ 65.056844] [<ffffffff8105b680>] ? autoremove_wake_function+0x0/0x40
[ 65.056844] [<ffffffff8106d94d>] ? trace_hardirqs_on+0xd/0x10
[ 65.056844] [<ffffffff81055d90>] ? worker_thread+0x0/0x340
[ 65.056844] [<ffffffff8105b28e>] kthread+0x9e/0xb0
[ 65.056844] [<ffffffff8100ce7a>] child_rip+0xa/0x20
[ 65.056844] [<ffffffff8100c83c>] ? restore_args+0x0/0x30
[ 65.056844] [<ffffffff8105b1f0>] ? kthread+0x0/0xb0
[ 65.056844] [<ffffffff8100ce70>] ? child_rip+0x0/0x20





2009-07-26 06:21:37

by Johannes Berg

[permalink] [raw]
Subject: Re: new lockdep warning in 2.6.31-rc3-wl

On Sat, 2009-07-25 at 12:21 -0700, Luis R. Rodriguez wrote:
> On Sat, Jul 25, 2009 at 2:13 AM, Johannes Berg<[email protected]> wrote:
> > Obviously 3) is my favourite solution, not only because it's the only
> > one that seems feasible at all, but it's somewhat more complex, and I
> > don't really have time to do that today.
>
> Hm yeah this seems very appealing, it should be noted that this is
> called *very* often too right now because we call this for every
> beacon.

Actually, no, because I added filtering in mac80211 to make it behave
similarly with and without hardware beacon filtering.

> If we want to merge beacon hint and 11d stuff with the added
> advantage to make new functionality available to cfg80211 it would
> seem then to make sense to add some sort of beacon parser so we can
> extend it later if needed.

True.

> But can fullmac cards send full beacon stuff somehow to cfg80211? Can
> they send 11d stuff or would they have to try to build one? Or can
> they realistically only send alpha2 stuff?

They are supposed to send us at least the IEs from the beacon frame,
which we can process similarly. Otherwise, they cannot even support WPA.

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-07-25 19:21:46

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: new lockdep warning in 2.6.31-rc3-wl

On Sat, Jul 25, 2009 at 2:13 AM, Johannes Berg<[email protected]> wrote:
> Obviously 3) is my favourite solution, not only because it's the only
> one that seems feasible at all, but it's somewhat more complex, and I
> don't really have time to do that today.

Hm yeah this seems very appealing, it should be noted that this is
called *very* often too right now because we call this for every
beacon. If we want to merge beacon hint and 11d stuff with the added
advantage to make new functionality available to cfg80211 it would
seem then to make sense to add some sort of beacon parser so we can
extend it later if needed.

But can fullmac cards send full beacon stuff somehow to cfg80211? Can
they send 11d stuff or would they have to try to build one? Or can
they realistically only send alpha2 stuff?

Luis

2009-07-25 09:13:51

by Johannes Berg

[permalink] [raw]
Subject: Re: new lockdep warning in 2.6.31-rc3-wl

Reinette, Luis,

It's easy to see why this is happening, but I have no easy fix.

> [ 65.053694] phy0/3223 is trying to acquire lock:
> [ 65.053738] (cfg80211_mutex){+.+.+.}, at: [<ffffffffa01c9a72>] regulatory_hint_11d+0x32/0x430 [cfg80211]
> [ 65.053887]
> [ 65.053888] but task is already holding lock:
> [ 65.053968] (&ifmgd->mtx){+.+.+.}, at: [<ffffffffa0220f78>] ieee80211_sta_work+0x118/0x11c0 [mac80211]

Clearly, ifmgd->mtx has to be taken from a bunch of cfg80211 calls, but
all calls from cfg80211 into mac80211 hold the cfg80211_mutex.

Now the problem here is that regulatory_hint_11d takes the
cfg80211_mutex.

I looked at two possible solutions:

1) Remove the requirement to hold cfg80211_mutex in cfg80211 for this
function. This is my preferred solution, since I don't think a hint
function should have strange locking constraints. However, the
function accesses a whole bunch of global variables and right now I
don't see how to reduce that, except possibly by allocating and
queueing new work struct, but then I don't see how to cancel that
work struct when the wiphy is unregistered while it might be pending.

2) Move the regulatory_hint_11d call outside the ifmgd->mtx locked
section. This isn't nice, as it means we require more effort from the
callers. Also, I think it's not sufficient, because mac80211 has to
cancel_work_sync() the sta work, which would be calling this, when an
interface is taken down --- BUT taking down an interface can happen,
when it's simply removed with nl80211 (!), within cfg80211 context,
and as such again under the lock, which has the same problem.

3) Remove the regulatory_hint_11d function (or rather make it not
exported), and instead pass the country IE (or the whole beacon
frame) to regulatory_hint_found_beacon(), and process it all from the
regulatory work that hint_found_beacon queues. That also reduces the
number of entry points from drivers, and means that if the driver
uses cfg80211-based scanning, it also automatically takes part in the
regulatory infrastructure. Win.

Obviously 3) is my favourite solution, not only because it's the only
one that seems feasible at all, but it's somewhat more complex, and I
don't really have time to do that today.

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part