[PATCH v2] wifi: wilc1000: Added back-off algorithm to balance tx queue packets.

[PATCH v2] wifi: wilc1000: Added back-off algorithm to balance tx queue packets.

[Prasurjya.Rohansaikia]

From: Prasurjya Rohan Saikia <prasurjya.rohansaikia@microchip.com>

Added an algorithm to backoff the Tx Task when low memory scenario is
triggered at firmware. During high data transfer from host, the firmware
runs out of VMM memory, which is used to hold the frames from the host.
So added flow control to delay the transmit from host side when there is
not enough space to accomodate frames in firmware side.

Signed-off-by: Prasurjya Rohan Saikia <prasurjya.rohansaikia@microchip.com>
---
v1 -> v2: Removed the unused `timeout` variable.
---
 .../net/wireless/microchip/wilc1000/netdev.c  | 21 +++++++++++++++----
 .../net/wireless/microchip/wilc1000/netdev.h  |  2 ++
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.c b/drivers/net/wireless/microchip/wilc1000/netdev.c
index e9f59de31b0b..faf175a0f05b 100644
--- a/drivers/net/wireless/microchip/wilc1000/netdev.c
+++ b/drivers/net/wireless/microchip/wilc1000/netdev.c
@@ -148,8 +148,8 @@ static int wilc_txq_task(void *vp)
 
 	complete(&wl->txq_thread_started);
 	while (1) {
-		wait_for_completion(&wl->txq_event);
-
+		if (wait_for_completion_interruptible(&wl->txq_event))
+			continue;
 		if (wl->close) {
 			complete(&wl->txq_thread_started);
 
@@ -166,12 +166,25 @@ static int wilc_txq_task(void *vp)
 				srcu_idx = srcu_read_lock(&wl->srcu);
 				list_for_each_entry_rcu(ifc, &wl->vif_list,
 							list) {
-					if (ifc->mac_opened && ifc->ndev)
+					if (ifc->mac_opened &&
+					    netif_queue_stopped(ifc->ndev))
 						netif_wake_queue(ifc->ndev);
 				}
 				srcu_read_unlock(&wl->srcu, srcu_idx);
 			}
-		} while (ret == WILC_VMM_ENTRY_FULL_RETRY && !wl->close);
+			if (ret != WILC_VMM_ENTRY_FULL_RETRY)
+				break;
+			/* Back off from sending packets for some time.
+			 * schedule_timeout will allow RX task to run and free
+			 * buffers. Setting state to TASK_INTERRUPTIBLE will
+			 * put the thread back to CPU running queue when it's
+			 * signaled even if 'timeout' isn't elapsed. This gives
+			 * faster chance for reserved SK buffers to be freed
+			 */
+			set_current_state(TASK_INTERRUPTIBLE);
+			schedule_timeout(msecs_to_jiffies
+					 (TX_BACKOFF_WEIGHT_MS));
+		} while (!wl->close);
 	}
 	return 0;
 }
diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.h b/drivers/net/wireless/microchip/wilc1000/netdev.h
index bb1a315a7b7e..aafe3dc44ac6 100644
--- a/drivers/net/wireless/microchip/wilc1000/netdev.h
+++ b/drivers/net/wireless/microchip/wilc1000/netdev.h
@@ -27,6 +27,8 @@
 #define TCP_ACK_FILTER_LINK_SPEED_THRESH	54
 #define DEFAULT_LINK_SPEED			72
 
+#define TX_BACKOFF_WEIGHT_MS			1
+
 struct wilc_wfi_stats {
 	unsigned long rx_packets;
 	unsigned long tx_packets;
-- 
2.34.1

Re: [PATCH v2] wifi: wilc1000: Added back-off algorithm to balance tx queue packets.

[Kalle Valo]

<Prasurjya.Rohansaikia@microchip.com> writes:

> From: Prasurjya Rohan Saikia <prasurjya.rohansaikia@microchip.com>
>
> Added an algorithm to backoff the Tx Task when low memory scenario is
> triggered at firmware. During high data transfer from host, the firmware
> runs out of VMM memory, which is used to hold the frames from the host.
> So added flow control to delay the transmit from host side when there is
> not enough space to accomodate frames in firmware side.
>
> Signed-off-by: Prasurjya Rohan Saikia <prasurjya.rohansaikia@microchip.com>

[...]

> -		} while (ret == WILC_VMM_ENTRY_FULL_RETRY && !wl->close);
> +			if (ret != WILC_VMM_ENTRY_FULL_RETRY)
> +				break;
> +			/* Back off from sending packets for some time.
> +			 * schedule_timeout will allow RX task to run and free
> +			 * buffers. Setting state to TASK_INTERRUPTIBLE will
> +			 * put the thread back to CPU running queue when it's
> +			 * signaled even if 'timeout' isn't elapsed. This gives
> +			 * faster chance for reserved SK buffers to be freed
> +			 */
> +			set_current_state(TASK_INTERRUPTIBLE);
> +			schedule_timeout(msecs_to_jiffies
> +					 (TX_BACKOFF_WEIGHT_MS));
> +		} while (!wl->close);

