Subject: Re: [PATCH] mac80211: do not call driver wake_tx_queue op during
 reconfig
To:     Felix Fietkau <nbd@nbd.name>, linux-wireless@vger.kernel.org
Cc:     johannes@sipsolutions.net, stable@vger.kernel.org
References: <20190301134837.88087-1-nbd@nbd.name>
From:   Ben Greear <greearb@candelatech.com>
Organization: Candela Technologies
Message-ID: <09a12ed0-dd86-2077-9ecb-c62a52186952@candelatech.com>
Date:   Fri, 1 Mar 2019 07:29:37 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.4.0
MIME-Version: 1.0
In-Reply-To: <20190301134837.88087-1-nbd@nbd.name>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On 3/1/19 5:48 AM, Felix Fietkau wrote:
> There are several scenarios in which mac80211 can call drv_wake_tx_queue
> after ieee80211_restart_hw has been called and has not yet completed.
> Driver private structs are considered uninitialized until mac80211 has
> uploaded the vifs, stations and keys again, so using private tx queue
> data during that time is not safe.
> 
> The driver can also not rely on drv_reconfig_complete to figure out when
> it is safe to accept drv_wake_tx_queue calls again, because it is only
> called after all tx queues are woken again.
> 
> To fix this, bail out early in drv_wake_tx_queue if local->in_reconfig
> is set.

This reminded me of a patch I posted back in 2016.  The discussion just sort of
ended on it, but curious if you have a new opinion on it after debugging the
issue in this patch:

https://patchwork.kernel.org/patch/9457709/

For what its worth, I've been using the patch above since I posted it, and
it seems to work well for ath9k and ath10k.

Thanks,
Ben


> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> ---
>   net/mac80211/driver-ops.h | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
> index 28d022a3eee3..ae4f0be3b393 100644
> --- a/net/mac80211/driver-ops.h
> +++ b/net/mac80211/driver-ops.h
> @@ -1195,6 +1195,9 @@ static inline void drv_wake_tx_queue(struct ieee80211_local *local,
>   {
>   	struct ieee80211_sub_if_data *sdata = vif_to_sdata(txq->txq.vif);
>   
> +	if (local->in_reconfig)
> +		return;
> +
>   	if (!check_sdata_in_driver(sdata))
>   		return;
>   
> 


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com