mbox series

[0/2] ravb and sh_eth: Fix descriptor counters' conditions

Message ID 20210727082147.270734-1-yoshihiro.shimoda.uh@renesas.com (mailing list archive)
Headers show
Series ravb and sh_eth: Fix descriptor counters' conditions | expand

Message

Yoshihiro Shimoda July 27, 2021, 8:21 a.m. UTC
When we change the type of {cur,dirty}_tx from u32 to u8, we can
reproduce this issue easily.

Yoshihiro Shimoda (2):
  ravb: Fix descriptor counters' conditions
  sh_eth: Fix descriptor counters' conditions

 drivers/net/ethernet/renesas/ravb_main.c | 22 +++++++++++++++-------
 drivers/net/ethernet/renesas/sh_eth.c    | 16 ++++++++++++----
 2 files changed, 27 insertions(+), 11 deletions(-)

Comments

Yoshihiro Shimoda July 29, 2021, 4:59 a.m. UTC | #1
Hi all,

> From: Yoshihiro Shimoda, Sent: Tuesday, July 27, 2021 5:22 PM
> 
> When we change the type of {cur,dirty}_tx from u32 to u8, we can
> reproduce this issue easily.

I'm afraid but I would like to recall this patch series because
the original code didn't have any issue around these counters
so that the patch series should not be applied.

For just a record, I tried to write why I misunderstood these counters are incorrect below:
- I got a report locally about an issue happen when the system sends/receives data in long time transfer
- So, I doubted these counters' overflow.
- To reproduce the issue quickly, I changed the type from u32 to u8.
- And then, the following condition will not work correctly (as I mentioned on the patch).

       for (; priv->cur_tx[q] - priv->dirty_tx[q] > 0; priv->dirty_tx[q]++) { --- [1]

- Also, I found if we used "> 0U" instead of "0", the issue disappeared.

       for (; priv->cur_tx[q] - priv->dirty_tx[q] > 0U; priv->dirty_tx[q]++) { --- [2]
                                                   ~~
- However, today I got a report from our team member locally. The object code of
  the get_num_desc() is just "from - subtract".

- If we use u32 as the original code, I realized the object codes between [1] and [2]
  are the same.
- Also, I realized just priv->cur_tx[q] - priv->dirty_tx[q] is no problem.
- So, I would like to recall this patch series.

Best regards,
Yoshihiro Shimoda