Why not msleep_interruptible()?

Also an ack from Ajay would be good.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v2] wifi: wilc1000: Added back-off algorithm to balance tx queue packets.

[Kalle Valo]

<Prasurjya.Rohansaikia@microchip.com> writes:

> <Prasurjya.Rohansaikia@microchip.com> writes:
>
>> From: Prasurjya Rohan Saikia <prasurjya.rohansaikia@microchip.com>
>>
>> Added an algorithm to backoff the Tx Task when low memory scenario is
>> triggered at firmware. During high data transfer from host, the firmware
>> runs out of VMM memory, which is used to hold the frames from the host.
>> So added flow control to delay the transmit from host side when there is
>> not enough space to accomodate frames in firmware side.
>>
>> Signed-off-by: Prasurjya Rohan Saikia <prasurjya.rohansaikia@microchip.com>
>
> [...]
>
>> -             } while (ret == WILC_VMM_ENTRY_FULL_RETRY && !wl->close);
>> +                     if (ret != WILC_VMM_ENTRY_FULL_RETRY)
>> +                             break;
>> +                     /* Back off from sending packets for some time.
>> +                      * schedule_timeout will allow RX task to run and free
>> +                      * buffers. Setting state to TASK_INTERRUPTIBLE will
>> +                      * put the thread back to CPU running queue when it's
>> +                      * signaled even if 'timeout' isn't elapsed. This gives
>> +                      * faster chance for reserved SK buffers to be freed
>> +                      */
>> +                     set_current_state(TASK_INTERRUPTIBLE);
>> +                     schedule_timeout(msecs_to_jiffies
>> +                                      (TX_BACKOFF_WEIGHT_MS));
>> +             } while (!wl->close);
>
> Why not msleep_interruptible()?
>
> Thanks you for your suggestion. However, I decided to proceed with
> schedule_timeout_interruptible() after testing and I will resubmit the patch. 

You need to provide more information than that.

Please quote your emails properly and don't send HTML emails, our lists
automaticall drop all HTML mail.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v2] wifi: wilc1000: Added back-off algorithm to balance tx queue packets.

[Prasurjya.Rohansaikia]

On 9/8/23 13:48, Kalle Valo wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> <Prasurjya.Rohansaikia@microchip.com> writes:
> 
>> <Prasurjya.Rohansaikia@microchip.com> writes:
>>
>>> From: Prasurjya Rohan Saikia <prasurjya.rohansaikia@microchip.com>
>>>
>>> Added an algorithm to backoff the Tx Task when low memory scenario is
>>> triggered at firmware. During high data transfer from host, the firmware
>>> runs out of VMM memory, which is used to hold the frames from the host.
>>> So added flow control to delay the transmit from host side when there is
>>> not enough space to accomodate frames in firmware side.
>>>
>>> Signed-off-by: Prasurjya Rohan Saikia <prasurjya.rohansaikia@microchip.com>
>>
>> [...]
>>
>>> -             } while (ret == WILC_VMM_ENTRY_FULL_RETRY && !wl->close);
>>> +                     if (ret != WILC_VMM_ENTRY_FULL_RETRY)
>>> +                             break;
>>> +                     /* Back off from sending packets for some time.
>>> +                      * schedule_timeout will allow RX task to run and free
>>> +                      * buffers. Setting state to TASK_INTERRUPTIBLE will
>>> +                      * put the thread back to CPU running queue when it's
>>> +                      * signaled even if 'timeout' isn't elapsed. This gives
>>> +                      * faster chance for reserved SK buffers to be freed
>>> +                      */
>>> +                     set_current_state(TASK_INTERRUPTIBLE);
>>> +                     schedule_timeout(msecs_to_jiffies
>>> +                                      (TX_BACKOFF_WEIGHT_MS));
>>> +             } while (!wl->close);
>>
>> Why not msleep_interruptible()?
>>
>> Thanks you for your suggestion. However, I decided to proceed with
>> schedule_timeout_interruptible() after testing and I will resubmit the patch.
> 
> You need to provide more information than that.
> 
> Please quote your emails properly and don't send HTML emails, our lists
> automaticall drop all HTML mail.

Ohh, somehow this configuration was missed out. I have now updated the
email client configuration.

Also, apologies I didn't provide more information earlier. There are no
specific reasons to use schedule_timeout_interruptible(). Since
schedule_timeout_interruptible() wrapped the same functionality,
I don't think there's any issue if we use msleep_interruptible() instead
as it also provides the same functionality.

I have tested with msleep_interrruptible() and its working fine too. So,
I'll be submitting a patch using it.