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

From: Kalle Valo <kvalo@qca.qualcomm.com>
To: Michal Kazior <michal.kazior@tieto.com>
Cc: Varka Bhadram <varkabhadram@gmail.com>,
	linux-wireless <linux-wireless@vger.kernel.org>,
	"ath10k@lists.infradead.org" <ath10k@lists.infradead.org>,
	Denton Gentry <denton.gentry@gmail.com>
Subject: Re: [PATCH v3] ath10k: fix Rx aggregation reordering
Date: Fri, 18 Jul 2014 15:41:07 +0300	[thread overview]
Message-ID: <87tx6e261o.fsf@kamboji.qca.qualcomm.com> (raw)
In-Reply-To: <CA+BoTQkk5oYS4JZ5a6W02rZ62AgxcHE76knS7QA2VfZDxYi2Jg@mail.gmail.com> (Michal Kazior's message of "Wed, 16 Jul 2014 14:41:27 +0200")

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

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k