| Message ID | 20230816162259.33342-1-prasurjya.rohansaikia@microchip.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] wifi: wilc1000: Added back-off algorithm to balance tx queue packets. | expand |
<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.
<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.
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.
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;