[PATCH v5] wifi: ath11k: Fix memory reuse logic

[PATCH v5] wifi: ath11k: Fix memory reuse logic

[Muhammad Usama Anjum]

Firmware requests 2 segments at first. The first segment is of 6799360
whose allocation fails due to dma remapping not available. The success
is returned to firmware. Then firmware asks for 22 smaller segments
instead of 2 big ones. Those get allocated successfully. At suspend/
hibernation time, these segments aren't freed as they will be reused
by firmware after resuming.

After resuming, the firmware asks for the 2 segments again with the
first segment of 6799360 size. Since chunk->vaddr is not NULL, the
type and size are compared with the previous type and size to know if
it can be reused or not. Unfortunately, it is detected that it cannot
be reused and this first smaller segment is freed. Then we continue to
allocate 6799360 size memory which fails and ath11k_qmi_free_target_mem_chunk()
is called which frees the second smaller segment as well. Later success
is returned to firmware which asks for 22 smaller segments again. But
as we had freed 2 segments already, we'll allocate the first 2 new
smaller segments again and reuse the remaining 20. Hence 20 small
segments are being reused instead of 22.

Add skip logic when vaddr is set, but size/type don't match. Use the
same skip and success logic as used when dma_alloc_coherent() fails.
By skipping, the possibility of resume failure due to kernel failing to
allocate memory for QMI can be avoided.

	kernel: ath11k_pci 0000:03:00.0: failed to allocate dma memory for qmi (524288 B type 1)
	ath11k_pci 0000:03:00.0: failed to allocate qmi target memory: -22

Tested-on: WCN6855 WLAN.HSP.1.1-03926.13-QCAHSPSWPL_V2_SILICONZ_CE-2.52297.6

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
Changes since v1:
- Update description

Changes since v2:
- Update description and title of patch

Changes since v3:
- Update description and title of patch

Changes since v4:
- Update title of the patch
---
 drivers/net/wireless/ath/ath11k/qmi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
index 47b9d4126d3a9..2782f4723e413 100644
--- a/drivers/net/wireless/ath/ath11k/qmi.c
+++ b/drivers/net/wireless/ath/ath11k/qmi.c
@@ -1993,6 +1993,15 @@ static int ath11k_qmi_alloc_target_mem_chunk(struct ath11k_base *ab)
 			    chunk->prev_size == chunk->size)
 				continue;
 
+			if (ab->qmi.mem_seg_count <= ATH11K_QMI_FW_MEM_REQ_SEGMENT_CNT) {
+				ath11k_dbg(ab, ATH11K_DBG_QMI,
+					   "size/type mismatch (current %d %u) (prev %d %u), try later with small size\n",
+					    chunk->size, chunk->type,
+					    chunk->prev_size, chunk->prev_type);
+				ab->qmi.target_mem_delayed = true;
+				return 0;
+			}
+
 			/* cannot reuse the existing chunk */
 			dma_free_coherent(ab->dev, chunk->prev_size,
 					  chunk->vaddr, chunk->paddr);
-- 
2.43.0

Re: [PATCH v5] wifi: ath11k: Fix memory reuse logic

[Baochen Qiang]

