Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <b99fe7bd-f97f-31dd-de4f-2314ecf4b753@wetzel-home.de>
Date:   Mon, 28 Nov 2022 19:16:51 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.5.0
From:   Alexander Wetzel <alexander@wetzel-home.de>
Subject: Re: [PATCH v2] wifi: mac80211: integrate PS buffering into iTXQ
To:     linux-wireless@vger.kernel.org
Cc:     Johannes Berg <johannes@sipsolutions.net>
References: <20221102184005.10649-1-alexander@wetzel-home.de>
Content-Language: en-US
In-Reply-To: <20221102184005.10649-1-alexander@wetzel-home.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

> @@ -1579,16 +1561,39 @@ void ieee80211_sta_ps_deliver_wakeup(struct sta_info *sta)
>   		tmp = skb_queue_len(&pending);
>   		filtered += tmp - count;
>   		count = tmp;
> -
> -		spin_lock_irqsave(&sta->ps_tx_buf[ac].lock, flags);
> -		skb_queue_splice_tail_init(&sta->ps_tx_buf[ac], &pending);
> -		spin_unlock_irqrestore(&sta->ps_tx_buf[ac].lock, flags);
> -		tmp = skb_queue_len(&pending);
> -		buffered += tmp - count;
>   	}
>   
> +	skb_queue_head_init(&pending);
>   	ieee80211_add_pending_skbs(local, &pending);
>   
> +	for (i = 0; i < ARRAY_SIZE(sta->sta.txq); i++) {
> +		if (!sta->sta.txq[i])
> +			continue;
> +
> +		txqi = to_txq_info(sta->sta.txq[i]);
> +		clear_bit(IEEE80211_TXQ_STOP_PS, &txqi->flags);
> +	}
> +
> +	/* Make sure TX path has stopped setting txq_buffered_tids */
> +	synchronize_net();
> +

We are atomic here and can't sleep. Working on that now, but this gets 
surprisingly complex fast.
Review of the rest is still welcome but this patch must not be merged as 
it is.

It kind of looks like the current (unpatched) code is not only sending 
out filtered frames after iTXQ but also misses the required 
rcu_read_lock to be allowed to call schedule_and_wake_txq() in this 
function when called from drv_deliver_wk()...

Alexander