Re: [PATCH v3] ath10k: fix Rx aggregation reordering

[Kalle Valo <kvalo@qca.qualcomm.com>]

Michal Kazior <michal.kazior@tieto.com> writes:

>>> 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.

I actually prefer also the original form of "warn(); unlock(); return;",
somehow it feels more natural for me. And this is on error path where
efficiency is not really a priority.

-- 
Kalle Valo