Return-path: Received: from sabertooth01.qualcomm.com ([65.197.215.72]:57104 "EHLO sabertooth01.qualcomm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760689AbaGRMlO (ORCPT ); Fri, 18 Jul 2014 08:41:14 -0400 From: Kalle Valo To: Michal Kazior CC: Varka Bhadram , linux-wireless , "ath10k@lists.infradead.org" , Denton Gentry Subject: Re: [PATCH v3] ath10k: fix Rx aggregation reordering References: <1405505855-13741-1-git-send-email-michal.kazior@tieto.com> <1405509540-32691-1-git-send-email-michal.kazior@tieto.com> <53C66422.10702@gmail.com> Date: Fri, 18 Jul 2014 15:41:07 +0300 In-Reply-To: (Michal Kazior's message of "Wed, 16 Jul 2014 14:41:27 +0200") Message-ID: <87tx6e261o.fsf@kamboji.qca.qualcomm.com> (sfid-20140718_144137_840797_04A2EB26) MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Sender: linux-wireless-owner@vger.kernel.org List-ID: Michal Kazior 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