[PATCH net 4/5] be2net: wait longer to reap TX compls in be_close()

From: Vasundhara Volam <redacted>

be_close() currently waits for a max of 200ms to receive all pending
TX compls. This timeout value was roughly calcuated based on 10G
transmission speeds and the TX queue depth. This timeout may not be
enough when the link is operating at lower speeds or in
multi-channel/SR-IOV configs with TX-rate limiting setting.
Increase the timeout to 2000ms and also bail-out if there is
a HW-error.

Signed-off-by: Vasundhara Volam <redacted>
Signed-off-by: Sathya Perla <redacted>
Signed-off-by: Somnath Kotur <redacted>
---
 drivers/net/ethernet/emulex/benet/be_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index a9da6f9..f6a4481 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c

@@ -1985,7 +1985,7 @@ static void be_tx_compl_clean(struct be_adapter *adapter)
 				pending_txqs--;
 		}
 
-		if (pending_txqs == 0 || ++timeo > 200)
+		if (pending_txqs == 0 || ++timeo > 2000 || be_hw_error(adapter))
 			break;
 
 		mdelay(1);

-- 
1.5.6.1

From: Of Somnath Kotur

be_close() currently waits for a max of 200ms to receive all pending
TX compls. This timeout value was roughly calcuated based on 10G
transmission speeds and the TX queue depth. This timeout may not be
enough when the link is operating at lower speeds or in
multi-channel/SR-IOV configs with TX-rate limiting setting.
Increase the timeout to 2000ms and also bail-out if there is
a HW-error.

What about monitoring whether transmits are actually completing,
and giving up if none are completed within a suitable time period?

	David

-----Original Message-----
From: David Laight [mailto:David.Laight@ACULAB.COM]

From: Of Somnath Kotur

quoted

be_close() currently waits for a max of 200ms to receive all pending
TX compls. This timeout value was roughly calcuated based on 10G
transmission speeds and the TX queue depth. This timeout may not be
enough when the link is operating at lower speeds or in
multi-channel/SR-IOV configs with TX-rate limiting setting.
Increase the timeout to 2000ms and also bail-out if there is
a HW-error.

What about monitoring whether transmits are actually completing,
and giving up if none are completed within a suitable time period?

David, the code already does that. All the queues are checked for
any new TX compls each 1ms in the be_close()->be_tx_compl_clean()
path. This patch is not changing that logic.

thanks,
-Sathya

From: Sathya Perla 

quoted

From: David Laight [mailto:David.Laight@ACULAB.COM]

From: Of Somnath Kotur

quoted

be_close() currently waits for a max of 200ms to receive all pending
TX compls. This timeout value was roughly calcuated based on 10G
transmission speeds and the TX queue depth. This timeout may not be
enough when the link is operating at lower speeds or in
multi-channel/SR-IOV configs with TX-rate limiting setting.
Increase the timeout to 2000ms and also bail-out if there is
a HW-error.

What about monitoring whether transmits are actually completing,
and giving up if none are completed within a suitable time period?

David, the code already does that. All the queues are checked for
any new TX compls each 1ms in the be_close()->be_tx_compl_clean()
path. This patch is not changing that logic.

Yes, but it might be better to modify the logic rather than just
increase the timeout.

The code is waiting for any pending transmits to complete.
The purpose of the timeout is to give up if the transmits aren't going
to complete.
So instead of assuming that a specific interval is long enough, abort
if no transmits completed in (say) a 10ms period.
Move the sleep() to the top of the loop and abort if nothing is found
on any queue.
Oh and move the zeroing of cmpl and num_wrbs to a sensible place.

	David

-----Original Message-----
From: David Laight [mailto:David.Laight@ACULAB.COM]

quoted

From: Sathya Perla

quoted

From: David Laight [mailto:David.Laight@ACULAB.COM]

From: Of Somnath Kotur

quoted

be_close() currently waits for a max of 200ms to receive all pending
TX compls. This timeout value was roughly calcuated based on 10G
transmission speeds and the TX queue depth. This timeout may not be
enough when the link is operating at lower speeds or in
multi-channel/SR-IOV configs with TX-rate limiting setting.
Increase the timeout to 2000ms and also bail-out if there is
a HW-error.

What about monitoring whether transmits are actually completing,
and giving up if none are completed within a suitable time period?

David, the code already does that. All the queues are checked for
any new TX compls each 1ms in the be_close()->be_tx_compl_clean()
path. This patch is not changing that logic.

Yes, but it might be better to modify the logic rather than just
increase the timeout.

The code is waiting for any pending transmits to complete.
The purpose of the timeout is to give up if the transmits aren't going
to complete.
So instead of assuming that a specific interval is long enough, abort
if no transmits completed in (say) a 10ms period.
Move the sleep() to the top of the loop and abort if nothing is found
on any queue.
Oh and move the zeroing of cmpl and num_wrbs to a sensible place.

So you're saying, keep polling on completions till the HW has been
completely silent for a particular (you mention 10ms) period of time.

This does sound like a nicer scheme. Thanks!
Will implement this and re-post the patch.

-Sathya