| Message ID | 1459352551-11773-3-git-send-email-rmanohar@qti.qualcomm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On 03/30/2016 08:42 AM, Rajkumar Manoharan wrote: > Decrement num_mpdus_ready only when rx amsdu is processed successfully. > Not doing so, will result in leak and impact stabilty under low memory > cases. Should this patch be rebased on top of the "ath10k: process htt rx indication as batch mode" patch? Thanks, Ben
On 2016-04-01 00:22, Ben Greear wrote: > On 03/30/2016 08:42 AM, Rajkumar Manoharan wrote: >> Decrement num_mpdus_ready only when rx amsdu is processed >> successfully. >> Not doing so, will result in leak and impact stabilty under low memory >> cases. > > Should this patch be rebased on top of the "ath10k: process htt rx > indication as batch mode" patch? > It should be on top of "ath10k: speedup htt rx descriptor processing for rx_ind". Instead of sending next version of original series, i sent it as follow up. -Rajkumar
Rajkumar Manoharan <rmanohar@codeaurora.org> writes: > On 2016-04-01 00:22, Ben Greear wrote: >> On 03/30/2016 08:42 AM, Rajkumar Manoharan wrote: >>> Decrement num_mpdus_ready only when rx amsdu is processed >>> successfully. >>> Not doing so, will result in leak and impact stabilty under low memory >>> cases. >> >> Should this patch be rebased on top of the "ath10k: process htt rx >> indication as batch mode" patch? >> > It should be on top of "ath10k: speedup htt rx descriptor processing > for rx_ind". Instead of sending next version of original series, i > sent it as follow up. Should this commit log have a fixes line like this: Fixes: 59465fe46ef1 ("ath10k: speedup htt rx descriptor processing for tx completion")
Rajkumar Manoharan <rmanohar@qti.qualcomm.com> writes: > Decrement num_mpdus_ready only when rx amsdu is processed successfully. > Not doing so, will result in leak and impact stabilty under low memory > cases. > > Signed-off-by: Rajkumar Manoharan <rmanohar@qti.qualcomm.com> > --- > drivers/net/wireless/ath/ath10k/htt_rx.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c > index 96a7417..9696c2e 100644 > --- a/drivers/net/wireless/ath/ath10k/htt_rx.c > +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c > @@ -2412,14 +2412,12 @@ static void ath10k_htt_txrx_compl_task(unsigned long ptr) > struct ath10k_htt *htt = (struct ath10k_htt *)ptr; > struct ath10k *ar = htt->ar; > struct htt_tx_done tx_done = {}; > - struct sk_buff_head rx_q; > struct sk_buff_head rx_ind_q; > struct sk_buff_head tx_ind_q; > struct sk_buff *skb; > unsigned long flags; > int num_mpdus; > > - __skb_queue_head_init(&rx_q); > __skb_queue_head_init(&rx_ind_q); > __skb_queue_head_init(&tx_ind_q); I guess you are removing the unused rx_q just as a cleanup? It's good practise to mention that in the commit log (or better yet in a separate patch).
On 2016-04-05 18:18, Valo, Kalle wrote: > Rajkumar Manoharan <rmanohar@qti.qualcomm.com> writes: > >> Decrement num_mpdus_ready only when rx amsdu is processed >> successfully. >> Not doing so, will result in leak and impact stabilty under low memory >> cases. >> >> Signed-off-by: Rajkumar Manoharan <rmanohar@qti.qualcomm.com> >> --- >> drivers/net/wireless/ath/ath10k/htt_rx.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c >> b/drivers/net/wireless/ath/ath10k/htt_rx.c >> index 96a7417..9696c2e 100644 >> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c >> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c >> @@ -2412,14 +2412,12 @@ static void >> ath10k_htt_txrx_compl_task(unsigned long ptr) >> struct ath10k_htt *htt = (struct ath10k_htt *)ptr; >> struct ath10k *ar = htt->ar; >> struct htt_tx_done tx_done = {}; >> - struct sk_buff_head rx_q; >> struct sk_buff_head rx_ind_q; >> struct sk_buff_head tx_ind_q; >> struct sk_buff *skb; >> unsigned long flags; >> int num_mpdus; >> >> - __skb_queue_head_init(&rx_q); >> __skb_queue_head_init(&rx_ind_q); >> __skb_queue_head_init(&tx_ind_q); > > I guess you are removing the unused rx_q just as a cleanup? It's good > practise to mention that in the commit log (or better yet in a separate > patch). update the commit log and send next version. -Rajkumar
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c index 96a7417..9696c2e 100644 --- a/drivers/net/wireless/ath/ath10k/htt_rx.c +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c @@ -2412,14 +2412,12 @@ static void ath10k_htt_txrx_compl_task(unsigned long ptr) struct ath10k_htt *htt = (struct ath10k_htt *)ptr; struct ath10k *ar = htt->ar; struct htt_tx_done tx_done = {}; - struct sk_buff_head rx_q; struct sk_buff_head rx_ind_q; struct sk_buff_head tx_ind_q; struct sk_buff *skb; unsigned long flags; int num_mpdus; - __skb_queue_head_init(&rx_q); __skb_queue_head_init(&rx_ind_q); __skb_queue_head_init(&tx_ind_q); @@ -2448,11 +2446,13 @@ static void ath10k_htt_txrx_compl_task(unsigned long ptr) ath10k_mac_tx_push_pending(ar); num_mpdus = atomic_read(&htt->num_mpdus_ready); - atomic_sub(num_mpdus, &htt->num_mpdus_ready); - while (num_mpdus--) { + while (num_mpdus) { if (ath10k_htt_rx_handle_amsdu(htt)) break; + + num_mpdus--; + atomic_dec(&htt->num_mpdus_ready); } while ((skb = __skb_dequeue(&rx_ind_q))) {
Decrement num_mpdus_ready only when rx amsdu is processed successfully. Not doing so, will result in leak and impact stabilty under low memory cases. Signed-off-by: Rajkumar Manoharan <rmanohar@qti.qualcomm.com> --- drivers/net/wireless/ath/ath10k/htt_rx.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)