diff mbox series

[v5.7,7/8] iwlwifi: mvm: fix inactive TID removal return value usage

Message ID iwlwifi.20200403112332.75faaf2137f4.I9e27ccc3ee3c8855fc13682592b571581925dfbd@changeid (mailing list archive)
State Superseded
Delegated to: Luca Coelho
Headers show
Series iwlwifi: fixes intended for v5.7 2020-04-03 | expand

Commit Message

Luca Coelho April 3, 2020, 8:29 a.m. UTC
From: Johannes Berg <johannes.berg@intel.com>

The function iwl_mvm_remove_inactive_tids() returns bool, so we
should just check "if (ret)", not "if (ret >= 0)" (which would
do nothing useful here). We obviously therefore cannot use the
return value of the function for the free_queue, we need to use
the queue (i) we're currently dealing with instead.

Cc: stable@vger.kernel.org # v5.4+
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/sta.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Mark Asselstine April 3, 2020, 2:46 p.m. UTC | #1
On Fri, Apr 3, 2020 at 4:31 AM Luca Coelho <luca@coelho.fi> wrote:
>
> From: Johannes Berg <johannes.berg@intel.com>

I sent Johannes part of this fix weeks ago and heard nothing back. I
am far from a glory hound but something is wrong with this list if
fixes are sat on for weeks and then the fix shows up with any
acknowledgment lost. At minimum a note saying that a fix existed and
would be merged shortly would have been nice.

Mark

>
> The function iwl_mvm_remove_inactive_tids() returns bool, so we
> should just check "if (ret)", not "if (ret >= 0)" (which would
> do nothing useful here). We obviously therefore cannot use the
> return value of the function for the free_queue, we need to use
> the queue (i) we're currently dealing with instead.
>
> Cc: stable@vger.kernel.org # v5.4+
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> ---
>  drivers/net/wireless/intel/iwlwifi/mvm/sta.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
> index 251d6fbb1da5..56ae72debb96 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
> @@ -1169,9 +1169,9 @@ static int iwl_mvm_inactivity_check(struct iwl_mvm *mvm, u8 alloc_for_sta)
>                                                    inactive_tid_bitmap,
>                                                    &unshare_queues,
>                                                    &changetid_queues);
> -               if (ret >= 0 && free_queue < 0) {
> +               if (ret && free_queue < 0) {
>                         queue_owner = sta;
> -                       free_queue = ret;
> +                       free_queue = i;
>                 }
>                 /* only unlock sta lock - we still need the queue info lock */
>                 spin_unlock_bh(&mvmsta->lock);
> --
> 2.25.1
>
Johannes Berg April 3, 2020, 6:58 p.m. UTC | #2
On Fri, 2020-04-03 at 10:46 -0400, Mark Asselstine wrote:
> On Fri, Apr 3, 2020 at 4:31 AM Luca Coelho <luca@coelho.fi> wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> 
> I sent Johannes part of this fix weeks ago and heard nothing back. I
> am far from a glory hound but something is wrong with this list if
> fixes are sat on for weeks and then the fix shows up with any
> acknowledgment lost. At minimum a note saying that a fix existed and
> would be merged shortly would have been nice.

Uh, sorry. I really didn't have your fix on my radar when developing
this, and cannot even remember it now.

I guess I could've saved myself some work there ...

Sorry!

johannes
Luca Coelho April 3, 2020, 7:08 p.m. UTC | #3
On Fri, 2020-04-03 at 20:58 +0200, Johannes Berg wrote:
> On Fri, 2020-04-03 at 10:46 -0400, Mark Asselstine wrote:
> > On Fri, Apr 3, 2020 at 4:31 AM Luca Coelho <luca@coelho.fi> wrote:
> > > From: Johannes Berg <johannes.berg@intel.com>
> > 
> > I sent Johannes part of this fix weeks ago and heard nothing back. I
> > am far from a glory hound but something is wrong with this list if
> > fixes are sat on for weeks and then the fix shows up with any
> > acknowledgment lost. At minimum a note saying that a fix existed and
> > would be merged shortly would have been nice.
> 
> Uh, sorry. I really didn't have your fix on my radar when developing
> this, and cannot even remember it now.
> 
> I guess I could've saved myself some work there ...

This is my fault, sorry.  I've been sitting on patches sent to the list
for some time now.  I have a big backlog of patches in our internal
tree to send out and have been prioritizing those before processing
patches coming from the community.

I'm finally catching up now with fixes for v5.7 (and stable) and I
promise to do better from now on.

My sincere apologies.

--
Cheers,
Luca.
Mark Asselstine April 3, 2020, 9:26 p.m. UTC | #4
On Fri, Apr 3, 2020 at 3:08 PM Luca Coelho <luca@coelho.fi> wrote:
>
> On Fri, 2020-04-03 at 20:58 +0200, Johannes Berg wrote:
> > On Fri, 2020-04-03 at 10:46 -0400, Mark Asselstine wrote:
> > > On Fri, Apr 3, 2020 at 4:31 AM Luca Coelho <luca@coelho.fi> wrote:
> > > > From: Johannes Berg <johannes.berg@intel.com>
> > >
> > > I sent Johannes part of this fix weeks ago and heard nothing back. I
> > > am far from a glory hound but something is wrong with this list if
> > > fixes are sat on for weeks and then the fix shows up with any
> > > acknowledgment lost. At minimum a note saying that a fix existed and
> > > would be merged shortly would have been nice.
> >
> > Uh, sorry. I really didn't have your fix on my radar when developing
> > this, and cannot even remember it now.
> >
> > I guess I could've saved myself some work there ...
>
> This is my fault, sorry.  I've been sitting on patches sent to the list
> for some time now.  I have a big backlog of patches in our internal
> tree to send out and have been prioritizing those before processing
> patches coming from the community.
>
> I'm finally catching up now with fixes for v5.7 (and stable) and I
> promise to do better from now on.
>
> My sincere apologies.

np. More than anything its the duplication of work that sucks. In the
end we all want the same goal, to improve the functionality of the
drivers so let's keep pushing forward with that. Stay safe and have a
good weekend.

Mark


>
> --
> Cheers,
> Luca.
>
diff mbox series

Patch

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
index 251d6fbb1da5..56ae72debb96 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
@@ -1169,9 +1169,9 @@  static int iwl_mvm_inactivity_check(struct iwl_mvm *mvm, u8 alloc_for_sta)
 						   inactive_tid_bitmap,
 						   &unshare_queues,
 						   &changetid_queues);
-		if (ret >= 0 && free_queue < 0) {
+		if (ret && free_queue < 0) {
 			queue_owner = sta;
-			free_queue = ret;
+			free_queue = i;
 		}
 		/* only unlock sta lock - we still need the queue info lock */
 		spin_unlock_bh(&mvmsta->lock);