On 4/28/2025 4:02 PM, Muhammad Usama Anjum wrote:
> Firmware requests 2 segments at first. The first segment is of 6799360
> whose allocation fails due to dma remapping not available. The success
> is returned to firmware. Then firmware asks for 22 smaller segments
> instead of 2 big ones. Those get allocated successfully. At suspend/
> hibernation time, these segments aren't freed as they will be reused
> by firmware after resuming.
> 
> After resuming, the firmware asks for the 2 segments again with the
> first segment of 6799360 size. Since chunk->vaddr is not NULL, the
> type and size are compared with the previous type and size to know if
> it can be reused or not. Unfortunately, it is detected that it cannot
> be reused and this first smaller segment is freed. Then we continue to
> allocate 6799360 size memory which fails and ath11k_qmi_free_target_mem_chunk()
> is called which frees the second smaller segment as well. Later success
> is returned to firmware which asks for 22 smaller segments again. But
> as we had freed 2 segments already, we'll allocate the first 2 new
> smaller segments again and reuse the remaining 20. Hence 20 small
> segments are being reused instead of 22.
> 
> Add skip logic when vaddr is set, but size/type don't match. Use the
> same skip and success logic as used when dma_alloc_coherent() fails.
> By skipping, the possibility of resume failure due to kernel failing to
> allocate memory for QMI can be avoided.
> 
> 	kernel: ath11k_pci 0000:03:00.0: failed to allocate dma memory for qmi (524288 B type 1)
> 	ath11k_pci 0000:03:00.0: failed to allocate qmi target memory: -22
> 
> Tested-on: WCN6855 WLAN.HSP.1.1-03926.13-QCAHSPSWPL_V2_SILICONZ_CE-2.52297.6
> 
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
> Changes since v1:
> - Update description
> 
> Changes since v2:
> - Update description and title of patch
> 
> Changes since v3:
> - Update description and title of patch
> 
> Changes since v4:
> - Update title of the patch
> ---
>  drivers/net/wireless/ath/ath11k/qmi.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
> index 47b9d4126d3a9..2782f4723e413 100644
> --- a/drivers/net/wireless/ath/ath11k/qmi.c
> +++ b/drivers/net/wireless/ath/ath11k/qmi.c
> @@ -1993,6 +1993,15 @@ static int ath11k_qmi_alloc_target_mem_chunk(struct ath11k_base *ab)
>  			    chunk->prev_size == chunk->size)
>  				continue;
>  
> +			if (ab->qmi.mem_seg_count <= ATH11K_QMI_FW_MEM_REQ_SEGMENT_CNT) {
> +				ath11k_dbg(ab, ATH11K_DBG_QMI,
> +					   "size/type mismatch (current %d %u) (prev %d %u), try later with small size\n",
> +					    chunk->size, chunk->type,
> +					    chunk->prev_size, chunk->prev_type);
> +				ab->qmi.target_mem_delayed = true;
> +				return 0;
> +			}
> +
>  			/* cannot reuse the existing chunk */
>  			dma_free_coherent(ab->dev, chunk->prev_size,
>  					  chunk->vaddr, chunk->paddr);

Reviewed-by: Baochen Qiang <quic_bqiang@quicinc.com>

Re: [PATCH v5] wifi: ath11k: Fix memory reuse logic

[Muhammad Usama Anjum]

Reminder to pick this patch.

