Re: [PATCH 00/11] SDIO support for ath10k
Hi Erik, We will work to have this support mainlined as soon as possible. Would appreciate your support in making sure that the patches do not affect the USB high latency path. On 02-10-2017 14:32, Erik Stromdahl wrote: Hi Alagu, It is great to see that we are finally about have fully working mainline support for QCA9377 SDIO chipsets! Great job! On 2017-09-30 19:37, silexcom...@gmail.com wrote: From: Alagu Sankar This patchset, generated against master-pending branch, enables a fully functional SDIO interface driver for ath10k. Patches have been verified on QCA9377-3 WB396 and Silex's SX-SDCAC reference cards with Station, Access Point and P2P modes. The driver is verified with the firmware WLAN.TF.1.1.1-00061-QCATFSWPZ-1 with the board data from respective SDIO card vendors. Receive performance matches the QCA reference driver when used with SDIO3.0 enabled platforms. iperf tests indicate a downlink UDP of 275Mbit/s and TCP of 150Mbit/s Can you share any scripts etc. (wrapping hostapd and wpa_supplicant stuff) or provide some more info about you test setup? I am not using any specific scripts for this. The standard ones from the ath10k configuration page https://wireless.wiki.kernel.org/en/users/drivers/ath10k/configuration works just fine with the NL80211 path. I made a quick socat based test on an old laptop (I don't think it has SDIO 3.0 support) and I did unfortunately not get the same figures as you did :( If it is SDIO v1.x, then the max bus speed is only 100Mbit/s. With v2.x it is 200Mbit/s and 3.x it is 832 Mbit/s. Throughput primarily depends on it. We used i.MX6 SoloX Sabre SDB platform which supports SDIO3.x and could see the maximum performance. This patchset differs from the previous high latency patches, specific to SDIO. HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET is enabled for HI_ACS. This instructs the firmware to use HTT_T2H_MSG_TYPE_TX_COMPL_IND for outgoing packets. Without this flag, the management frames are not sent out by the firmware. Possibility of management frames being sent via WMI and data frames through the reduced Tx completion needs to be probed further. Ah, so that explains why I couldn't see any messages in the air. Further improvements can be done on the transmit path by implementing packet bundle. Scatter Gather is another area of improvement for both Transmit and Receive, but may not work on all platforms Known issues: Surprise removal of the card, when the device is in connected state, delays sdio function remove due to delayed WMI command failures. Existing ath10k framework can not differentiate between a kernel module removal and the surprise removal of teh card. Alagu Sankar (11): ath10k_sdio: sdio htt data transfer fixes ath10k_sdio: wb396 reference card fix ath10k_sdio: DMA bounce buffers for read write ath10k_sdio: reduce transmit msdu count ath10k_sdio: use clean packet headers ath10k_sdio: high latency fixes for beacon buffer ath10k_sdio: fix rssi indication ath10k_sdio: common read write ath10k_sdio: virtual scatter gather for receive ath10k_sdio: enable firmware crash dump ath10k_sdio: hif start once addition drivers/net/wireless/ath/ath10k/core.c| 35 ++- drivers/net/wireless/ath/ath10k/debug.c | 3 + drivers/net/wireless/ath/ath10k/htc.c | 4 +- drivers/net/wireless/ath/ath10k/htc.h | 1 + drivers/net/wireless/ath/ath10k/htt_rx.c | 19 +- drivers/net/wireless/ath/ath10k/htt_tx.c | 24 +- drivers/net/wireless/ath/ath10k/hw.c | 2 + drivers/net/wireless/ath/ath10k/hw.h | 1 + drivers/net/wireless/ath/ath10k/mac.c | 31 ++- drivers/net/wireless/ath/ath10k/sdio.c| 398 ++ drivers/net/wireless/ath/ath10k/sdio.h| 10 +- drivers/net/wireless/ath/ath10k/wmi-tlv.c | 2 +- 12 files changed, 403 insertions(+), 127 deletions(-)
Re: iwlwifi: ToF usage
Hi Joel, Unfortunately we don't have full support for ToF on the mainline kernel yet. But you can try to use one of our Core releases (which is a backports-based tree) that you can find here: You could try the release/Core30 branch, for example. -- Cheers, Luca. On Thu, 2017-09-21 at 21:35 +0200, Joel Bjurström wrote: > Including some people I've seen mentioned in ToF-related e-mails... > > This is what I've tried: > > > -- Responder -- > > * hostapd running, hostapd.conf: > driver=nl80211 > interface=wlan1 > hw_mode=g > channel=1 > wmm_enabled=1 > ssid=tof_test > ftm_responder=1 > ftm_initiator=1 > > $ cd /sys/kernel/debug/iwlwifi/:06:00.0/iwlmvm/netdev:wlan1 > $ echo channel_num=1 > tof_responder_params > $ echo bssid=bb:bb:bb:bb:bb:bb > tof_responder_params > $ echo rate=1 > tof_responder_params > $ echo ftm_per_burst=5 > tof_responder_params > > $ echo send_responder_cfg=1 > tof_responder_params > > > -- Initiator -- > > $ cd /sys/kernel/debug/iwlwifi/:06:00.0/iwlmvm/netdev:wlan1 > $ echo send_tof_cfg=1 > tof_enable > $ echo 'num_of_ap=1' > tof_range_request > $ echo 'ap=0 1 0 0 bb:bb:bb:bb:bb:bb 0 10 0 5 5 0 0 0 0 -40' \ > > tof_range_request > > $ echo 'send_range_request=1' > tof_range_request > > $ cat tof_range_response > request_id = 0 > status = 2 > last_in_batch = 1 > num_of_aps = 0 > > > > (bb:bb:bb:bb:bb:bb is the responder's BSSID) > > Am I even close? Fumbling in the dark here. > > The 'status = 2' from tof_range_response comes directly from the FW, > AFAICT. What do different values mean? > > Cheers, > Joel > > > On 09/14/2017 09:44 PM, Joel B wrote: > > Hi, > > > > Starting to play around with the FTM/ToF support in iwlwifi, but > > documentation is (understandably) scarce at this point. Using a > > pair of > > 8260:s I had lying around. > > > > Can someone give me some hints on how to work the debugfs API for > > a > > successful measurement? > > > > I've set up one card as AP with hostapd (with ftm_responder=1 and > > ftm_initiator=1 in hostapd.conf), the other as a STA. > > > > From the STA, I've played with 'tof_range_request' in debugfs. > > Writing > > send_range_request=1, which it doesn't choke on or anything, but > > nothing > > seems to happen. Guess I need to set things up a bit first, but > > how? > > > > If it's possible to get this to work at all at this stage, some > > hints on > > how to do it would be great. > > > > > > Thanks, > > Joel
Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned
On Tue, Oct 03, 2017 at 07:33:08PM -0300, Marcelo Ricardo Leitner wrote: > On Tue, Oct 03, 2017 at 10:25:22AM +0800, Jia-Ju Bai wrote: > > The SCTP program may sleep under a spinlock, and the function call path is: > > sctp_generate_t3_rtx_event (acquire the spinlock) > > sctp_do_sm > > sctp_side_effects > > sctp_cmd_interpreter > > sctp_make_init_ack > > sctp_pack_cookie > > crypto_shash_setkey > > shash_setkey_unaligned > > kmalloc(GFP_KERNEL) > > Are you sure this can happen? > The host is not supposed to store any information when replying to an > INIT packet (which generated the INIT_ACK listed above). That said, > it's weird to see the timer function triggering so. > > Checking now, that code is dead actually: > $ git grep -A 2 SCTP_CMD_GEN_INIT_ACK > sm_sideeffect.c:case SCTP_CMD_GEN_INIT_ACK: > sm_sideeffect.c-/* Generate an INIT ACK chunk. > */ > sm_sideeffect.c-new_obj = > sctp_make_init_ack(asoc, chunk, GFP_ATOMIC, > > Nobody is triggering a call to sctp_cmd_interpreter with > SCTP_CMD_GEN_INIT_ACK command, which would generate the callstack > above. Nevertheless, the issue is real through other call paths. Thanks, Marcelo
Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned
On Tue, Oct 03, 2017 at 01:26:43PM +0800, Herbert Xu wrote: > On Mon, Oct 02, 2017 at 09:18:24PM -0700, Andy Lutomirski wrote: > > > On Oct 2, 2017, at 7:25 PM, Jia-Ju Bai wrote: > > > > > > The SCTP program may sleep under a spinlock, and the function call path > > > is: > > > sctp_generate_t3_rtx_event (acquire the spinlock) > > > sctp_do_sm > > >sctp_side_effects > > > sctp_cmd_interpreter > > >sctp_make_init_ack > > > sctp_pack_cookie > > >crypto_shash_setkey > > > shash_setkey_unaligned > > >kmalloc(GFP_KERNEL) > > > > I'm going to go out on a limb here: why on Earth is out crypto API so > > full of indirection that we allocate memory at all here? > > The crypto API operates on a one key per-tfm basis. So normally > tfm allocation and key setting is done once only and not done on > the data path. > > I have looked at the SCTP code and it appears to fit this paradigm. > That is, we should be able to allocate the tfm and set the key when > the key is actually generated via get_random_bytes, rather than every > time the key is used which is not only a waste but as you see runs > into API issues. Fair point, but > > Usually if you're invoking setkey from a non-sleeping code-path > you're probably doing something wrong. Usually but not always. There are 3 calls to that function on SCTP code: - pack a cookie, which is sent on an INIT_ACK packet to the client - unpack the cookie above, after it is sent back by the client on a COOKIE_ECHO packet - send a chunk authenticated by a hash the first two happen during softirq processing, while processing a packet that was received. As I explained on the other email, SCTP code is not supposed to store any information about the peer between the 1st and the 2nd moments above, to be less vulnerable to DoS attacks (it's planned so by the RFC), thus why it uses the cookie. The 3rd one we probably can improve, but I don't think we can do much about the 2 first ones from the SCTP side. Note on sctp_sf_do_5_1B_init() how sctp_make_init_ack() is explicitly called with GFP_ATOMIC, and also on sctp_sf_do_unexpected_init(). Though we can't propagate that to crypto_shash_setkey. Ideas? Thanks, Marcelo > > As someone else noted recently, there is no single forum for > reviewing code that uses the crypto API so buggy code like this > is not surprising. > > > We're synchronously computing a hash of a small amount of data using > > either HMAC-SHA1 or HMAC-SHA256 (determined at runtime) if I read it > > right. There's a sane way to do this that doesn't need kmalloc, > > alloca, or fancy indirection. And then there's crypto_shash_xyz(). > > There are some legitimate cases where you want to use a different > key for every hashing operation. But so far these are uses have > been very few so there has been no need to provide an API for them. > > Cheers, > -- > Email: Herbert Xu > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt > -- > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned
On Tue, Oct 03, 2017 at 10:25:22AM +0800, Jia-Ju Bai wrote: > The SCTP program may sleep under a spinlock, and the function call path is: > sctp_generate_t3_rtx_event (acquire the spinlock) > sctp_do_sm > sctp_side_effects > sctp_cmd_interpreter > sctp_make_init_ack > sctp_pack_cookie > crypto_shash_setkey > shash_setkey_unaligned > kmalloc(GFP_KERNEL) Are you sure this can happen? The host is not supposed to store any information when replying to an INIT packet (which generated the INIT_ACK listed above). That said, it's weird to see the timer function triggering so. Checking now, that code is dead actually: $ git grep -A 2 SCTP_CMD_GEN_INIT_ACK sm_sideeffect.c:case SCTP_CMD_GEN_INIT_ACK: sm_sideeffect.c-/* Generate an INIT ACK chunk. */ sm_sideeffect.c-new_obj = sctp_make_init_ack(asoc, chunk, GFP_ATOMIC, Nobody is triggering a call to sctp_cmd_interpreter with SCTP_CMD_GEN_INIT_ACK command, which would generate the callstack above. Marcelo
[PATCH v2] ath10k: Retry pci probe on failure.
From: Ben Greear
This works around a problem we see when sometimes the wifi NIC does
not respond the first time. This seems to happen especially often on
some of the 9984 NICs in mid-range platforms.
Signed-off-by: Ben Greear
---
v2: Change to mdelay instead of udelay to fix compile issue on ARM.
drivers/net/wireless/ath/ath10k/pci.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/pci.c
b/drivers/net/wireless/ath/ath10k/pci.c
index 77beb13..0861f7f 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -3487,8 +3487,8 @@ static const struct ath10k_bus_ops ath10k_pci_bus_ops = {
.get_num_banks = ath10k_pci_get_num_banks,
};
-static int ath10k_pci_probe(struct pci_dev *pdev,
- const struct pci_device_id *pci_dev)
+static int __ath10k_pci_probe(struct pci_dev *pdev,
+ const struct pci_device_id *pci_dev)
{
int ret = 0;
struct ath10k *ar;
@@ -3672,6 +3672,22 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
return ret;
}
+static int ath10k_pci_probe(struct pci_dev *pdev,
+ const struct pci_device_id *pci_dev)
+{
+ int cnt = 0;
+ int rv;
+ do {
+ rv = __ath10k_pci_probe(pdev, pci_dev);
+ if (rv == 0)
+ return rv;
+ pr_err("ath10k: failed to probe PCI : %d, retry-count: %d\n",
rv, cnt);
+ mdelay(10); /* let the ath10k firmware gerbil take a small
break */
+ } while (cnt++ < 10);
+ return rv;
+}
+
+
static void ath10k_pci_remove(struct pci_dev *pdev)
{
struct ath10k *ar = pci_get_drvdata(pdev);
--
2.4.11
[PATCH] ath10k: Store coverage-class in case firmware is not booted.
From: Ben Greear
This way, we can apply the values when the NIC does come up.
Signed-off-by: Ben Greear
---
drivers/net/wireless/ath/ath10k/hw.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/hw.c
b/drivers/net/wireless/ath/ath10k/hw.c
index afb0c01..31c0589 100644
--- a/drivers/net/wireless/ath/ath10k/hw.c
+++ b/drivers/net/wireless/ath/ath10k/hw.c
@@ -454,8 +454,13 @@ static void ath10k_hw_qca988x_set_coverage_class(struct
ath10k *ar,
/* Only modify registers if the core is started. */
if ((ar->state != ATH10K_STATE_ON) &&
- (ar->state != ATH10K_STATE_RESTARTED))
+ (ar->state != ATH10K_STATE_RESTARTED)) {
+ spin_lock_bh(&ar->data_lock);
+ /* Store config value for when radio boots up */
+ ar->fw_coverage.coverage_class = value;
+ spin_unlock_bh(&ar->data_lock);
goto unlock;
+ }
/* Retrieve the current values of the two registers that need to be
* adjusted.
@@ -487,7 +492,7 @@ static void ath10k_hw_qca988x_set_coverage_class(struct
ath10k *ar,
ar->fw_coverage.reg_ack_cts_timeout_orig = timeout_reg;
ar->fw_coverage.reg_phyclk = phyclk_reg;
- /* Calculat new value based on the (original) firmware calculation. */
+ /* Calculate new value based on the (original) firmware calculation. */
slottime_reg = ar->fw_coverage.reg_slottime_orig;
timeout_reg = ar->fw_coverage.reg_ack_cts_timeout_orig;
--
2.4.11
Re: [PATCH] PCI MSI: allow alignment restrictions on vector allocation
On Tue, 3 Oct 2017, Bjorn Helgaas wrote: > On Tue, Oct 03, 2017 at 11:07:58PM +0200, Thomas Gleixner wrote: > > On Mon, 2 Oct 2017, Thomas Gleixner wrote: > > > On Mon, 2 Oct 2017, Thomas Gleixner wrote: > > > > On Mon, 2 Oct 2017, Daniel Drake wrote: > > > > 2) The affinity setting of straight MSI interrupts (w/o remapping) on > > > > x86 > > > > requires to make the affinity change from the interrupt context of > > > > the > > > > current active vector in order not to lose interrupts or worst case > > > > getting into a stale state. > > > > > > > > That works for single vectors, but trying to move all vectors in > > > > one > > > > go is more or less impossible, as there is no reliable way to > > > > determine that none of the other vectors is on flight. > > > > > > > > There might be some 'workarounds' for that, but I rather avoid that > > > > unless we get an official documented one from Intel/AMD. > > > > > > Thinking more about it. That might be actually a non issue for MSI, but we > > > have that modus operandi in the current code and we need to address that > > > first before even thinking about multi MSI support. > > > > But even if its possible, it's very debatable whether it's worth the effort > > when this driver just can use the legacy INTx.and be done with it. > > Daniel said "Legacy interrupts do not work on that module, so MSI > support is required," so I assume he means INTx doesn't work. Maybe Hmm, I missed that detail obviously. > the driver could poll? I don't know how much slower that would be, > but at least it would penalize the broken device, not everybody. That would definitely be prefered. Thanks, tglx
Re: [PATCH] PCI MSI: allow alignment restrictions on vector allocation
On Tue, Oct 03, 2017 at 11:07:58PM +0200, Thomas Gleixner wrote: > On Mon, 2 Oct 2017, Thomas Gleixner wrote: > > On Mon, 2 Oct 2017, Thomas Gleixner wrote: > > > On Mon, 2 Oct 2017, Daniel Drake wrote: > > > 2) The affinity setting of straight MSI interrupts (w/o remapping) on > > > x86 > > > requires to make the affinity change from the interrupt context of > > > the > > > current active vector in order not to lose interrupts or worst case > > > getting into a stale state. > > > > > > That works for single vectors, but trying to move all vectors in one > > > go is more or less impossible, as there is no reliable way to > > > determine that none of the other vectors is on flight. > > > > > > There might be some 'workarounds' for that, but I rather avoid that > > > unless we get an official documented one from Intel/AMD. > > > > Thinking more about it. That might be actually a non issue for MSI, but we > > have that modus operandi in the current code and we need to address that > > first before even thinking about multi MSI support. > > But even if its possible, it's very debatable whether it's worth the effort > when this driver just can use the legacy INTx.and be done with it. Daniel said "Legacy interrupts do not work on that module, so MSI support is required," so I assume he means INTx doesn't work. Maybe the driver could poll? I don't know how much slower that would be, but at least it would penalize the broken device, not everybody. Bjorn
Re: [PATCH] PCI MSI: allow alignment restrictions on vector allocation
On Mon, 2 Oct 2017, Thomas Gleixner wrote: > On Mon, 2 Oct 2017, Thomas Gleixner wrote: > > On Mon, 2 Oct 2017, Daniel Drake wrote: > > 2) The affinity setting of straight MSI interrupts (w/o remapping) on x86 > > requires to make the affinity change from the interrupt context of the > > current active vector in order not to lose interrupts or worst case > > getting into a stale state. > > > > That works for single vectors, but trying to move all vectors in one > > go is more or less impossible, as there is no reliable way to > > determine that none of the other vectors is on flight. > > > > There might be some 'workarounds' for that, but I rather avoid that > > unless we get an official documented one from Intel/AMD. > > Thinking more about it. That might be actually a non issue for MSI, but we > have that modus operandi in the current code and we need to address that > first before even thinking about multi MSI support. But even if its possible, it's very debatable whether it's worth the effort when this driver just can use the legacy INTx.and be done with it. Thanks, tglx
Re: lockdep splat on 4.13.3 (plus hacks)
On 10/03/2017 11:17 AM, Ben Greear wrote:
We are seeing deadlocks related to wifi in our 4.13.3+ kernels,
so I enabled lockdep and immediately saw this. Anyone know if this
is a known issue? Otherwise, I guess it could be related to some local
patch I have added...
I think I found the fix in the stable queue, so I guess it will be in
4.13.5 when that is out...
Will continue testing...
Thanks,
Ben
[ 476.172823]
[ 476.176863] WARNING: possible recursive locking detected
[ 476.180895] 4.13.3+ #1 Not tainted
[ 476.183025]
[ 476.187053] kworker/u8:2/281 is trying to acquire lock:
[ 476.190993] (&sta->ampdu_mlme.mtx){+.+...}, at: []
__ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
[ 476.201004]
but task is already holding lock:
[ 476.204270] (&sta->ampdu_mlme.mtx){+.+...}, at: []
ieee80211_ba_session_work+0x46/0x2b0 [mac80211]
[ 476.213645]
other info that might help us debug this:
[ 476.217587] Possible unsafe locking scenario:
[ 476.220930]CPU0
[ 476.222082]
[ 476.223236] lock(&sta->ampdu_mlme.mtx);
[ 476.225957] lock(&sta->ampdu_mlme.mtx);
[ 476.228673]
*** DEADLOCK ***
[ 476.230689] May be due to missing lock nesting notation
[ 476.234879] 3 locks held by kworker/u8:2/281:
[ 476.237941] #0: ("%s"wiphy_name(local->hw.wiphy)){.+}, at:
[] process_one_work+0x14f/0x6a0
[ 476.247033] #1: ((&sta->ampdu_mlme.work)){+.+...}, at:
[] process_one_work+0x14f/0x6a0
[ 476.255393] #2: (&sta->ampdu_mlme.mtx){+.+...}, at: []
ieee80211_ba_session_work+0x46/0x2b0 [mac80211]
[ 476.265170]
stack backtrace:
[ 476.266928] CPU: 0 PID: 281 Comm: kworker/u8:2 Not tainted 4.13.3+ #1
[ 476.272073] Hardware name: _ _/ , BIOS 5.11 08/26/2016
[ 476.275927] Workqueue: phy1 ieee80211_ba_session_work [mac80211]
[ 476.280640] Call Trace:
[ 476.281792] dump_stack+0x85/0xc7
[ 476.283811] __lock_acquire+0x14ba/0x1520
[ 476.286526] ? __save_stack_trace+0x6e/0xd0
[ 476.289412] ? ret_from_fork+0x2a/0x40
[ 476.291867] lock_acquire+0xac/0x200
[ 476.294145] ? lock_acquire+0xac/0x200
[ 476.296610] ? __ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
[ 476.301766] ? __ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
[ 476.306910] __mutex_lock+0x69/0x930
[ 476.309194] ? __ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
[ 476.314343] ? rcu_read_lock_sched_held+0x6d/0x80
[ 476.317766] ? __sdata_dbg+0x14a/0x1a0 [mac80211]
[ 476.321181] mutex_lock_nested+0x16/0x20
[ 476.323810] ? mutex_lock_nested+0x16/0x20
[ 476.326626] __ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
[ 476.331627] ieee80211_ba_session_work+0x157/0x2b0 [mac80211]
[ 476.336078] ? process_one_work+0x14f/0x6a0
[ 476.338985] process_one_work+0x1ce/0x6a0
[ 476.341696] worker_thread+0x46/0x400
[ 476.344061] kthread+0x10f/0x150
[ 476.346001] ? process_one_work+0x6a0/0x6a0
[ 476.348886] ? kthread_create_on_node+0x40/0x40
[ 476.352122] ret_from_fork+0x2a/0x40
Thanks,
Ben
--
Ben Greear
Candela Technologies Inc http://www.candelatech.com
lockdep splat on 4.13.3 (plus hacks)
We are seeing deadlocks related to wifi in our 4.13.3+ kernels,
so I enabled lockdep and immediately saw this. Anyone know if this
is a known issue? Otherwise, I guess it could be related to some local
patch I have added...
[ 476.172823]
[ 476.176863] WARNING: possible recursive locking detected
[ 476.180895] 4.13.3+ #1 Not tainted
[ 476.183025]
[ 476.187053] kworker/u8:2/281 is trying to acquire lock:
[ 476.190993] (&sta->ampdu_mlme.mtx){+.+...}, at: []
__ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
[ 476.201004]
but task is already holding lock:
[ 476.204270] (&sta->ampdu_mlme.mtx){+.+...}, at: []
ieee80211_ba_session_work+0x46/0x2b0 [mac80211]
[ 476.213645]
other info that might help us debug this:
[ 476.217587] Possible unsafe locking scenario:
[ 476.220930]CPU0
[ 476.222082]
[ 476.223236] lock(&sta->ampdu_mlme.mtx);
[ 476.225957] lock(&sta->ampdu_mlme.mtx);
[ 476.228673]
*** DEADLOCK ***
[ 476.230689] May be due to missing lock nesting notation
[ 476.234879] 3 locks held by kworker/u8:2/281:
[ 476.237941] #0: ("%s"wiphy_name(local->hw.wiphy)){.+}, at:
[] process_one_work+0x14f/0x6a0
[ 476.247033] #1: ((&sta->ampdu_mlme.work)){+.+...}, at:
[] process_one_work+0x14f/0x6a0
[ 476.255393] #2: (&sta->ampdu_mlme.mtx){+.+...}, at: []
ieee80211_ba_session_work+0x46/0x2b0 [mac80211]
[ 476.265170]
stack backtrace:
[ 476.266928] CPU: 0 PID: 281 Comm: kworker/u8:2 Not tainted 4.13.3+ #1
[ 476.272073] Hardware name: _ _/ , BIOS 5.11 08/26/2016
[ 476.275927] Workqueue: phy1 ieee80211_ba_session_work [mac80211]
[ 476.280640] Call Trace:
[ 476.281792] dump_stack+0x85/0xc7
[ 476.283811] __lock_acquire+0x14ba/0x1520
[ 476.286526] ? __save_stack_trace+0x6e/0xd0
[ 476.289412] ? ret_from_fork+0x2a/0x40
[ 476.291867] lock_acquire+0xac/0x200
[ 476.294145] ? lock_acquire+0xac/0x200
[ 476.296610] ? __ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
[ 476.301766] ? __ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
[ 476.306910] __mutex_lock+0x69/0x930
[ 476.309194] ? __ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
[ 476.314343] ? rcu_read_lock_sched_held+0x6d/0x80
[ 476.317766] ? __sdata_dbg+0x14a/0x1a0 [mac80211]
[ 476.321181] mutex_lock_nested+0x16/0x20
[ 476.323810] ? mutex_lock_nested+0x16/0x20
[ 476.326626] __ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
[ 476.331627] ieee80211_ba_session_work+0x157/0x2b0 [mac80211]
[ 476.336078] ? process_one_work+0x14f/0x6a0
[ 476.338985] process_one_work+0x1ce/0x6a0
[ 476.341696] worker_thread+0x46/0x400
[ 476.344061] kthread+0x10f/0x150
[ 476.346001] ? process_one_work+0x6a0/0x6a0
[ 476.348886] ? kthread_create_on_node+0x40/0x40
[ 476.352122] ret_from_fork+0x2a/0x40
Thanks,
Ben
--
Ben Greear
Candela Technologies Inc http://www.candelatech.com
[PATCH v3 0/2] Allwinner XR819 SDIO Wi-Fi DT binding and OPi Zero XR819 IRQ
The Allwinner XR819 SDIO Wi-Fi chip supports an out-of-band interrupt line, and the in-band interrupt is also supported. However the current out-of-tree driver uses the out-of-band interrupt by default. This patchset adds the device tree binding for the chip as well as the out-of-band interrupt, then adds the interrupt to the device tree of Orange Pi Zero. Icenowy Zheng (1): dt-bindings: add device tree binding for Allwinner XR819 SDIO Wi-Fi Sergey Matyukevich (1): ARM: sun8i: h2+: specify wifi interrupts for Orange Pi Zero .../bindings/net/wireless/allwinner,xr819.txt | 38 ++ arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts | 3 ++ 2 files changed, 41 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt -- 2.13.5
[PATCH v3 1/2] dt-bindings: add device tree binding for Allwinner XR819 SDIO Wi-Fi
Allwinner XR819 is a SDIO Wi-Fi chip, which has the functionality to use
an out-of-band interrupt pin instead of SDIO in-band interrupt.
Add the device tree binding of this chip, in order to make it possible
to add this interrupt pin to device trees.
Signed-off-by: Icenowy Zheng
Acked-by: Rob Herring
---
Changes in v3:
- Renames the node name.
- Adds ACK from Rob.
Changes in v2:
- Removed status property in example.
- Added required property reg.
.../bindings/net/wireless/allwinner,xr819.txt | 38 ++
1 file changed, 38 insertions(+)
create mode 100644
Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt
diff --git a/Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt
b/Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt
new file mode 100644
index ..7ae40441e343
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt
@@ -0,0 +1,38 @@
+Allwinner XRadio wireless SDIO devices
+
+This node provides properties for controlling the XRadio wireless device. The
+node is expected to be specified as a child node to the SDIO controller that
+connects the device to the system.
+
+Required properties:
+
+ - reg : The SDIO function number, see "Use of function subnodes" in
+ ../../mmc/mmc.txt.
+ - compatible : Should be "allwinner,xr819".
+
+Optional properties:
+ - interrupt-parent : the phandle for the interrupt controller to which the
+ device interrupts are connected.
+ - interrupts : specifies attributes for the out-of-band interrupt (host-wake).
+ When not specified the device will use in-band SDIO interrupts.
+
+Example:
+
+mmc1: mmc@01c1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&mmc1_pins_a>;
+ vmmc-supply = <®_vcc_wifi>;
+ mmc-pwrseq = <&wifi_pwrseq>;
+ bus-width = <4>;
+ non-removable;
+
+ xr819: wifi@1 {
+ reg = <1>;
+ compatible = "allwinner,xr819";
+ interrupt-parent = <&pio>;
+ interrupts = <6 10 IRQ_TYPE_EDGE_RISING>;
+ };
+};
--
2.13.5
[PATCH v3 2/2] ARM: sun8i: h2+: specify wifi interrupts for Orange Pi Zero
From: Sergey Matyukevich
The Orange Pi Zero board has Allwinner XR819 SDIO wifi chip. The board
dts file provides a node enabling mmc1 controller, and a out-of-band
interrupt line of the chip is also connected, although the chip also
supports in-band interrupt.
The current out-of-tree driver is hardcoded to use out-of-band interrupt
as default, and it needs to be modified to use the in-band interrupt.
This commit adds the out-of-band interrupt line into the device tree.
Signed-off-by: Sergey Matyukevich
[Icenowy: Changed vendor prefix to allwinner and modify commit message]
Signed-off-by: Icenowy Zheng
---
Changes in v3 by Icenowy:
- Change the compatible string vendor prefix to "allwinner".
- Modify the commit message.
Changes in v2 by Sergey:
- Adds the compatible string.
arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
index b1502df7b509..6595617204b3 100644
--- a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
+++ b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
@@ -127,6 +127,9 @@
*/
xr819: sdio_wifi@1 {
reg = <1>;
+ compatible = "allwinner,xr819";
+ interrupt-parent = <&pio>;
+ interrupts = <6 10 IRQ_TYPE_EDGE_RISING>;
};
};
--
2.13.5
Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned
On Mon, Oct 2, 2017 at 10:26 PM, Herbert Xu wrote: > On Mon, Oct 02, 2017 at 09:18:24PM -0700, Andy Lutomirski wrote: >> > On Oct 2, 2017, at 7:25 PM, Jia-Ju Bai wrote: >> > >> > The SCTP program may sleep under a spinlock, and the function call path is: >> > sctp_generate_t3_rtx_event (acquire the spinlock) >> > sctp_do_sm >> >sctp_side_effects >> > sctp_cmd_interpreter >> >sctp_make_init_ack >> > sctp_pack_cookie >> >crypto_shash_setkey >> > shash_setkey_unaligned >> >kmalloc(GFP_KERNEL) >> >> I'm going to go out on a limb here: why on Earth is out crypto API so >> full of indirection that we allocate memory at all here? > > The crypto API operates on a one key per-tfm basis. So normally > tfm allocation and key setting is done once only and not done on > the data path. > > I have looked at the SCTP code and it appears to fit this paradigm. > That is, we should be able to allocate the tfm and set the key when > the key is actually generated via get_random_bytes, rather than every > time the key is used which is not only a waste but as you see runs > into API issues. It's a waste because it loses a pre-computation advantage. The fact that it has memory allocation issues is crypto API's fault, full stop. There is no legit reason to need to allocate anything.
[PATCH 2/2] mwifiex: minor cleanups w/ sta_list_spinlock in cfg80211.c
From: Douglas Anderson
The sta_list_spinlock looks to be used to control locking of the
list. Specifically when someone has the lock they may be allowed
to modify or delete elements of the list.
That implies that we shouldn't access the fields of the elements
returned by mwifiex_get_sta_entry() after we've released the
spinlock. Let's make some small changes so this is true.
It's unlikely that this matters since it looks to be just error
handling, but it's nice to be clean.
Signed-off-by: Douglas Anderson
Signed-off-by: Ganapathi Bhat
---
drivers/net/wireless/marvell/mwifiex/cfg80211.c | 14 +-
drivers/net/wireless/marvell/mwifiex/sta_event.c | 6 ++
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index cc7d777..3638b613 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -3794,9 +3794,8 @@ static int mwifiex_cfg80211_set_coalesce(struct wiphy
*wiphy,
spin_lock_irqsave(&priv->sta_list_spinlock, flags);
sta_ptr = mwifiex_get_sta_entry(priv, addr);
- spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
-
if (!sta_ptr) {
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
wiphy_err(wiphy, "%s: Invalid TDLS peer %pM\n",
__func__, addr);
return -ENOENT;
@@ -3804,15 +3803,18 @@ static int mwifiex_cfg80211_set_coalesce(struct wiphy
*wiphy,
if (!(sta_ptr->tdls_cap.extcap.ext_capab[3] &
WLAN_EXT_CAPA4_TDLS_CHAN_SWITCH)) {
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
wiphy_err(wiphy, "%pM do not support tdls cs\n", addr);
return -ENOENT;
}
if (sta_ptr->tdls_status == TDLS_CHAN_SWITCHING ||
sta_ptr->tdls_status == TDLS_IN_OFF_CHAN) {
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
wiphy_err(wiphy, "channel switch is running, abort request\n");
return -EALREADY;
}
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
chan = chandef->chan->hw_value;
second_chan_offset = mwifiex_get_sec_chan_offset(chan);
@@ -3833,18 +3835,20 @@ static int mwifiex_cfg80211_set_coalesce(struct wiphy
*wiphy,
spin_lock_irqsave(&priv->sta_list_spinlock, flags);
sta_ptr = mwifiex_get_sta_entry(priv, addr);
- spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
-
if (!sta_ptr) {
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
wiphy_err(wiphy, "%s: Invalid TDLS peer %pM\n",
__func__, addr);
} else if (!(sta_ptr->tdls_status == TDLS_CHAN_SWITCHING ||
sta_ptr->tdls_status == TDLS_IN_BASE_CHAN ||
sta_ptr->tdls_status == TDLS_IN_OFF_CHAN)) {
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
wiphy_err(wiphy, "tdls chan switch not initialize by %pM\n",
addr);
- } else
+ } else {
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
mwifiex_stop_tdls_cs(priv, addr);
+ }
}
static int
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_event.c
b/drivers/net/wireless/marvell/mwifiex/sta_event.c
index 839df8a..d8db412 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_event.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_event.c
@@ -359,13 +359,12 @@ static void mwifiex_process_uap_tx_pause(struct
mwifiex_private *priv,
} else {
spin_lock_irqsave(&priv->sta_list_spinlock, flags);
sta_ptr = mwifiex_get_sta_entry(priv, tp->peermac);
- spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
-
if (sta_ptr && sta_ptr->tx_pause != tp->tx_pause) {
sta_ptr->tx_pause = tp->tx_pause;
mwifiex_update_ralist_tx_pause(priv, tp->peermac,
tp->tx_pause);
}
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
}
}
@@ -396,14 +395,13 @@ static void mwifiex_process_sta_tx_pause(struct
mwifiex_private *priv,
if (mwifiex_is_tdls_link_setup(status)) {
spin_lock_irqsave(&priv->sta_list_spinlock, flags);
sta_ptr = mwifiex_get_sta_entry(priv, tp->peermac);
- spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
-
if (sta_ptr && sta_ptr->tx_pause != tp->tx_pause) {
sta_ptr->tx_pause = tp->tx_pause;
mwifiex_update_ralist_tx_pause(priv,
[PATCH 1/2] mwifiex: kill useless list_empty checks
From: Douglas Anderson
There's absolutely no reason to check to see if a list is empty
before iterating through it. It's just like writing code like
this:
if (count != 0) {
for (i = 0; i < count; i++) {
...
}
}
The loop will already be avoided if "count == 0" so there was no
reason to check.
Signed-off-by: Douglas Anderson
Signed-off-by: Ganapathi Bhat
---
drivers/net/wireless/marvell/mwifiex/11n.c | 9 -
drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c | 6 --
drivers/net/wireless/marvell/mwifiex/init.c | 4
drivers/net/wireless/marvell/mwifiex/tdls.c | 7 ---
4 files changed, 26 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/11n.c
b/drivers/net/wireless/marvell/mwifiex/11n.c
index 7252069..8772e39 100644
--- a/drivers/net/wireless/marvell/mwifiex/11n.c
+++ b/drivers/net/wireless/marvell/mwifiex/11n.c
@@ -658,12 +658,6 @@ void mwifiex_11n_delba(struct mwifiex_private *priv, int
tid)
unsigned long flags;
spin_lock_irqsave(&priv->rx_reorder_tbl_lock, flags);
- if (list_empty(&priv->rx_reorder_tbl_ptr)) {
- dev_dbg(priv->adapter->dev,
- "mwifiex_11n_delba: rx_reorder_tbl_ptr empty\n");
- goto exit;
- }
-
list_for_each_entry(rx_reor_tbl_ptr, &priv->rx_reorder_tbl_ptr, list) {
if (rx_reor_tbl_ptr->tid == tid) {
dev_dbg(priv->adapter->dev,
@@ -854,9 +848,6 @@ u8 mwifiex_get_sec_chan_offset(int chan)
struct mwifiex_adapter *adapter = priv->adapter;
struct mwifiex_tx_ba_stream_tbl *tx_ba_stream_tbl_ptr;
- if (list_empty(&priv->tx_ba_stream_tbl_ptr))
- return;
-
list_for_each_entry(tx_ba_stream_tbl_ptr,
&priv->tx_ba_stream_tbl_ptr, list) {
if (tx_ba_stream_tbl_ptr->ba_status == BA_SETUP_COMPLETE) {
diff --git a/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c
b/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c
index 274dd5a..d87df2d 100644
--- a/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c
+++ b/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c
@@ -835,12 +835,6 @@ void mwifiex_update_rxreor_flags(struct mwifiex_adapter
*adapter, u8 flags)
continue;
spin_lock_irqsave(&priv->rx_reorder_tbl_lock, lock_flags);
- if (list_empty(&priv->rx_reorder_tbl_ptr)) {
- spin_unlock_irqrestore(&priv->rx_reorder_tbl_lock,
- lock_flags);
- continue;
- }
-
list_for_each_entry(tbl, &priv->rx_reorder_tbl_ptr, list)
tbl->flags = flags;
spin_unlock_irqrestore(&priv->rx_reorder_tbl_lock, lock_flags);
diff --git a/drivers/net/wireless/marvell/mwifiex/init.c
b/drivers/net/wireless/marvell/mwifiex/init.c
index e11919d..1176706 100644
--- a/drivers/net/wireless/marvell/mwifiex/init.c
+++ b/drivers/net/wireless/marvell/mwifiex/init.c
@@ -579,10 +579,6 @@ static void mwifiex_delete_bss_prio_tbl(struct
mwifiex_private *priv)
{
spin_lock_irqsave(lock, flags);
- if (list_empty(head)) {
- spin_unlock_irqrestore(lock, flags);
- continue;
- }
list_for_each_entry_safe(bssprio_node, tmp_node, head,
list) {
if (bssprio_node->priv == priv) {
diff --git a/drivers/net/wireless/marvell/mwifiex/tdls.c
b/drivers/net/wireless/marvell/mwifiex/tdls.c
index e76af286..9fe0bae 100644
--- a/drivers/net/wireless/marvell/mwifiex/tdls.c
+++ b/drivers/net/wireless/marvell/mwifiex/tdls.c
@@ -1413,13 +1413,6 @@ void mwifiex_check_auto_tdls(unsigned long context)
priv->check_tdls_tx = false;
- if (list_empty(&priv->auto_tdls_list)) {
- mod_timer(&priv->auto_tdls_timer,
- jiffies +
- msecs_to_jiffies(MWIFIEX_TIMER_10S));
- return;
- }
-
spin_lock_irqsave(&priv->auto_tdls_lock, flags);
list_for_each_entry(tdls_peer, &priv->auto_tdls_list, list) {
if ((jiffies - tdls_peer->rssi_jiffies) >
--
1.9.1
Re: [PATCH 05/18] net: use ARRAY_SIZE
On Tue, Oct 3, 2017 at 4:22 AM, Jérémy Lefaure
wrote:
> On Mon, 2 Oct 2017 16:07:36 +0300
> Andy Shevchenko wrote:
>
>> > + {&gainctrl_lut_core0_rev0, ARRAY_SIZE(gainctrl_lut_core0_rev0),
>> > 26, 192,
>> > +32},
>>
>> For all such cases I would rather put on one line disregard checkpatch
>> warning for better readability.
> I agree that it would be better. I didn't know that it was possible to
> not follow this rule for anything else than a string.
IMO, it increases readability quite enough to overrule checkpatch recomendation.
--
With Best Regards,
Andy Shevchenko
Re: [PATCH] cfg80211/nl80211: add a port authorized event
On Mon, 2017-10-02 at 23:09 +0200, Arend van Spriel wrote: > On 29-09-17 14:21, Johannes Berg wrote: > > From: Avraham Stern > > > > Add an event that indicates that a connection is authorized > > (i.e. the 4 way handshake was performed by the driver). This event > > should be sent by the driver after sending a connect/roamed event. > > So is this event required for drivers supporting 4-way handshake > offload. If so, the "should" above might need to be "shall" and I > have some changes to do in brcmfmac ;-) I'm not sure it's *required*? I guess it would be for 802.1X on the host/4-way-HS in the device, but not necessarily for the other cases? johannes
