On 16 July 2014 14:35, Michal Kazior <michal.kazior@xxxxxxxxx> wrote:
> On 16 July 2014 13:38, Varka Bhadram <varkabhadram@xxxxxxxxx> wrote:
>> On 07/16/2014 04:49 PM, Michal Kazior wrote:
>>
>> (...)
>>
>>
>>> +static void ath10k_htt_rx_addba(struct ath10k *ar, struct htt_resp *resp)
>>> +{
>>> + struct htt_rx_addba *ev = &resp->rx_addba;
>>> + struct ath10k_peer *peer;
>>> + struct ath10k_vif *arvif;
>>> + u16 info0, tid, peer_id;
>>> +
>>> + info0 = __le32_to_cpu(ev->info0);
>>> + tid = MS(info0, HTT_RX_BA_INFO0_TID);
>>> + peer_id = MS(info0, HTT_RX_BA_INFO0_PEER_ID);
>>> +
>>> + ath10k_dbg(ATH10K_DBG_HTT,
>>> + "htt rx addba tid %hu peer_id %hu size %hhu\n",
>>> + tid, peer_id, ev->window_size);
>>> +
>>> + spin_lock_bh(&ar->data_lock);
>>> + peer = ath10k_peer_find_by_id(ar, peer_id);
>>> + if (!peer) {
>>> + ath10k_warn("received addba event for invalid peer_id:
>>> %hu\n",
>>> + peer_id);
>>> + spin_unlock_bh(&ar->data_lock);
>>> + return;
>>> + }
>>
>>
>> Here my concern in amount of time holding the lock...
>>
>> spin_lock_bh(&ar->data_lock);
>> peer = ath10k_peer_find_by_id(ar, peer_id);
>> if (!peer) {
>> spin_unlock_bh(&ar->data_lock);
>>
>> ath10k_warn("received addba event for invalid peer_id: %hu\n",
>> peer_id);
>> return;
>> }
>>
>> No need to of putting the debug message inside the critical region... :-)
>
> Sounds reasonable in this case as I'm not printing spinlock-protected values.
..and I realized this isn't true upon hitting the send button.
The other print uses peer->vdev_id. The peer was acquired under a lock
and must not be used after the lock is released. It'll just look
confusing if I mix ordering of unlock/print in some cases so I'll
leave it as is.
(sorry for the noise)
Michał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