On 4/28/25 1:02 PM, Muhammad Usama Anjum wrote:
> Firmware requests 2 segments at first. The first segment is of 6799360
> whose allocation fails due to dma remapping not available. The success
> is returned to firmware. Then firmware asks for 22 smaller segments
> instead of 2 big ones. Those get allocated successfully. At suspend/
> hibernation time, these segments aren't freed as they will be reused
> by firmware after resuming.
> 
> After resuming, the firmware asks for the 2 segments again with the
> first segment of 6799360 size. Since chunk->vaddr is not NULL, the
> type and size are compared with the previous type and size to know if
> it can be reused or not. Unfortunately, it is detected that it cannot
> be reused and this first smaller segment is freed. Then we continue to
> allocate 6799360 size memory which fails and ath11k_qmi_free_target_mem_chunk()
> is called which frees the second smaller segment as well. Later success
> is returned to firmware which asks for 22 smaller segments again. But
> as we had freed 2 segments already, we'll allocate the first 2 new
> smaller segments again and reuse the remaining 20. Hence 20 small
> segments are being reused instead of 22.
> 
> Add skip logic when vaddr is set, but size/type don't match. Use the
> same skip and success logic as used when dma_alloc_coherent() fails.
> By skipping, the possibility of resume failure due to kernel failing to
> allocate memory for QMI can be avoided.
> 
> 	kernel: ath11k_pci 0000:03:00.0: failed to allocate dma memory for qmi (524288 B type 1)
> 	ath11k_pci 0000:03:00.0: failed to allocate qmi target memory: -22
> 
> Tested-on: WCN6855 WLAN.HSP.1.1-03926.13-QCAHSPSWPL_V2_SILICONZ_CE-2.52297.6
> 
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
> Changes since v1:
> - Update description
> 
> Changes since v2:
> - Update description and title of patch
> 
> Changes since v3:
> - Update description and title of patch
> 
> Changes since v4:
> - Update title of the patch
> ---
>  drivers/net/wireless/ath/ath11k/qmi.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
> index 47b9d4126d3a9..2782f4723e413 100644
> --- a/drivers/net/wireless/ath/ath11k/qmi.c
> +++ b/drivers/net/wireless/ath/ath11k/qmi.c
> @@ -1993,6 +1993,15 @@ static int ath11k_qmi_alloc_target_mem_chunk(struct ath11k_base *ab)
>  			    chunk->prev_size == chunk->size)
>  				continue;
>  
> +			if (ab->qmi.mem_seg_count <= ATH11K_QMI_FW_MEM_REQ_SEGMENT_CNT) {
> +				ath11k_dbg(ab, ATH11K_DBG_QMI,
> +					   "size/type mismatch (current %d %u) (prev %d %u), try later with small size\n",
> +					    chunk->size, chunk->type,
> +					    chunk->prev_size, chunk->prev_type);
> +				ab->qmi.target_mem_delayed = true;
> +				return 0;
> +			}
> +
>  			/* cannot reuse the existing chunk */
>  			dma_free_coherent(ab->dev, chunk->prev_size,
>  					  chunk->vaddr, chunk->paddr);

-- 
Regards,
Usama

Re: [PATCH v5] wifi: ath11k: Fix memory reuse logic

[Jeff Johnson]

v2 feedback was not incorporated:
For starters, can we make the subject a bit more specific, i.e.
Fix MHI target memory reuse logic

But don't repost for this -- I'll make that change in ath/pending

However, does ath12k need the same fix?
If so, can you post a separate patch for that?

/jeff

Re: [PATCH v5] wifi: ath11k: Fix memory reuse logic

[Baochen Qiang]

On 5/6/2025 3:17 AM, Jeff Johnson wrote:
> v2 feedback was not incorporated:
> For starters, can we make the subject a bit more specific, i.e.
> Fix MHI target memory reuse logic

Regarding this specific comment:

This change it to refine the QMI memory reuse logic, it is not related to any MHI target
memory.

Actually Muhammad changed the subject according to your comment in v3, but changed it back
based on my above comment in v5.

> 
> But don't repost for this -- I'll make that change in ath/pending
> 
> However, does ath12k need the same fix?
> If so, can you post a separate patch for that?
> 
> /jeff

Re: [PATCH v5] wifi: ath11k: Fix memory reuse logic

[Baochen Qiang]

On 5/6/2025 3:17 AM, Jeff Johnson wrote:
> v2 feedback was not incorporated:
> For starters, can we make the subject a bit more specific, i.e.
> Fix MHI target memory reuse logic
> 

Ideally I prefer below subject

wifi: ath12k: Fix QMI target memory reuse logic


> But don't repost for this -- I'll make that change in ath/pending
> 
> However, does ath12k need the same fix?
> If so, can you post a separate patch for that?
> 
> /jeff

Re: [PATCH v5] wifi: ath11k: Fix memory reuse logic

[Muhammad Usama Anjum]

On 5/6/25 12:17 AM, Jeff Johnson wrote:
> v2 feedback was not incorporated:
> For starters, can we make the subject a bit more specific, i.e.
> Fix MHI target memory reuse logic
> 
> But don't repost for this -- I'll make that change in ath/pending
I'd changed again on the request of another reviewer. Please feel free
to change as you like. I don't have any opinion on it.

> 
> However, does ath12k need the same fix?
Looking at ath12k, there is similar code structure in
ath12k_qmi_alloc_chunk(). By adding some logging, we can confirm if
ath12k requires the fix or not. As a lot of code is similar in both
drivers, ath12k may require the same fix.

