2020-03-13 03:06:39

by Mark Asselstine

[permalink] [raw]
Subject: [PATCH] iwlwifi: set 'free_queue' to the loop variable, not a meaningless bool

From: Mark Asselstine <[email protected]>

The loop variable 'i' is passed to iwl_mvm_remove_inactive_tids() as
the queue number to operate on. iwl_mvm_remove_inactive_tids()
operates on that queue number then returns true if the queue can be
reused, the returned bool is stored in 'ret'. We do not want to set
'free_queue' to the returned bool stored in 'ret' but rather the loop
variable, so we are actually operating on the right queue through the
rest of iwl_mvm_inactivity_check() and have it return a proper queue
number.

Signed-off-by: Mark Asselstine <[email protected]>
---
drivers/net/wireless/intel/iwlwifi/mvm/sta.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
index 9db2555518aa..2fcaf779649a 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
@@ -1166,7 +1166,7 @@ static int iwl_mvm_inactivity_check(struct iwl_mvm *mvm, u8 alloc_for_sta)
&changetid_queues);
if (ret >= 0 && 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.20.1


2020-03-17 20:13:42

by Mark Asselstine

[permalink] [raw]
Subject: Re: [PATCH] iwlwifi: set 'free_queue' to the loop variable, not a meaningless bool

Luca,

This is my first time sending a fix for iwlwifi and I haven't seen any
feedback or seen the code merged. Should I have sent this somewhere
else?

Thanks,
Mark

On Thu, Mar 12, 2020 at 11:06 PM Mark Asselstine
<[email protected]> wrote:
>
> From: Mark Asselstine <[email protected]>
>
> The loop variable 'i' is passed to iwl_mvm_remove_inactive_tids() as
> the queue number to operate on. iwl_mvm_remove_inactive_tids()
> operates on that queue number then returns true if the queue can be
> reused, the returned bool is stored in 'ret'. We do not want to set
> 'free_queue' to the returned bool stored in 'ret' but rather the loop
> variable, so we are actually operating on the right queue through the
> rest of iwl_mvm_inactivity_check() and have it return a proper queue
> number.
>
> Signed-off-by: Mark Asselstine <[email protected]>
> ---
> drivers/net/wireless/intel/iwlwifi/mvm/sta.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
> index 9db2555518aa..2fcaf779649a 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
> @@ -1166,7 +1166,7 @@ static int iwl_mvm_inactivity_check(struct iwl_mvm *mvm, u8 alloc_for_sta)
> &changetid_queues);
> if (ret >= 0 && 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.20.1
>

2020-03-17 20:17:22

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH] iwlwifi: set 'free_queue' to the loop variable, not a meaningless bool

On Tue, 2020-03-17 at 16:12 -0400, Mark Asselstine wrote:
> Luca,
>
> This is my first time sending a fix for iwlwifi and I haven't seen any
> feedback or seen the code merged. Should I have sent this somewhere
> else?

Hi Mark,

I'm sorry, I have a huge backlog of work and I'm trying to catch up. I
will review your patch and apply it to our internal tree and then
upstream it according to our usual process asap.

Thanks for your patch!

--
Cheers,
Luca.

2020-03-18 20:42:39

by Mark Asselstine

[permalink] [raw]
Subject: Re: [PATCH] iwlwifi: set 'free_queue' to the loop variable, not a meaningless bool

On Tue, Mar 17, 2020 at 4:15 PM Luciano Coelho <[email protected]> wrote:
>
> On Tue, 2020-03-17 at 16:12 -0400, Mark Asselstine wrote:
> > Luca,
> >
> > This is my first time sending a fix for iwlwifi and I haven't seen any
> > feedback or seen the code merged. Should I have sent this somewhere
> > else?
>
> Hi Mark,
>
> I'm sorry, I have a huge backlog of work and I'm trying to catch up. I
> will review your patch and apply it to our internal tree and then
> upstream it according to our usual process asap.
>
> Thanks for your patch!

Thanks Luca,

I appreciate the work maintainers do and how busy you can get, so I
completely understand. Normally I would have just waited, but again
this is not an area I have worked in so just wanted to double check I
didn't screw up. Thanks for the response.

Mark

>
> --
> Cheers,
> Luca.
>