* [BUGFIX PATCH 0/2] Bugfixes for brcmfmac
@ 2016-08-15 9:40 Masami Hiramatsu
2016-08-15 9:40 ` [BUGFIX PATCH 1/2] brcmfmac: Check rtnl_lock is locked when removing interface Masami Hiramatsu
2016-08-15 9:41 ` Masami Hiramatsu
0 siblings, 2 replies; 14+ messages in thread
From: Masami Hiramatsu @ 2016-08-15 9:40 UTC (permalink / raw)
To: Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo,
Pieter-Paul Giesberts, Rafał Miłecki
Cc: linux-wireless, brcm80211-dev-list.pdl, netdev, linux-kernel
Hi,
Here are 2 patches for fixing bugs which I recently faced on my PC.
There are 2 bugs I've hit on brcmfmac, one issue was critical,
the other was just found when I investigated the first issue.
1) when I shutdown or reboot my pc with wifi, it always stopped
when disabling networking. I tried to just disable wifi and saw
task hung up messages on dmesg.
All those taskes were blocked on rtnl_lock according to the
stacktrace, and found a suspicious task in the list. Actually
the wpa_supplicant is blocked while stopping the interface.
2) I also tried to get more information about that and enabled
DEBUG_ATOMIC_SLEEP and got another warning in brcmfmac. That
warned a mutex (which can yeild/sleep) is held in !TASK_RUNNING
state. I've found a mutex is held when in wait_event_timeout()
condition parameter.
I traced the source code and found that #1 was caused by double
locking of rtnl_lock in brcmfmac driver, because it doesn't
check the rtnl_lock is already held in a path (actually, other
paths checked that). So I fixed it by checking rtnl_locked and
skip locking rtnl_lock. It works, but not seems the best way
to fix, since original code (rtnl_lock locking around
cfg80211_unregister_wdev) itself looks add-hoc. Anyway, since
I don't have any knowladge of this subsystem, I'd like to ask
maintainer's help.
To fix #2 issue, I've checked the mutex (vif_event_lock) in
struct brcmf_cfg80211_vif_event just protect updating other
members and can be replaced by a spinlock because in the
protected regions are not involving any scheduler related
code.
Thank you,
---
Masami Hiramatsu (2):
brcmfmac: Check rtnl_lock is locked when removing interface
brcmfmac: Change vif_event_lock to spinlock
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 26 ++++++++++----------
.../broadcom/brcm80211/brcmfmac/cfg80211.h | 2 +-
.../wireless/broadcom/brcm80211/brcmfmac/core.c | 2 +-
.../net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 8 ++++--
.../net/wireless/broadcom/brcm80211/brcmfmac/p2p.h | 2 +-
5 files changed, 21 insertions(+), 19 deletions(-)
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [BUGFIX PATCH 1/2] brcmfmac: Check rtnl_lock is locked when removing interface
2016-08-15 9:40 [BUGFIX PATCH 0/2] Bugfixes for brcmfmac Masami Hiramatsu
@ 2016-08-15 9:40 ` Masami Hiramatsu
2016-08-15 10:41 ` Rafał Miłecki
2016-08-24 13:14 ` [BUGFIX, " Kalle Valo
2016-08-15 9:41 ` Masami Hiramatsu
1 sibling, 2 replies; 14+ messages in thread
From: Masami Hiramatsu @ 2016-08-15 9:40 UTC (permalink / raw)
To: Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo,
Pieter-Paul Giesberts, Rafał Miłecki
Cc: linux-wireless, brcm80211-dev-list.pdl, netdev, linux-kernel
Check rtnl_lock is locked in brcmf_p2p_ifp_removed() by passing
rtnl_locked flag. Actually the caller brcmf_del_if() checks whether
the rtnl_lock is locked, but doesn't pass it to brcmf_p2p_ifp_removed().
Without this fix, wpa_supplicant goes softlockup with rtnl_lock
holding (this means all other process using netlink are locked up too)
e.g.
[ 4495.876627] INFO: task wpa_supplicant:7307 blocked for more than 10 seconds.
[ 4495.876632] Tainted: G W 4.8.0-rc1+ #8
[ 4495.876635] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 4495.876638] wpa_supplicant D ffff974c647b39a0 0 7307 1 0x00000000
[ 4495.876644] ffff974c647b39a0 0000000000000000 ffff974c00000000 ffff974c7dc59c58
[ 4495.876651] ffff974c6b7417c0 ffff974c645017c0 ffff974c647b4000 ffffffff86f16c08
[ 4495.876657] ffff974c645017c0 0000000000000246 00000000ffffffff ffff974c647b39b8
[ 4495.876664] Call Trace:
[ 4495.876671] [<ffffffff868aeccc>] schedule+0x3c/0x90
[ 4495.876676] [<ffffffff868af065>] schedule_preempt_disabled+0x15/0x20
[ 4495.876682] [<ffffffff868b0996>] mutex_lock_nested+0x176/0x3b0
[ 4495.876686] [<ffffffff867a2067>] ? rtnl_lock+0x17/0x20
[ 4495.876690] [<ffffffff867a2067>] rtnl_lock+0x17/0x20
[ 4495.876720] [<ffffffffc0ae9a5d>] brcmf_p2p_ifp_removed+0x4d/0x70 [brcmfmac]
[ 4495.876741] [<ffffffffc0aebde6>] brcmf_remove_interface+0x196/0x1b0 [brcmfmac]
[ 4495.876760] [<ffffffffc0ae9901>] brcmf_p2p_del_vif+0x111/0x220 [brcmfmac]
[ 4495.876777] [<ffffffffc0adefab>] brcmf_cfg80211_del_iface+0x21b/0x270 [brcmfmac]
[ 4495.876820] [<ffffffffc097b39e>] nl80211_del_interface+0xfe/0x3a0 [cfg80211]
[ 4495.876825] [<ffffffff867ca335>] genl_family_rcv_msg+0x1b5/0x370
[ 4495.876832] [<ffffffff860e5d8d>] ? trace_hardirqs_on+0xd/0x10
[ 4495.876836] [<ffffffff867ca56d>] genl_rcv_msg+0x7d/0xb0
[ 4495.876839] [<ffffffff867ca4f0>] ? genl_family_rcv_msg+0x370/0x370
[ 4495.876846] [<ffffffff867c9a47>] netlink_rcv_skb+0x97/0xb0
[ 4495.876849] [<ffffffff867ca168>] genl_rcv+0x28/0x40
[ 4495.876854] [<ffffffff867c93c3>] netlink_unicast+0x1d3/0x2f0
[ 4495.876860] [<ffffffff867c933b>] ? netlink_unicast+0x14b/0x2f0
[ 4495.876866] [<ffffffff867c97cb>] netlink_sendmsg+0x2eb/0x3a0
[ 4495.876870] [<ffffffff8676dad8>] sock_sendmsg+0x38/0x50
[ 4495.876874] [<ffffffff8676e4df>] ___sys_sendmsg+0x27f/0x290
[ 4495.876882] [<ffffffff8628b935>] ? mntput_no_expire+0x5/0x3f0
[ 4495.876888] [<ffffffff8628b9be>] ? mntput_no_expire+0x8e/0x3f0
[ 4495.876894] [<ffffffff8628b935>] ? mntput_no_expire+0x5/0x3f0
[ 4495.876899] [<ffffffff8628bd44>] ? mntput+0x24/0x40
[ 4495.876904] [<ffffffff86267830>] ? __fput+0x190/0x200
[ 4495.876909] [<ffffffff8676f125>] __sys_sendmsg+0x45/0x80
[ 4495.876914] [<ffffffff8676f172>] SyS_sendmsg+0x12/0x20
[ 4495.876918] [<ffffffff868b5680>] entry_SYSCALL_64_fastpath+0x23/0xc1
[ 4495.876924] [<ffffffff860e2b8f>] ? trace_hardirqs_off_caller+0x1f/0xc0
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
.../wireless/broadcom/brcm80211/brcmfmac/core.c | 2 +-
.../net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 8 +++++---
.../net/wireless/broadcom/brcm80211/brcmfmac/p2p.h | 2 +-
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 8d16f02..65e8c87 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -743,7 +743,7 @@ static void brcmf_del_if(struct brcmf_pub *drvr, s32 bsscfgidx,
* serious troublesome side effects. The p2p module will clean
* up the ifp if needed.
*/
- brcmf_p2p_ifp_removed(ifp);
+ brcmf_p2p_ifp_removed(ifp, rtnl_locked);
kfree(ifp);
}
}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index 66f942f..de19c7c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -2297,7 +2297,7 @@ int brcmf_p2p_del_vif(struct wiphy *wiphy, struct wireless_dev *wdev)
return err;
}
-void brcmf_p2p_ifp_removed(struct brcmf_if *ifp)
+void brcmf_p2p_ifp_removed(struct brcmf_if *ifp, bool rtnl_locked)
{
struct brcmf_cfg80211_info *cfg;
struct brcmf_cfg80211_vif *vif;
@@ -2306,9 +2306,11 @@ void brcmf_p2p_ifp_removed(struct brcmf_if *ifp)
vif = ifp->vif;
cfg = wdev_to_cfg(&vif->wdev);
cfg->p2p.bss_idx[P2PAPI_BSSCFG_DEVICE].vif = NULL;
- rtnl_lock();
+ if (!rtnl_locked)
+ rtnl_lock();
cfg80211_unregister_wdev(&vif->wdev);
- rtnl_unlock();
+ if (!rtnl_locked)
+ rtnl_unlock();
brcmf_free_vif(vif);
}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h
index a3bd18c..8ce9447 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h
@@ -155,7 +155,7 @@ struct wireless_dev *brcmf_p2p_add_vif(struct wiphy *wiphy, const char *name,
int brcmf_p2p_del_vif(struct wiphy *wiphy, struct wireless_dev *wdev);
int brcmf_p2p_ifchange(struct brcmf_cfg80211_info *cfg,
enum brcmf_fil_p2p_if_types if_type);
-void brcmf_p2p_ifp_removed(struct brcmf_if *ifp);
+void brcmf_p2p_ifp_removed(struct brcmf_if *ifp, bool rtnl_locked);
int brcmf_p2p_start_device(struct wiphy *wiphy, struct wireless_dev *wdev);
void brcmf_p2p_stop_device(struct wiphy *wiphy, struct wireless_dev *wdev);
int brcmf_p2p_scan_prep(struct wiphy *wiphy,
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [BUGFIX PATCH 2/2] brcmfmac: Change vif_event_lock to spinlock
2016-08-15 9:40 [BUGFIX PATCH 0/2] Bugfixes for brcmfmac Masami Hiramatsu
@ 2016-08-15 9:41 ` Masami Hiramatsu
2016-08-15 9:41 ` Masami Hiramatsu
1 sibling, 0 replies; 14+ messages in thread
From: Masami Hiramatsu @ 2016-08-15 9:41 UTC (permalink / raw)
To: Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo,
Pieter-Paul Giesberts, Rafał Miłecki
Cc: linux-wireless, brcm80211-dev-list.pdl, netdev, linux-kernel
Change vif_event_lock to spinlock from mutex, since this lock is
used in wait_event_timeout() via vif_event_equals(). This caused
a warning report as below.
As far as I can see, this lock protects regions where updating
structure members, not function calls. Also, since those
regions are not called from interrupt handlers (of course, it
was a mutex), spin_lock is used instead of spin_lock_irqsave.
[ 186.678550] ------------[ cut here ]------------
[ 186.678556] WARNING: CPU: 2 PID: 7140 at /home/mhiramat/ksrc/linux/kernel/sched/core.c:7545 __might_sleep+0x7c/0x80
[ 186.678560] do not call blocking ops when !TASK_RUNNING; state=2 set at [<ffffffff980d9090>] prepare_to_wait_event+0x60/0x100
[ 186.678560] Modules linked in: brcmfmac xt_CHECKSUM rfcomm ipt_MASQUERADE nf_nat_masquerade_ipv4 xt_addrtype br_netfilter xt_tcpudp ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 xt_conntrack ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_raw ip6table_security ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_filter ip6_tables iptable_raw iptable_security iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_filter ip_tables x_tables bnep nls_iso8859_1 i2c_designware_platform i2c_designware_core snd_hda_codec_hdmi snd_hda_codec_realtek dcdbas snd_hda_codec_generic snd_hda_intel snd_hda_codec intel_rapl snd_hda_core x86_pkg_temp_thermal intel_powerclamp coretemp
[ 186.678594] snd_pcm crct10dif_pclmul crc32_pclmul aesni_intel aes_x86_64 joydev glue_helper snd_hwdep lrw gf128mul uvcvideo ablk_helper snd_seq_midi cryptd snd_seq_midi_event snd_rawmidi videobuf2_vmalloc videobuf2_memops snd_seq input_leds videobuf2_v4l2 cfg80211 videobuf2_core snd_timer videodev serio_raw btusb snd_seq_device media btrtl rtsx_pci_ms snd mei_me memstick hid_multitouch mei soundcore brcmutil idma64 virt_dma intel_lpss_pci processor_thermal_device intel_soc_dts_iosf hci_uart btbcm btqca btintel bluetooth int3403_thermal dell_smo8800 intel_lpss_acpi intel_lpss int3402_thermal int340x_thermal_zone intel_hid mac_hid int3400_thermal shpchp sparse_keymap acpi_pad acpi_thermal_rel acpi_als kfifo_buf industrialio kvm_intel kvm irqbypass parport_pc ppdev lp parport autofs4 btrfs xor raid6_pq
[ 186.678631] usbhid nouveau ttm i915 rtsx_pci_sdmmc mxm_wmi i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops psmouse drm ahci rtsx_pci nvme nvme_core libahci i2c_hid hid pinctrl_sunrisepoint video wmi pinctrl_intel fjes [last unloaded: brcmfmac]
[ 186.678646] CPU: 2 PID: 7140 Comm: wpa_supplicant Not tainted 4.8.0-rc1+ #8
[ 186.678647] Hardware name: Dell Inc. XPS 15 9550/0N7TVV, BIOS 01.02.00 04/07/2016
[ 186.678648] 0000000000000000 ffff9d8c64b5b900 ffffffff98442f23 ffff9d8c64b5b950
[ 186.678651] 0000000000000000 ffff9d8c64b5b940 ffffffff9808b22b 00001d790000000d
[ 186.678653] ffffffff98c75e78 000000000000026c 0000000000000000 ffff9d8c2706d058
[ 186.678655] Call Trace:
[ 186.678659] [<ffffffff98442f23>] dump_stack+0x85/0xc2
[ 186.678666] [<ffffffff9808b22b>] __warn+0xcb/0xf0
[ 186.678668] [<ffffffff9808b29f>] warn_slowpath_fmt+0x4f/0x60
[ 186.678671] [<ffffffff980d9090>] ? prepare_to_wait_event+0x60/0x100
[ 186.678672] [<ffffffff980d9090>] ? prepare_to_wait_event+0x60/0x100
[ 186.678674] [<ffffffff980b922c>] __might_sleep+0x7c/0x80
[ 186.678680] [<ffffffff988b0853>] mutex_lock_nested+0x33/0x3b0
[ 186.678682] [<ffffffff980e5d8d>] ? trace_hardirqs_on+0xd/0x10
[ 186.678689] [<ffffffffc0c57d2d>] brcmf_cfg80211_wait_vif_event+0xcd/0x130 [brcmfmac]
[ 186.678691] [<ffffffff980d9190>] ? wake_atomic_t_function+0x60/0x60
[ 186.678697] [<ffffffffc0c628e9>] brcmf_p2p_del_vif+0xf9/0x220 [brcmfmac]
[ 186.678702] [<ffffffffc0c57fab>] brcmf_cfg80211_del_iface+0x21b/0x270 [brcmfmac]
[ 186.678716] [<ffffffffc0b0539e>] nl80211_del_interface+0xfe/0x3a0 [cfg80211]
[ 186.678718] [<ffffffff987ca335>] genl_family_rcv_msg+0x1b5/0x370
[ 186.678720] [<ffffffff980e5d8d>] ? trace_hardirqs_on+0xd/0x10
[ 186.678721] [<ffffffff987ca56d>] genl_rcv_msg+0x7d/0xb0
[ 186.678722] [<ffffffff987ca4f0>] ? genl_family_rcv_msg+0x370/0x370
[ 186.678724] [<ffffffff987c9a47>] netlink_rcv_skb+0x97/0xb0
[ 186.678726] [<ffffffff987ca168>] genl_rcv+0x28/0x40
[ 186.678727] [<ffffffff987c93c3>] netlink_unicast+0x1d3/0x2f0
[ 186.678729] [<ffffffff987c933b>] ? netlink_unicast+0x14b/0x2f0
[ 186.678731] [<ffffffff987c97cb>] netlink_sendmsg+0x2eb/0x3a0
[ 186.678733] [<ffffffff9876dad8>] sock_sendmsg+0x38/0x50
[ 186.678734] [<ffffffff9876e4df>] ___sys_sendmsg+0x27f/0x290
[ 186.678737] [<ffffffff9828b935>] ? mntput_no_expire+0x5/0x3f0
[ 186.678739] [<ffffffff9828b9be>] ? mntput_no_expire+0x8e/0x3f0
[ 186.678741] [<ffffffff9828b935>] ? mntput_no_expire+0x5/0x3f0
[ 186.678743] [<ffffffff9828bd44>] ? mntput+0x24/0x40
[ 186.678744] [<ffffffff98267830>] ? __fput+0x190/0x200
[ 186.678746] [<ffffffff9876f125>] __sys_sendmsg+0x45/0x80
[ 186.678748] [<ffffffff9876f172>] SyS_sendmsg+0x12/0x20
[ 186.678749] [<ffffffff988b5680>] entry_SYSCALL_64_fastpath+0x23/0xc1
[ 186.678751] [<ffffffff980e2b8f>] ? trace_hardirqs_off_caller+0x1f/0xc0
[ 186.678752] ---[ end trace e224d66c5d8408b5 ]---
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 26 ++++++++++----------
.../broadcom/brcm80211/brcmfmac/cfg80211.h | 2 +-
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 2628d5e..5db56a7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5635,7 +5635,7 @@ static s32 brcmf_notify_vif_event(struct brcmf_if *ifp,
ifevent->action, ifevent->flags, ifevent->ifidx,
ifevent->bsscfgidx);
- mutex_lock(&event->vif_event_lock);
+ spin_lock(&event->vif_event_lock);
event->action = ifevent->action;
vif = event->vif;
@@ -5643,7 +5643,7 @@ static s32 brcmf_notify_vif_event(struct brcmf_if *ifp,
case BRCMF_E_IF_ADD:
/* waiting process may have timed out */
if (!cfg->vif_event.vif) {
- mutex_unlock(&event->vif_event_lock);
+ spin_unlock(&event->vif_event_lock);
return -EBADF;
}
@@ -5654,24 +5654,24 @@ static s32 brcmf_notify_vif_event(struct brcmf_if *ifp,
ifp->ndev->ieee80211_ptr = &vif->wdev;
SET_NETDEV_DEV(ifp->ndev, wiphy_dev(cfg->wiphy));
}
- mutex_unlock(&event->vif_event_lock);
+ spin_unlock(&event->vif_event_lock);
wake_up(&event->vif_wq);
return 0;
case BRCMF_E_IF_DEL:
- mutex_unlock(&event->vif_event_lock);
+ spin_unlock(&event->vif_event_lock);
/* event may not be upon user request */
if (brcmf_cfg80211_vif_event_armed(cfg))
wake_up(&event->vif_wq);
return 0;
case BRCMF_E_IF_CHANGE:
- mutex_unlock(&event->vif_event_lock);
+ spin_unlock(&event->vif_event_lock);
wake_up(&event->vif_wq);
return 0;
default:
- mutex_unlock(&event->vif_event_lock);
+ spin_unlock(&event->vif_event_lock);
break;
}
return -EINVAL;
@@ -5792,7 +5792,7 @@ static void wl_deinit_priv(struct brcmf_cfg80211_info *cfg)
static void init_vif_event(struct brcmf_cfg80211_vif_event *event)
{
init_waitqueue_head(&event->vif_wq);
- mutex_init(&event->vif_event_lock);
+ spin_lock_init(&event->vif_event_lock);
}
static s32 brcmf_dongle_roam(struct brcmf_if *ifp)
@@ -6691,9 +6691,9 @@ static inline bool vif_event_equals(struct brcmf_cfg80211_vif_event *event,
{
u8 evt_action;
- mutex_lock(&event->vif_event_lock);
+ spin_lock(&event->vif_event_lock);
evt_action = event->action;
- mutex_unlock(&event->vif_event_lock);
+ spin_unlock(&event->vif_event_lock);
return evt_action == action;
}
@@ -6702,10 +6702,10 @@ void brcmf_cfg80211_arm_vif_event(struct brcmf_cfg80211_info *cfg,
{
struct brcmf_cfg80211_vif_event *event = &cfg->vif_event;
- mutex_lock(&event->vif_event_lock);
+ spin_lock(&event->vif_event_lock);
event->vif = vif;
event->action = 0;
- mutex_unlock(&event->vif_event_lock);
+ spin_unlock(&event->vif_event_lock);
}
bool brcmf_cfg80211_vif_event_armed(struct brcmf_cfg80211_info *cfg)
@@ -6713,9 +6713,9 @@ bool brcmf_cfg80211_vif_event_armed(struct brcmf_cfg80211_info *cfg)
struct brcmf_cfg80211_vif_event *event = &cfg->vif_event;
bool armed;
- mutex_lock(&event->vif_event_lock);
+ spin_lock(&event->vif_event_lock);
armed = event->vif != NULL;
- mutex_unlock(&event->vif_event_lock);
+ spin_unlock(&event->vif_event_lock);
return armed;
}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
index 7d77f86..8889832 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
@@ -227,7 +227,7 @@ struct escan_info {
*/
struct brcmf_cfg80211_vif_event {
wait_queue_head_t vif_wq;
- struct mutex vif_event_lock;
+ spinlock_t vif_event_lock;
u8 action;
struct brcmf_cfg80211_vif *vif;
};
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [BUGFIX PATCH 2/2] brcmfmac: Change vif_event_lock to spinlock
@ 2016-08-15 9:41 ` Masami Hiramatsu
0 siblings, 0 replies; 14+ messages in thread
From: Masami Hiramatsu @ 2016-08-15 9:41 UTC (permalink / raw)
To: Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo,
Pieter-Paul Giesberts, Rafał Miłecki
Cc: linux-wireless, brcm80211-dev-list.pdl, netdev, linux-kernel
Change vif_event_lock to spinlock from mutex, since this lock is
used in wait_event_timeout() via vif_event_equals(). This caused
a warning report as below.
As far as I can see, this lock protects regions where updating
structure members, not function calls. Also, since those
regions are not called from interrupt handlers (of course, it
was a mutex), spin_lock is used instead of spin_lock_irqsave.
[ 186.678550] ------------[ cut here ]------------
[ 186.678556] WARNING: CPU: 2 PID: 7140 at /home/mhiramat/ksrc/linux/kernel/sched/core.c:7545 __might_sleep+0x7c/0x80
[ 186.678560] do not call blocking ops when !TASK_RUNNING; state=2 set at [<ffffffff980d9090>] prepare_to_wait_event+0x60/0x100
[ 186.678560] Modules linked in: brcmfmac xt_CHECKSUM rfcomm ipt_MASQUERADE nf_nat_masquerade_ipv4 xt_addrtype br_netfilter xt_tcpudp ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 xt_conntrack ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_raw ip6table_security ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_filter ip6_tables iptable_raw iptable_security iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_filter ip_tables x_tables bnep nls_iso8859_1 i2c_designware_platform i2c_designware_core snd_hda_codec_hdmi snd_hda_codec_realtek dcdbas snd_hda_codec_generic snd_hda_intel snd_hda_codec intel_rapl snd_hda_core x86_pkg_temp_thermal intel_pow
erclamp coretemp
[ 186.678594] snd_pcm crct10dif_pclmul crc32_pclmul aesni_intel aes_x86_64 joydev glue_helper snd_hwdep lrw gf128mul uvcvideo ablk_helper snd_seq_midi cryptd snd_seq_midi_event snd_rawmidi videobuf2_vmalloc videobuf2_memops snd_seq input_leds videobuf2_v4l2 cfg80211 videobuf2_core snd_timer videodev serio_raw btusb snd_seq_device media btrtl rtsx_pci_ms snd mei_me memstick hid_multitouch mei soundcore brcmutil idma64 virt_dma intel_lpss_pci processor_thermal_device intel_soc_dts_iosf hci_uart btbcm btqca btintel bluetooth int3403_thermal dell_smo8800 intel_lpss_acpi intel_lpss int3402_thermal int340x_thermal_zone intel_hid mac_hid int3400_thermal shpchp sparse_keymap acpi_pad acpi_thermal_rel acpi_als kfifo_buf industrialio kvm_intel kvm irqbypass parport_pc ppdev lp parport autofs4 btrf
s xor raid6_pq
[ 186.678631] usbhid nouveau ttm i915 rtsx_pci_sdmmc mxm_wmi i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops psmouse drm ahci rtsx_pci nvme nvme_core libahci i2c_hid hid pinctrl_sunrisepoint video wmi pinctrl_intel fjes [last unloaded: brcmfmac]
[ 186.678646] CPU: 2 PID: 7140 Comm: wpa_supplicant Not tainted 4.8.0-rc1+ #8
[ 186.678647] Hardware name: Dell Inc. XPS 15 9550/0N7TVV, BIOS 01.02.00 04/07/2016
[ 186.678648] 0000000000000000 ffff9d8c64b5b900 ffffffff98442f23 ffff9d8c64b5b950
[ 186.678651] 0000000000000000 ffff9d8c64b5b940 ffffffff9808b22b 00001d790000000d
[ 186.678653] ffffffff98c75e78 000000000000026c 0000000000000000 ffff9d8c2706d058
[ 186.678655] Call Trace:
[ 186.678659] [<ffffffff98442f23>] dump_stack+0x85/0xc2
[ 186.678666] [<ffffffff9808b22b>] __warn+0xcb/0xf0
[ 186.678668] [<ffffffff9808b29f>] warn_slowpath_fmt+0x4f/0x60
[ 186.678671] [<ffffffff980d9090>] ? prepare_to_wait_event+0x60/0x100
[ 186.678672] [<ffffffff980d9090>] ? prepare_to_wait_event+0x60/0x100
[ 186.678674] [<ffffffff980b922c>] __might_sleep+0x7c/0x80
[ 186.678680] [<ffffffff988b0853>] mutex_lock_nested+0x33/0x3b0
[ 186.678682] [<ffffffff980e5d8d>] ? trace_hardirqs_on+0xd/0x10
[ 186.678689] [<ffffffffc0c57d2d>] brcmf_cfg80211_wait_vif_event+0xcd/0x130 [brcmfmac]
[ 186.678691] [<ffffffff980d9190>] ? wake_atomic_t_function+0x60/0x60
[ 186.678697] [<ffffffffc0c628e9>] brcmf_p2p_del_vif+0xf9/0x220 [brcmfmac]
[ 186.678702] [<ffffffffc0c57fab>] brcmf_cfg80211_del_iface+0x21b/0x270 [brcmfmac]
[ 186.678716] [<ffffffffc0b0539e>] nl80211_del_interface+0xfe/0x3a0 [cfg80211]
[ 186.678718] [<ffffffff987ca335>] genl_family_rcv_msg+0x1b5/0x370
[ 186.678720] [<ffffffff980e5d8d>] ? trace_hardirqs_on+0xd/0x10
[ 186.678721] [<ffffffff987ca56d>] genl_rcv_msg+0x7d/0xb0
[ 186.678722] [<ffffffff987ca4f0>] ? genl_family_rcv_msg+0x370/0x370
[ 186.678724] [<ffffffff987c9a47>] netlink_rcv_skb+0x97/0xb0
[ 186.678726] [<ffffffff987ca168>] genl_rcv+0x28/0x40
[ 186.678727] [<ffffffff987c93c3>] netlink_unicast+0x1d3/0x2f0
[ 186.678729] [<ffffffff987c933b>] ? netlink_unicast+0x14b/0x2f0
[ 186.678731] [<ffffffff987c97cb>] netlink_sendmsg+0x2eb/0x3a0
[ 186.678733] [<ffffffff9876dad8>] sock_sendmsg+0x38/0x50
[ 186.678734] [<ffffffff9876e4df>] ___sys_sendmsg+0x27f/0x290
[ 186.678737] [<ffffffff9828b935>] ? mntput_no_expire+0x5/0x3f0
[ 186.678739] [<ffffffff9828b9be>] ? mntput_no_expire+0x8e/0x3f0
[ 186.678741] [<ffffffff9828b935>] ? mntput_no_expire+0x5/0x3f0
[ 186.678743] [<ffffffff9828bd44>] ? mntput+0x24/0x40
[ 186.678744] [<ffffffff98267830>] ? __fput+0x190/0x200
[ 186.678746] [<ffffffff9876f125>] __sys_sendmsg+0x45/0x80
[ 186.678748] [<ffffffff9876f172>] SyS_sendmsg+0x12/0x20
[ 186.678749] [<ffffffff988b5680>] entry_SYSCALL_64_fastpath+0x23/0xc1
[ 186.678751] [<ffffffff980e2b8f>] ? trace_hardirqs_off_caller+0x1f/0xc0
[ 186.678752] ---[ end trace e224d66c5d8408b5 ]---
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 26 ++++++++++----------
.../broadcom/brcm80211/brcmfmac/cfg80211.h | 2 +-
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 2628d5e..5db56a7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5635,7 +5635,7 @@ static s32 brcmf_notify_vif_event(struct brcmf_if *ifp,
ifevent->action, ifevent->flags, ifevent->ifidx,
ifevent->bsscfgidx);
- mutex_lock(&event->vif_event_lock);
+ spin_lock(&event->vif_event_lock);
event->action = ifevent->action;
vif = event->vif;
@@ -5643,7 +5643,7 @@ static s32 brcmf_notify_vif_event(struct brcmf_if *ifp,
case BRCMF_E_IF_ADD:
/* waiting process may have timed out */
if (!cfg->vif_event.vif) {
- mutex_unlock(&event->vif_event_lock);
+ spin_unlock(&event->vif_event_lock);
return -EBADF;
}
@@ -5654,24 +5654,24 @@ static s32 brcmf_notify_vif_event(struct brcmf_if *ifp,
ifp->ndev->ieee80211_ptr = &vif->wdev;
SET_NETDEV_DEV(ifp->ndev, wiphy_dev(cfg->wiphy));
}
- mutex_unlock(&event->vif_event_lock);
+ spin_unlock(&event->vif_event_lock);
wake_up(&event->vif_wq);
return 0;
case BRCMF_E_IF_DEL:
- mutex_unlock(&event->vif_event_lock);
+ spin_unlock(&event->vif_event_lock);
/* event may not be upon user request */
if (brcmf_cfg80211_vif_event_armed(cfg))
wake_up(&event->vif_wq);
return 0;
case BRCMF_E_IF_CHANGE:
- mutex_unlock(&event->vif_event_lock);
+ spin_unlock(&event->vif_event_lock);
wake_up(&event->vif_wq);
return 0;
default:
- mutex_unlock(&event->vif_event_lock);
+ spin_unlock(&event->vif_event_lock);
break;
}
return -EINVAL;
@@ -5792,7 +5792,7 @@ static void wl_deinit_priv(struct brcmf_cfg80211_info *cfg)
static void init_vif_event(struct brcmf_cfg80211_vif_event *event)
{
init_waitqueue_head(&event->vif_wq);
- mutex_init(&event->vif_event_lock);
+ spin_lock_init(&event->vif_event_lock);
}
static s32 brcmf_dongle_roam(struct brcmf_if *ifp)
@@ -6691,9 +6691,9 @@ static inline bool vif_event_equals(struct brcmf_cfg80211_vif_event *event,
{
u8 evt_action;
- mutex_lock(&event->vif_event_lock);
+ spin_lock(&event->vif_event_lock);
evt_action = event->action;
- mutex_unlock(&event->vif_event_lock);
+ spin_unlock(&event->vif_event_lock);
return evt_action == action;
}
@@ -6702,10 +6702,10 @@ void brcmf_cfg80211_arm_vif_event(struct brcmf_cfg80211_info *cfg,
{
struct brcmf_cfg80211_vif_event *event = &cfg->vif_event;
- mutex_lock(&event->vif_event_lock);
+ spin_lock(&event->vif_event_lock);
event->vif = vif;
event->action = 0;
- mutex_unlock(&event->vif_event_lock);
+ spin_unlock(&event->vif_event_lock);
}
bool brcmf_cfg80211_vif_event_armed(struct brcmf_cfg80211_info *cfg)
@@ -6713,9 +6713,9 @@ bool brcmf_cfg80211_vif_event_armed(struct brcmf_cfg80211_info *cfg)
struct brcmf_cfg80211_vif_event *event = &cfg->vif_event;
bool armed;
- mutex_lock(&event->vif_event_lock);
+ spin_lock(&event->vif_event_lock);
armed = event->vif != NULL;
- mutex_unlock(&event->vif_event_lock);
+ spin_unlock(&event->vif_event_lock);
return armed;
}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
index 7d77f86..8889832 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
@@ -227,7 +227,7 @@ struct escan_info {
*/
struct brcmf_cfg80211_vif_event {
wait_queue_head_t vif_wq;
- struct mutex vif_event_lock;
+ spinlock_t vif_event_lock;
u8 action;
struct brcmf_cfg80211_vif *vif;
};
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [BUGFIX PATCH 1/2] brcmfmac: Check rtnl_lock is locked when removing interface
2016-08-15 9:40 ` [BUGFIX PATCH 1/2] brcmfmac: Check rtnl_lock is locked when removing interface Masami Hiramatsu
@ 2016-08-15 10:41 ` Rafał Miłecki
2016-08-15 10:57 ` Kalle Valo
2016-08-24 13:14 ` [BUGFIX, " Kalle Valo
1 sibling, 1 reply; 14+ messages in thread
From: Rafał Miłecki @ 2016-08-15 10:41 UTC (permalink / raw)
To: Masami Hiramatsu, Arend van Spriel, Franky Lin, Hante Meuleman,
Kalle Valo, Pieter-Paul Giesberts
Cc: linux-wireless, brcm80211-dev-list.pdl, netdev, linux-kernel
On 08/15/2016 11:40 AM, Masami Hiramatsu wrote:
> Check rtnl_lock is locked in brcmf_p2p_ifp_removed() by passing
> rtnl_locked flag. Actually the caller brcmf_del_if() checks whether
> the rtnl_lock is locked, but doesn't pass it to brcmf_p2p_ifp_removed().
>
> Without this fix, wpa_supplicant goes softlockup with rtnl_lock
> holding (this means all other process using netlink are locked up too)
>
> e.g.
> [ 4495.876627] INFO: task wpa_supplicant:7307 blocked for more than 10 seconds.
> [ 4495.876632] Tainted: G W 4.8.0-rc1+ #8
> [ 4495.876635] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 4495.876638] wpa_supplicant D ffff974c647b39a0 0 7307 1 0x00000000
> [ 4495.876644] ffff974c647b39a0 0000000000000000 ffff974c00000000 ffff974c7dc59c58
> [ 4495.876651] ffff974c6b7417c0 ffff974c645017c0 ffff974c647b4000 ffffffff86f16c08
> [ 4495.876657] ffff974c645017c0 0000000000000246 00000000ffffffff ffff974c647b39b8
> [ 4495.876664] Call Trace:
> [ 4495.876671] [<ffffffff868aeccc>] schedule+0x3c/0x90
> [ 4495.876676] [<ffffffff868af065>] schedule_preempt_disabled+0x15/0x20
> [ 4495.876682] [<ffffffff868b0996>] mutex_lock_nested+0x176/0x3b0
> [ 4495.876686] [<ffffffff867a2067>] ? rtnl_lock+0x17/0x20
> [ 4495.876690] [<ffffffff867a2067>] rtnl_lock+0x17/0x20
> [ 4495.876720] [<ffffffffc0ae9a5d>] brcmf_p2p_ifp_removed+0x4d/0x70 [brcmfmac]
> [ 4495.876741] [<ffffffffc0aebde6>] brcmf_remove_interface+0x196/0x1b0 [brcmfmac]
> [ 4495.876760] [<ffffffffc0ae9901>] brcmf_p2p_del_vif+0x111/0x220 [brcmfmac]
> [ 4495.876777] [<ffffffffc0adefab>] brcmf_cfg80211_del_iface+0x21b/0x270 [brcmfmac]
> [ 4495.876820] [<ffffffffc097b39e>] nl80211_del_interface+0xfe/0x3a0 [cfg80211]
> [ 4495.876825] [<ffffffff867ca335>] genl_family_rcv_msg+0x1b5/0x370
> [ 4495.876832] [<ffffffff860e5d8d>] ? trace_hardirqs_on+0xd/0x10
> [ 4495.876836] [<ffffffff867ca56d>] genl_rcv_msg+0x7d/0xb0
> [ 4495.876839] [<ffffffff867ca4f0>] ? genl_family_rcv_msg+0x370/0x370
> [ 4495.876846] [<ffffffff867c9a47>] netlink_rcv_skb+0x97/0xb0
> [ 4495.876849] [<ffffffff867ca168>] genl_rcv+0x28/0x40
> [ 4495.876854] [<ffffffff867c93c3>] netlink_unicast+0x1d3/0x2f0
> [ 4495.876860] [<ffffffff867c933b>] ? netlink_unicast+0x14b/0x2f0
> [ 4495.876866] [<ffffffff867c97cb>] netlink_sendmsg+0x2eb/0x3a0
> [ 4495.876870] [<ffffffff8676dad8>] sock_sendmsg+0x38/0x50
> [ 4495.876874] [<ffffffff8676e4df>] ___sys_sendmsg+0x27f/0x290
> [ 4495.876882] [<ffffffff8628b935>] ? mntput_no_expire+0x5/0x3f0
> [ 4495.876888] [<ffffffff8628b9be>] ? mntput_no_expire+0x8e/0x3f0
> [ 4495.876894] [<ffffffff8628b935>] ? mntput_no_expire+0x5/0x3f0
> [ 4495.876899] [<ffffffff8628bd44>] ? mntput+0x24/0x40
> [ 4495.876904] [<ffffffff86267830>] ? __fput+0x190/0x200
> [ 4495.876909] [<ffffffff8676f125>] __sys_sendmsg+0x45/0x80
> [ 4495.876914] [<ffffffff8676f172>] SyS_sendmsg+0x12/0x20
> [ 4495.876918] [<ffffffff868b5680>] entry_SYSCALL_64_fastpath+0x23/0xc1
> [ 4495.876924] [<ffffffff860e2b8f>] ? trace_hardirqs_off_caller+0x1f/0xc0
This is probably caused by my commit:
a63b09872c1d ("brcmfmac: delete interface directly in code that sent fw request")
https://git.kernel.org/cgit/linux/kernel/git/kvalo/wireless-drivers-next.git/commit/?id=a63b09872c1dc0ce0da3628647da67a112b484bf
I changed condition for calling brcmf_remove_interface and it seems it broke P2P. Unfortunately I couldn't fully test my change due to firmware not supporting P2P.
I did similar fix for error path for P2P with commit
b50ddfa8530e ("brcmfmac: fix lockup when removing P2P interface after event timeout")
https://git.kernel.org/cgit/linux/kernel/git/kvalo/wireless-drivers-next.git/commit/?id=b50ddfa8530e9b5f52e873fdd6ff04f327a88799
so your change looks like a proper follow-up.
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Fixes: a63b09872c1d ("brcmfmac: delete interface directly in code that sent fw request")
Acked-by: Rafał Miłecki <rafal@milecki.pl>
Kalle: I'm acking this as bugfix for 4.8 release.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUGFIX PATCH 1/2] brcmfmac: Check rtnl_lock is locked when removing interface
2016-08-15 10:41 ` Rafał Miłecki
@ 2016-08-15 10:57 ` Kalle Valo
2016-08-15 11:52 ` Rafał Miłecki
0 siblings, 1 reply; 14+ messages in thread
From: Kalle Valo @ 2016-08-15 10:57 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Masami Hiramatsu, Arend van Spriel, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, linux-wireless, brcm80211-dev-list.pdl,
netdev, linux-kernel
Rafał Miłecki <zajec5@gmail.com> writes:
>> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
>
> Fixes: a63b09872c1d ("brcmfmac: delete interface directly in code that sent fw request")
> Acked-by: Rafał Miłecki <rafal@milecki.pl>
>
> Kalle: I'm acking this as bugfix for 4.8 release.
Ok. I'll wait few days for more comments before I apply this.
(I assume you are talking only about patch 1)
--
Kalle Valo
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUGFIX PATCH 1/2] brcmfmac: Check rtnl_lock is locked when removing interface
2016-08-15 10:57 ` Kalle Valo
@ 2016-08-15 11:52 ` Rafał Miłecki
0 siblings, 0 replies; 14+ messages in thread
From: Rafał Miłecki @ 2016-08-15 11:52 UTC (permalink / raw)
To: Kalle Valo
Cc: Masami Hiramatsu, Arend van Spriel, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, linux-wireless,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
Network Development, Linux Kernel Mailing List
On 15 August 2016 at 12:57, Kalle Valo <kvalo@codeaurora.org> wrote:
> Rafał Miłecki <zajec5@gmail.com> writes:
>
>>> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
>>
>> Fixes: a63b09872c1d ("brcmfmac: delete interface directly in code that sent fw request")
>> Acked-by: Rafał Miłecki <rafal@milecki.pl>
>>
>> Kalle: I'm acking this as bugfix for 4.8 release.
>
> Ok. I'll wait few days for more comments before I apply this.
Sure.
> (I assume you are talking only about patch 1)
Yes, I'll leave mutex vs. spinlock to the experts :)
--
Rafał
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUGFIX PATCH 1/2] brcmfmac: Check rtnl_lock is locked when removing interface
@ 2016-08-15 11:52 ` Rafał Miłecki
0 siblings, 0 replies; 14+ messages in thread
From: Rafał Miłecki @ 2016-08-15 11:52 UTC (permalink / raw)
To: Kalle Valo
Cc: Masami Hiramatsu, Arend van Spriel, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, linux-wireless,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
Network Development, Linux Kernel Mailing List
On 15 August 2016 at 12:57, Kalle Valo <kvalo@codeaurora.org> wrote:
> Rafał Miłecki <zajec5@gmail.com> writes:
>
>>> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
>>
>> Fixes: a63b09872c1d ("brcmfmac: delete interface directly in code that sent fw request")
>> Acked-by: Rafał Miłecki <rafal@milecki.pl>
>>
>> Kalle: I'm acking this as bugfix for 4.8 release.
>
> Ok. I'll wait few days for more comments before I apply this.
Sure.
> (I assume you are talking only about patch 1)
Yes, I'll leave mutex vs. spinlock to the experts :)
--
Rafał
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUGFIX PATCH 1/2] brcmfmac: Check rtnl_lock is locked when removing interface
2016-08-15 11:52 ` Rafał Miłecki
@ 2016-08-15 21:44 ` Arend Van Spriel
-1 siblings, 0 replies; 14+ messages in thread
From: Arend Van Spriel @ 2016-08-15 21:44 UTC (permalink / raw)
To: Rafał Miłecki, Kalle Valo
Cc: Masami Hiramatsu, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, linux-wireless,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
Network Development, Linux Kernel Mailing List
On 15-8-2016 13:52, Rafał Miłecki wrote:
> On 15 August 2016 at 12:57, Kalle Valo <kvalo@codeaurora.org> wrote:
>> Rafał Miłecki <zajec5@gmail.com> writes:
>>
>>>> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
>>>
>>> Fixes: a63b09872c1d ("brcmfmac: delete interface directly in code that sent fw request")
>>> Acked-by: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> Kalle: I'm acking this as bugfix for 4.8 release.
>>
>> Ok. I'll wait few days for more comments before I apply this.
>
> Sure.
>
>
>> (I assume you are talking only about patch 1)
>
> Yes, I'll leave mutex vs. spinlock to the experts :)
Don't know who the experts are. Surely not me :-p
I made an uneducated design decision using a mutex for this. The
reasoning for using a regular spinlock make sense. So I will go and ack
that patch.
Regards,
Arend
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUGFIX PATCH 1/2] brcmfmac: Check rtnl_lock is locked when removing interface
@ 2016-08-15 21:44 ` Arend Van Spriel
0 siblings, 0 replies; 14+ messages in thread
From: Arend Van Spriel @ 2016-08-15 21:44 UTC (permalink / raw)
To: Rafał Miłecki, Kalle Valo
Cc: Masami Hiramatsu, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, linux-wireless,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
Network Development, Linux Kernel Mailing List
On 15-8-2016 13:52, Rafał Miłecki wrote:
> On 15 August 2016 at 12:57, Kalle Valo <kvalo@codeaurora.org> wrote:
>> Rafał Miłecki <zajec5@gmail.com> writes:
>>
>>>> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
>>>
>>> Fixes: a63b09872c1d ("brcmfmac: delete interface directly in code that sent fw request")
>>> Acked-by: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> Kalle: I'm acking this as bugfix for 4.8 release.
>>
>> Ok. I'll wait few days for more comments before I apply this.
>
> Sure.
>
>
>> (I assume you are talking only about patch 1)
>
> Yes, I'll leave mutex vs. spinlock to the experts :)
Don't know who the experts are. Surely not me :-p
I made an uneducated design decision using a mutex for this. The
reasoning for using a regular spinlock make sense. So I will go and ack
that patch.
Regards,
Arend
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUGFIX PATCH 2/2] brcmfmac: Change vif_event_lock to spinlock
2016-08-15 9:41 ` Masami Hiramatsu
@ 2016-08-15 21:46 ` Arend Van Spriel
-1 siblings, 0 replies; 14+ messages in thread
From: Arend Van Spriel @ 2016-08-15 21:46 UTC (permalink / raw)
To: Masami Hiramatsu, Franky Lin, Hante Meuleman, Kalle Valo,
Pieter-Paul Giesberts, Rafał Miłecki
Cc: linux-wireless, brcm80211-dev-list.pdl, netdev, linux-kernel
On 15-8-2016 11:41, Masami Hiramatsu wrote:
> Change vif_event_lock to spinlock from mutex, since this lock is
> used in wait_event_timeout() via vif_event_equals(). This caused
> a warning report as below.
>
> As far as I can see, this lock protects regions where updating
> structure members, not function calls. Also, since those
> regions are not called from interrupt handlers (of course, it
> was a mutex), spin_lock is used instead of spin_lock_irqsave.
>
> [ 186.678550] ------------[ cut here ]------------
> [ 186.678556] WARNING: CPU: 2 PID: 7140 at /home/mhiramat/ksrc/linux/kernel/sched/core.c:7545 __might_sleep+0x7c/0x80
> [ 186.678560] do not call blocking ops when !TASK_RUNNING; state=2 set at [<ffffffff980d9090>] prepare_to_wait_event+0x60/0x100
> [ 186.678560] Modules linked in: brcmfmac xt_CHECKSUM rfcomm ipt_MASQUERADE nf_nat_masquerade_ipv4 xt_addrtype br_netfilter xt_tcpudp ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 xt_conntrack ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_raw ip6table_security ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_filter ip6_tables iptable_raw iptable_security iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_filter ip_tables x_tables bnep nls_iso8859_1 i2c_designware_platform i2c_designware_core snd_hda_codec_hdmi snd_hda_codec_realtek dcdbas snd_hda_codec_generic snd_hda_intel snd_hda_codec intel_rapl snd_hda_core x86_pkg_temp_thermal intel_powerclamp coretemp
> [ 186.678594] snd_pcm crct10dif_pclmul crc32_pclmul aesni_intel aes_x86_64 joydev glue_helper snd_hwdep lrw gf128mul uvcvideo ablk_helper snd_seq_midi cryptd snd_seq_midi_event snd_rawmidi videobuf2_vmalloc videobuf2_memops snd_seq input_leds videobuf2_v4l2 cfg80211 videobuf2_core snd_timer videodev serio_raw btusb snd_seq_device media btrtl rtsx_pci_ms snd mei_me memstick hid_multitouch mei soundcore brcmutil idma64 virt_dma intel_lpss_pci processor_thermal_device intel_soc_dts_iosf hci_uart btbcm btqca btintel bluetooth int3403_thermal dell_smo8800 intel_lpss_acpi intel_lpss int3402_thermal int340x_thermal_zone intel_hid mac_hid int3400_thermal shpchp sparse_keymap acpi_pad acpi_thermal_rel acpi_als kfifo_buf industrialio kvm_intel kvm irqbypass parport_pc ppdev lp parport autofs4 btrfs xor raid6_pq
> [ 186.678631] usbhid nouveau ttm i915 rtsx_pci_sdmmc mxm_wmi i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops psmouse drm ahci rtsx_pci nvme nvme_core libahci i2c_hid hid pinctrl_sunrisepoint video wmi pinctrl_intel fjes [last unloaded: brcmfmac]
> [ 186.678646] CPU: 2 PID: 7140 Comm: wpa_supplicant Not tainted 4.8.0-rc1+ #8
> [ 186.678647] Hardware name: Dell Inc. XPS 15 9550/0N7TVV, BIOS 01.02.00 04/07/2016
> [ 186.678648] 0000000000000000 ffff9d8c64b5b900 ffffffff98442f23 ffff9d8c64b5b950
> [ 186.678651] 0000000000000000 ffff9d8c64b5b940 ffffffff9808b22b 00001d790000000d
> [ 186.678653] ffffffff98c75e78 000000000000026c 0000000000000000 ffff9d8c2706d058
> [ 186.678655] Call Trace:
> [ 186.678659] [<ffffffff98442f23>] dump_stack+0x85/0xc2
> [ 186.678666] [<ffffffff9808b22b>] __warn+0xcb/0xf0
> [ 186.678668] [<ffffffff9808b29f>] warn_slowpath_fmt+0x4f/0x60
> [ 186.678671] [<ffffffff980d9090>] ? prepare_to_wait_event+0x60/0x100
> [ 186.678672] [<ffffffff980d9090>] ? prepare_to_wait_event+0x60/0x100
> [ 186.678674] [<ffffffff980b922c>] __might_sleep+0x7c/0x80
> [ 186.678680] [<ffffffff988b0853>] mutex_lock_nested+0x33/0x3b0
> [ 186.678682] [<ffffffff980e5d8d>] ? trace_hardirqs_on+0xd/0x10
> [ 186.678689] [<ffffffffc0c57d2d>] brcmf_cfg80211_wait_vif_event+0xcd/0x130 [brcmfmac]
> [ 186.678691] [<ffffffff980d9190>] ? wake_atomic_t_function+0x60/0x60
> [ 186.678697] [<ffffffffc0c628e9>] brcmf_p2p_del_vif+0xf9/0x220 [brcmfmac]
> [ 186.678702] [<ffffffffc0c57fab>] brcmf_cfg80211_del_iface+0x21b/0x270 [brcmfmac]
> [ 186.678716] [<ffffffffc0b0539e>] nl80211_del_interface+0xfe/0x3a0 [cfg80211]
> [ 186.678718] [<ffffffff987ca335>] genl_family_rcv_msg+0x1b5/0x370
> [ 186.678720] [<ffffffff980e5d8d>] ? trace_hardirqs_on+0xd/0x10
> [ 186.678721] [<ffffffff987ca56d>] genl_rcv_msg+0x7d/0xb0
> [ 186.678722] [<ffffffff987ca4f0>] ? genl_family_rcv_msg+0x370/0x370
> [ 186.678724] [<ffffffff987c9a47>] netlink_rcv_skb+0x97/0xb0
> [ 186.678726] [<ffffffff987ca168>] genl_rcv+0x28/0x40
> [ 186.678727] [<ffffffff987c93c3>] netlink_unicast+0x1d3/0x2f0
> [ 186.678729] [<ffffffff987c933b>] ? netlink_unicast+0x14b/0x2f0
> [ 186.678731] [<ffffffff987c97cb>] netlink_sendmsg+0x2eb/0x3a0
> [ 186.678733] [<ffffffff9876dad8>] sock_sendmsg+0x38/0x50
> [ 186.678734] [<ffffffff9876e4df>] ___sys_sendmsg+0x27f/0x290
> [ 186.678737] [<ffffffff9828b935>] ? mntput_no_expire+0x5/0x3f0
> [ 186.678739] [<ffffffff9828b9be>] ? mntput_no_expire+0x8e/0x3f0
> [ 186.678741] [<ffffffff9828b935>] ? mntput_no_expire+0x5/0x3f0
> [ 186.678743] [<ffffffff9828bd44>] ? mntput+0x24/0x40
> [ 186.678744] [<ffffffff98267830>] ? __fput+0x190/0x200
> [ 186.678746] [<ffffffff9876f125>] __sys_sendmsg+0x45/0x80
> [ 186.678748] [<ffffffff9876f172>] SyS_sendmsg+0x12/0x20
> [ 186.678749] [<ffffffff988b5680>] entry_SYSCALL_64_fastpath+0x23/0xc1
> [ 186.678751] [<ffffffff980e2b8f>] ? trace_hardirqs_off_caller+0x1f/0xc0
> [ 186.678752] ---[ end trace e224d66c5d8408b5 ]---
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 26 ++++++++++----------
> .../broadcom/brcm80211/brcmfmac/cfg80211.h | 2 +-
> 2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index 2628d5e..5db56a7 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -5635,7 +5635,7 @@ static s32 brcmf_notify_vif_event(struct brcmf_if *ifp,
> ifevent->action, ifevent->flags, ifevent->ifidx,
> ifevent->bsscfgidx);
>
> - mutex_lock(&event->vif_event_lock);
> + spin_lock(&event->vif_event_lock);
> event->action = ifevent->action;
> vif = event->vif;
>
> @@ -5643,7 +5643,7 @@ static s32 brcmf_notify_vif_event(struct brcmf_if *ifp,
> case BRCMF_E_IF_ADD:
> /* waiting process may have timed out */
> if (!cfg->vif_event.vif) {
> - mutex_unlock(&event->vif_event_lock);
> + spin_unlock(&event->vif_event_lock);
> return -EBADF;
> }
>
> @@ -5654,24 +5654,24 @@ static s32 brcmf_notify_vif_event(struct brcmf_if *ifp,
> ifp->ndev->ieee80211_ptr = &vif->wdev;
> SET_NETDEV_DEV(ifp->ndev, wiphy_dev(cfg->wiphy));
> }
> - mutex_unlock(&event->vif_event_lock);
> + spin_unlock(&event->vif_event_lock);
> wake_up(&event->vif_wq);
> return 0;
>
> case BRCMF_E_IF_DEL:
> - mutex_unlock(&event->vif_event_lock);
> + spin_unlock(&event->vif_event_lock);
> /* event may not be upon user request */
> if (brcmf_cfg80211_vif_event_armed(cfg))
> wake_up(&event->vif_wq);
> return 0;
>
> case BRCMF_E_IF_CHANGE:
> - mutex_unlock(&event->vif_event_lock);
> + spin_unlock(&event->vif_event_lock);
> wake_up(&event->vif_wq);
> return 0;
>
> default:
> - mutex_unlock(&event->vif_event_lock);
> + spin_unlock(&event->vif_event_lock);
> break;
> }
> return -EINVAL;
> @@ -5792,7 +5792,7 @@ static void wl_deinit_priv(struct brcmf_cfg80211_info *cfg)
> static void init_vif_event(struct brcmf_cfg80211_vif_event *event)
> {
> init_waitqueue_head(&event->vif_wq);
> - mutex_init(&event->vif_event_lock);
> + spin_lock_init(&event->vif_event_lock);
> }
>
> static s32 brcmf_dongle_roam(struct brcmf_if *ifp)
> @@ -6691,9 +6691,9 @@ static inline bool vif_event_equals(struct brcmf_cfg80211_vif_event *event,
> {
> u8 evt_action;
>
> - mutex_lock(&event->vif_event_lock);
> + spin_lock(&event->vif_event_lock);
> evt_action = event->action;
> - mutex_unlock(&event->vif_event_lock);
> + spin_unlock(&event->vif_event_lock);
> return evt_action == action;
> }
>
> @@ -6702,10 +6702,10 @@ void brcmf_cfg80211_arm_vif_event(struct brcmf_cfg80211_info *cfg,
> {
> struct brcmf_cfg80211_vif_event *event = &cfg->vif_event;
>
> - mutex_lock(&event->vif_event_lock);
> + spin_lock(&event->vif_event_lock);
> event->vif = vif;
> event->action = 0;
> - mutex_unlock(&event->vif_event_lock);
> + spin_unlock(&event->vif_event_lock);
> }
>
> bool brcmf_cfg80211_vif_event_armed(struct brcmf_cfg80211_info *cfg)
> @@ -6713,9 +6713,9 @@ bool brcmf_cfg80211_vif_event_armed(struct brcmf_cfg80211_info *cfg)
> struct brcmf_cfg80211_vif_event *event = &cfg->vif_event;
> bool armed;
>
> - mutex_lock(&event->vif_event_lock);
> + spin_lock(&event->vif_event_lock);
> armed = event->vif != NULL;
> - mutex_unlock(&event->vif_event_lock);
> + spin_unlock(&event->vif_event_lock);
>
> return armed;
> }
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> index 7d77f86..8889832 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> @@ -227,7 +227,7 @@ struct escan_info {
> */
> struct brcmf_cfg80211_vif_event {
> wait_queue_head_t vif_wq;
> - struct mutex vif_event_lock;
> + spinlock_t vif_event_lock;
> u8 action;
> struct brcmf_cfg80211_vif *vif;
> };
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUGFIX PATCH 2/2] brcmfmac: Change vif_event_lock to spinlock
@ 2016-08-15 21:46 ` Arend Van Spriel
0 siblings, 0 replies; 14+ messages in thread
From: Arend Van Spriel @ 2016-08-15 21:46 UTC (permalink / raw)
To: Masami Hiramatsu, Franky Lin, Hante Meuleman, Kalle Valo,
Pieter-Paul Giesberts, Rafał Miłecki
Cc: linux-wireless, brcm80211-dev-list.pdl, netdev, linux-kernel
On 15-8-2016 11:41, Masami Hiramatsu wrote:
> Change vif_event_lock to spinlock from mutex, since this lock is
> used in wait_event_timeout() via vif_event_equals(). This caused
> a warning report as below.
>
> As far as I can see, this lock protects regions where updating
> structure members, not function calls. Also, since those
> regions are not called from interrupt handlers (of course, it
> was a mutex), spin_lock is used instead of spin_lock_irqsave.
>
> [ 186.678550] ------------[ cut here ]------------
> [ 186.678556] WARNING: CPU: 2 PID: 7140 at /home/mhiramat/ksrc/linux/kernel/sched/core.c:7545 __might_sleep+0x7c/0x80
> [ 186.678560] do not call blocking ops when !TASK_RUNNING; state=2 set at [<ffffffff980d9090>] prepare_to_wait_event+0x60/0x100
> [ 186.678560] Modules linked in: brcmfmac xt_CHECKSUM rfcomm ipt_MASQUERADE nf_nat_masquerade_ipv4 xt_addrtype br_netfilter xt_tcpudp ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 xt_conntrack ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_raw ip6table_security ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_filter ip6_tables iptable_raw iptable_security iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_filter ip_tables x_tables bnep nls_iso8859_1 i2c_designware_platform i2c_designware_core snd_hda_codec_hdmi snd_hda_codec_realtek dcdbas snd_hda_codec_generic snd_hda_intel snd_hda_codec intel_rapl snd_hda_core x86_pkg_temp_thermal intel_p
owerclamp coretemp
> [ 186.678594] snd_pcm crct10dif_pclmul crc32_pclmul aesni_intel aes_x86_64 joydev glue_helper snd_hwdep lrw gf128mul uvcvideo ablk_helper snd_seq_midi cryptd snd_seq_midi_event snd_rawmidi videobuf2_vmalloc videobuf2_memops snd_seq input_leds videobuf2_v4l2 cfg80211 videobuf2_core snd_timer videodev serio_raw btusb snd_seq_device media btrtl rtsx_pci_ms snd mei_me memstick hid_multitouch mei soundcore brcmutil idma64 virt_dma intel_lpss_pci processor_thermal_device intel_soc_dts_iosf hci_uart btbcm btqca btintel bluetooth int3403_thermal dell_smo8800 intel_lpss_acpi intel_lpss int3402_thermal int340x_thermal_zone intel_hid mac_hid int3400_thermal shpchp sparse_keymap acpi_pad acpi_thermal_rel acpi_als kfifo_buf industrialio kvm_intel kvm irqbypass parport_pc ppdev lp parport autofs4 bt
rfs xor raid6_pq
> [ 186.678631] usbhid nouveau ttm i915 rtsx_pci_sdmmc mxm_wmi i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops psmouse drm ahci rtsx_pci nvme nvme_core libahci i2c_hid hid pinctrl_sunrisepoint video wmi pinctrl_intel fjes [last unloaded: brcmfmac]
> [ 186.678646] CPU: 2 PID: 7140 Comm: wpa_supplicant Not tainted 4.8.0-rc1+ #8
> [ 186.678647] Hardware name: Dell Inc. XPS 15 9550/0N7TVV, BIOS 01.02.00 04/07/2016
> [ 186.678648] 0000000000000000 ffff9d8c64b5b900 ffffffff98442f23 ffff9d8c64b5b950
> [ 186.678651] 0000000000000000 ffff9d8c64b5b940 ffffffff9808b22b 00001d790000000d
> [ 186.678653] ffffffff98c75e78 000000000000026c 0000000000000000 ffff9d8c2706d058
> [ 186.678655] Call Trace:
> [ 186.678659] [<ffffffff98442f23>] dump_stack+0x85/0xc2
> [ 186.678666] [<ffffffff9808b22b>] __warn+0xcb/0xf0
> [ 186.678668] [<ffffffff9808b29f>] warn_slowpath_fmt+0x4f/0x60
> [ 186.678671] [<ffffffff980d9090>] ? prepare_to_wait_event+0x60/0x100
> [ 186.678672] [<ffffffff980d9090>] ? prepare_to_wait_event+0x60/0x100
> [ 186.678674] [<ffffffff980b922c>] __might_sleep+0x7c/0x80
> [ 186.678680] [<ffffffff988b0853>] mutex_lock_nested+0x33/0x3b0
> [ 186.678682] [<ffffffff980e5d8d>] ? trace_hardirqs_on+0xd/0x10
> [ 186.678689] [<ffffffffc0c57d2d>] brcmf_cfg80211_wait_vif_event+0xcd/0x130 [brcmfmac]
> [ 186.678691] [<ffffffff980d9190>] ? wake_atomic_t_function+0x60/0x60
> [ 186.678697] [<ffffffffc0c628e9>] brcmf_p2p_del_vif+0xf9/0x220 [brcmfmac]
> [ 186.678702] [<ffffffffc0c57fab>] brcmf_cfg80211_del_iface+0x21b/0x270 [brcmfmac]
> [ 186.678716] [<ffffffffc0b0539e>] nl80211_del_interface+0xfe/0x3a0 [cfg80211]
> [ 186.678718] [<ffffffff987ca335>] genl_family_rcv_msg+0x1b5/0x370
> [ 186.678720] [<ffffffff980e5d8d>] ? trace_hardirqs_on+0xd/0x10
> [ 186.678721] [<ffffffff987ca56d>] genl_rcv_msg+0x7d/0xb0
> [ 186.678722] [<ffffffff987ca4f0>] ? genl_family_rcv_msg+0x370/0x370
> [ 186.678724] [<ffffffff987c9a47>] netlink_rcv_skb+0x97/0xb0
> [ 186.678726] [<ffffffff987ca168>] genl_rcv+0x28/0x40
> [ 186.678727] [<ffffffff987c93c3>] netlink_unicast+0x1d3/0x2f0
> [ 186.678729] [<ffffffff987c933b>] ? netlink_unicast+0x14b/0x2f0
> [ 186.678731] [<ffffffff987c97cb>] netlink_sendmsg+0x2eb/0x3a0
> [ 186.678733] [<ffffffff9876dad8>] sock_sendmsg+0x38/0x50
> [ 186.678734] [<ffffffff9876e4df>] ___sys_sendmsg+0x27f/0x290
> [ 186.678737] [<ffffffff9828b935>] ? mntput_no_expire+0x5/0x3f0
> [ 186.678739] [<ffffffff9828b9be>] ? mntput_no_expire+0x8e/0x3f0
> [ 186.678741] [<ffffffff9828b935>] ? mntput_no_expire+0x5/0x3f0
> [ 186.678743] [<ffffffff9828bd44>] ? mntput+0x24/0x40
> [ 186.678744] [<ffffffff98267830>] ? __fput+0x190/0x200
> [ 186.678746] [<ffffffff9876f125>] __sys_sendmsg+0x45/0x80
> [ 186.678748] [<ffffffff9876f172>] SyS_sendmsg+0x12/0x20
> [ 186.678749] [<ffffffff988b5680>] entry_SYSCALL_64_fastpath+0x23/0xc1
> [ 186.678751] [<ffffffff980e2b8f>] ? trace_hardirqs_off_caller+0x1f/0xc0
> [ 186.678752] ---[ end trace e224d66c5d8408b5 ]---
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 26 ++++++++++----------
> .../broadcom/brcm80211/brcmfmac/cfg80211.h | 2 +-
> 2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index 2628d5e..5db56a7 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -5635,7 +5635,7 @@ static s32 brcmf_notify_vif_event(struct brcmf_if *ifp,
> ifevent->action, ifevent->flags, ifevent->ifidx,
> ifevent->bsscfgidx);
>
> - mutex_lock(&event->vif_event_lock);
> + spin_lock(&event->vif_event_lock);
> event->action = ifevent->action;
> vif = event->vif;
>
> @@ -5643,7 +5643,7 @@ static s32 brcmf_notify_vif_event(struct brcmf_if *ifp,
> case BRCMF_E_IF_ADD:
> /* waiting process may have timed out */
> if (!cfg->vif_event.vif) {
> - mutex_unlock(&event->vif_event_lock);
> + spin_unlock(&event->vif_event_lock);
> return -EBADF;
> }
>
> @@ -5654,24 +5654,24 @@ static s32 brcmf_notify_vif_event(struct brcmf_if *ifp,
> ifp->ndev->ieee80211_ptr = &vif->wdev;
> SET_NETDEV_DEV(ifp->ndev, wiphy_dev(cfg->wiphy));
> }
> - mutex_unlock(&event->vif_event_lock);
> + spin_unlock(&event->vif_event_lock);
> wake_up(&event->vif_wq);
> return 0;
>
> case BRCMF_E_IF_DEL:
> - mutex_unlock(&event->vif_event_lock);
> + spin_unlock(&event->vif_event_lock);
> /* event may not be upon user request */
> if (brcmf_cfg80211_vif_event_armed(cfg))
> wake_up(&event->vif_wq);
> return 0;
>
> case BRCMF_E_IF_CHANGE:
> - mutex_unlock(&event->vif_event_lock);
> + spin_unlock(&event->vif_event_lock);
> wake_up(&event->vif_wq);
> return 0;
>
> default:
> - mutex_unlock(&event->vif_event_lock);
> + spin_unlock(&event->vif_event_lock);
> break;
> }
> return -EINVAL;
> @@ -5792,7 +5792,7 @@ static void wl_deinit_priv(struct brcmf_cfg80211_info *cfg)
> static void init_vif_event(struct brcmf_cfg80211_vif_event *event)
> {
> init_waitqueue_head(&event->vif_wq);
> - mutex_init(&event->vif_event_lock);
> + spin_lock_init(&event->vif_event_lock);
> }
>
> static s32 brcmf_dongle_roam(struct brcmf_if *ifp)
> @@ -6691,9 +6691,9 @@ static inline bool vif_event_equals(struct brcmf_cfg80211_vif_event *event,
> {
> u8 evt_action;
>
> - mutex_lock(&event->vif_event_lock);
> + spin_lock(&event->vif_event_lock);
> evt_action = event->action;
> - mutex_unlock(&event->vif_event_lock);
> + spin_unlock(&event->vif_event_lock);
> return evt_action == action;
> }
>
> @@ -6702,10 +6702,10 @@ void brcmf_cfg80211_arm_vif_event(struct brcmf_cfg80211_info *cfg,
> {
> struct brcmf_cfg80211_vif_event *event = &cfg->vif_event;
>
> - mutex_lock(&event->vif_event_lock);
> + spin_lock(&event->vif_event_lock);
> event->vif = vif;
> event->action = 0;
> - mutex_unlock(&event->vif_event_lock);
> + spin_unlock(&event->vif_event_lock);
> }
>
> bool brcmf_cfg80211_vif_event_armed(struct brcmf_cfg80211_info *cfg)
> @@ -6713,9 +6713,9 @@ bool brcmf_cfg80211_vif_event_armed(struct brcmf_cfg80211_info *cfg)
> struct brcmf_cfg80211_vif_event *event = &cfg->vif_event;
> bool armed;
>
> - mutex_lock(&event->vif_event_lock);
> + spin_lock(&event->vif_event_lock);
> armed = event->vif != NULL;
> - mutex_unlock(&event->vif_event_lock);
> + spin_unlock(&event->vif_event_lock);
>
> return armed;
> }
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> index 7d77f86..8889832 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> @@ -227,7 +227,7 @@ struct escan_info {
> */
> struct brcmf_cfg80211_vif_event {
> wait_queue_head_t vif_wq;
> - struct mutex vif_event_lock;
> + spinlock_t vif_event_lock;
> u8 action;
> struct brcmf_cfg80211_vif *vif;
> };
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUGFIX PATCH 1/2] brcmfmac: Check rtnl_lock is locked when removing interface
2016-08-15 21:44 ` Arend Van Spriel
(?)
@ 2016-08-16 2:27 ` Masami Hiramatsu
-1 siblings, 0 replies; 14+ messages in thread
From: Masami Hiramatsu @ 2016-08-16 2:27 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Rafał Miłecki, Kalle Valo, Masami Hiramatsu,
Franky Lin, Hante Meuleman, Pieter-Paul Giesberts,
linux-wireless,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
Network Development, Linux Kernel Mailing List
On Mon, 15 Aug 2016 23:44:05 +0200
Arend Van Spriel <arend.vanspriel@broadcom.com> wrote:
>
>
> On 15-8-2016 13:52, Rafał Miłecki wrote:
> > On 15 August 2016 at 12:57, Kalle Valo <kvalo@codeaurora.org> wrote:
> >> Rafał Miłecki <zajec5@gmail.com> writes:
> >>
> >>>> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> >>>
> >>> Fixes: a63b09872c1d ("brcmfmac: delete interface directly in code that sent fw request")
> >>> Acked-by: Rafał Miłecki <rafal@milecki.pl>
> >>>
> >>> Kalle: I'm acking this as bugfix for 4.8 release.
> >>
> >> Ok. I'll wait few days for more comments before I apply this.
Thanks!
> >
> > Sure.
> >
> >
> >> (I assume you are talking only about patch 1)
> >
> > Yes, I'll leave mutex vs. spinlock to the experts :)
>
> Don't know who the experts are. Surely not me :-p
>
> I made an uneducated design decision using a mutex for this. The
> reasoning for using a regular spinlock make sense. So I will go and ack
> that patch.
As far as I can see, that change is very local and
at least my environment it works well :)
Regards,
>
> Regards,
> Arend
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUGFIX, 1/2] brcmfmac: Check rtnl_lock is locked when removing interface
2016-08-15 9:40 ` [BUGFIX PATCH 1/2] brcmfmac: Check rtnl_lock is locked when removing interface Masami Hiramatsu
2016-08-15 10:41 ` Rafał Miłecki
@ 2016-08-24 13:14 ` Kalle Valo
1 sibling, 0 replies; 14+ messages in thread
From: Kalle Valo @ 2016-08-24 13:14 UTC (permalink / raw)
To: mhiramat
Cc: Arend van Spriel, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, Rafał Miłecki, linux-wireless,
brcm80211-dev-list.pdl, netdev, linux-kernel
mhiramat@kernel.org wrote:
> Check rtnl_lock is locked in brcmf_p2p_ifp_removed() by passing
> rtnl_locked flag. Actually the caller brcmf_del_if() checks whether
> the rtnl_lock is locked, but doesn't pass it to brcmf_p2p_ifp_removed().
>
> Without this fix, wpa_supplicant goes softlockup with rtnl_lock
> holding (this means all other process using netlink are locked up too)
>
> e.g.
> [ 4495.876627] INFO: task wpa_supplicant:7307 blocked for more than 10 seconds.
> [ 4495.876632] Tainted: G W 4.8.0-rc1+ #8
> [ 4495.876635] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 4495.876638] wpa_supplicant D ffff974c647b39a0 0 7307 1 0x00000000
> [ 4495.876644] ffff974c647b39a0 0000000000000000 ffff974c00000000 ffff974c7dc59c58
> [ 4495.876651] ffff974c6b7417c0 ffff974c645017c0 ffff974c647b4000 ffffffff86f16c08
> [ 4495.876657] ffff974c645017c0 0000000000000246 00000000ffffffff ffff974c647b39b8
> [ 4495.876664] Call Trace:
> [ 4495.876671] [<ffffffff868aeccc>] schedule+0x3c/0x90
> [ 4495.876676] [<ffffffff868af065>] schedule_preempt_disabled+0x15/0x20
> [ 4495.876682] [<ffffffff868b0996>] mutex_lock_nested+0x176/0x3b0
> [ 4495.876686] [<ffffffff867a2067>] ? rtnl_lock+0x17/0x20
> [ 4495.876690] [<ffffffff867a2067>] rtnl_lock+0x17/0x20
> [ 4495.876720] [<ffffffffc0ae9a5d>] brcmf_p2p_ifp_removed+0x4d/0x70 [brcmfmac]
> [ 4495.876741] [<ffffffffc0aebde6>] brcmf_remove_interface+0x196/0x1b0 [brcmfmac]
> [ 4495.876760] [<ffffffffc0ae9901>] brcmf_p2p_del_vif+0x111/0x220 [brcmfmac]
> [ 4495.876777] [<ffffffffc0adefab>] brcmf_cfg80211_del_iface+0x21b/0x270 [brcmfmac]
> [ 4495.876820] [<ffffffffc097b39e>] nl80211_del_interface+0xfe/0x3a0 [cfg80211]
> [ 4495.876825] [<ffffffff867ca335>] genl_family_rcv_msg+0x1b5/0x370
> [ 4495.876832] [<ffffffff860e5d8d>] ? trace_hardirqs_on+0xd/0x10
> [ 4495.876836] [<ffffffff867ca56d>] genl_rcv_msg+0x7d/0xb0
> [ 4495.876839] [<ffffffff867ca4f0>] ? genl_family_rcv_msg+0x370/0x370
> [ 4495.876846] [<ffffffff867c9a47>] netlink_rcv_skb+0x97/0xb0
> [ 4495.876849] [<ffffffff867ca168>] genl_rcv+0x28/0x40
> [ 4495.876854] [<ffffffff867c93c3>] netlink_unicast+0x1d3/0x2f0
> [ 4495.876860] [<ffffffff867c933b>] ? netlink_unicast+0x14b/0x2f0
> [ 4495.876866] [<ffffffff867c97cb>] netlink_sendmsg+0x2eb/0x3a0
> [ 4495.876870] [<ffffffff8676dad8>] sock_sendmsg+0x38/0x50
> [ 4495.876874] [<ffffffff8676e4df>] ___sys_sendmsg+0x27f/0x290
> [ 4495.876882] [<ffffffff8628b935>] ? mntput_no_expire+0x5/0x3f0
> [ 4495.876888] [<ffffffff8628b9be>] ? mntput_no_expire+0x8e/0x3f0
> [ 4495.876894] [<ffffffff8628b935>] ? mntput_no_expire+0x5/0x3f0
> [ 4495.876899] [<ffffffff8628bd44>] ? mntput+0x24/0x40
> [ 4495.876904] [<ffffffff86267830>] ? __fput+0x190/0x200
> [ 4495.876909] [<ffffffff8676f125>] __sys_sendmsg+0x45/0x80
> [ 4495.876914] [<ffffffff8676f172>] SyS_sendmsg+0x12/0x20
> [ 4495.876918] [<ffffffff868b5680>] entry_SYSCALL_64_fastpath+0x23/0xc1
> [ 4495.876924] [<ffffffff860e2b8f>] ? trace_hardirqs_off_caller+0x1f/0xc0
>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> Acked-by: Rafał Miłecki <rafal@milecki.pl>
Thanks, 2 patches applied to wireless-drivers.git:
15dacf880e49 brcmfmac: Check rtnl_lock is locked when removing interface
b64abcb7dae6 brcmfmac: Change vif_event_lock to spinlock
--
Sent by pwcli
https://patchwork.kernel.org/patch/9280681/
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-08-24 13:14 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-15 9:40 [BUGFIX PATCH 0/2] Bugfixes for brcmfmac Masami Hiramatsu
2016-08-15 9:40 ` [BUGFIX PATCH 1/2] brcmfmac: Check rtnl_lock is locked when removing interface Masami Hiramatsu
2016-08-15 10:41 ` Rafał Miłecki
2016-08-15 10:57 ` Kalle Valo
2016-08-15 11:52 ` Rafał Miłecki
2016-08-15 11:52 ` Rafał Miłecki
2016-08-15 21:44 ` Arend Van Spriel
2016-08-15 21:44 ` Arend Van Spriel
2016-08-16 2:27 ` Masami Hiramatsu
2016-08-24 13:14 ` [BUGFIX, " Kalle Valo
2016-08-15 9:41 ` [BUGFIX PATCH 2/2] brcmfmac: Change vif_event_lock to spinlock Masami Hiramatsu
2016-08-15 9:41 ` Masami Hiramatsu
2016-08-15 21:46 ` Arend Van Spriel
2016-08-15 21:46 ` Arend Van Spriel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.