I don't have access to ath12k. So I cannot test on it.

> If so, can you post a separate patch for that?
> 
> /jeff

-- 
Regards,
Usama

Re: [PATCH v5] wifi: ath11k: Fix memory reuse logic

[Jeff Johnson]

On 5/6/2025 6:10 AM, Muhammad Usama Anjum wrote:
> On 5/6/25 12:17 AM, Jeff Johnson wrote:
>> v2 feedback was not incorporated:
>> For starters, can we make the subject a bit more specific, i.e.
>> Fix MHI target memory reuse logic
>>
>> But don't repost for this -- I'll make that change in ath/pending
> I'd changed again on the request of another reviewer. Please feel free
> to change as you like. I don't have any opinion on it.
> 
>>
>> However, does ath12k need the same fix?
> Looking at ath12k, there is similar code structure in
> ath12k_qmi_alloc_chunk(). By adding some logging, we can confirm if
> ath12k requires the fix or not. As a lot of code is similar in both
> drivers, ath12k may require the same fix.
> 
> I don't have access to ath12k. So I cannot test on it.

Baochen, do you want to propagate the change to ath12k?

Re: [PATCH v5] wifi: ath11k: Fix memory reuse logic

[Jeff Johnson]

On 5/5/2025 11:43 PM, Baochen Qiang wrote:
> 
> 
> On 5/6/2025 3:17 AM, Jeff Johnson wrote:
>> v2 feedback was not incorporated:
>> For starters, can we make the subject a bit more specific, i.e.
>> Fix MHI target memory reuse logic
>>
> 
> Ideally I prefer below subject
> 
> wifi: ath12k: Fix QMI target memory reuse logic

Done:
https://git.kernel.org/pub/scm/linux/kernel/git/ath/ath.git/commit/?h=pending&id=ec570013de60b8b0bfa3cafd516ba323e6e29a8d

Re: [PATCH v5] wifi: ath11k: Fix memory reuse logic

[Baochen Qiang]

On 5/6/2025 10:37 PM, Jeff Johnson wrote:
> On 5/6/2025 6:10 AM, Muhammad Usama Anjum wrote:
>> On 5/6/25 12:17 AM, Jeff Johnson wrote:
>>> v2 feedback was not incorporated:
>>> For starters, can we make the subject a bit more specific, i.e.
>>> Fix MHI target memory reuse logic
>>>
>>> But don't repost for this -- I'll make that change in ath/pending
>> I'd changed again on the request of another reviewer. Please feel free
>> to change as you like. I don't have any opinion on it.
>>
>>>
>>> However, does ath12k need the same fix?
>> Looking at ath12k, there is similar code structure in
>> ath12k_qmi_alloc_chunk(). By adding some logging, we can confirm if
>> ath12k requires the fix or not. As a lot of code is similar in both
>> drivers, ath12k may require the same fix.
>>
>> I don't have access to ath12k. So I cannot test on it.
> 
> Baochen, do you want to propagate the change to ath12k?

Yeah, I can test and fix if same issue is there.

Re: [PATCH v5] wifi: ath11k: Fix memory reuse logic

[Jeff Johnson]

On Mon, 28 Apr 2025 13:02:41 +0500, Muhammad Usama Anjum wrote:
> Firmware requests 2 segments at first. The first segment is of 6799360
> whose allocation fails due to dma remapping not available. The success
> is returned to firmware. Then firmware asks for 22 smaller segments
> instead of 2 big ones. Those get allocated successfully. At suspend/
> hibernation time, these segments aren't freed as they will be reused
> by firmware after resuming.
> 
> [...]

Applied, thanks!

[1/1] wifi: ath11k: Fix memory reuse logic
      commit: cd2e7bae92bd7e65063ab8d04721d2b711ba4cbe

Best regards,
-- 
Jeff Johnson <jeff.johnson@oss.qualcomm.com>