RE: [PATCH] net: renesas: sh_eth: Fix freeing wrong tx descriptor

Hi Sergey,

From: Sergey Shtylyov, Sent: Wednesday, September 8, 2021 6:46 PM

On 08.09.2021 8:45, Yoshihiro Shimoda wrote:

quoted

quoted

quoted

The cur_tx counter must be incremented after TACT bit of
txdesc->status was set. However, a CPU is possible to reorder
instructions and/or memory accesses between cur_tx and
txdesc->status. And then, if TX interrupt happened at such a
timing, the sh_eth_tx_free() may free the descriptor wrongly.
So, add wmb() before cur_tx++.

    Not dma_wmb()? :-)

On armv8, dma_wmb() is DMB OSHST, and wmb() is DSB ST.
IIUC, DMB OSHST is not affected the ordering of instructions.
So, we have to use wmb().

    I should really read up the ARM manuals on the barrier instructions... :-)

:)

quoted

quoted

quoted

Otherwise NETDEV WATCHDOG timeout is possible to happen.

Fixes: 86a74ff21a7a ("net: sh_eth: add support for Renesas SuperH Ethernet")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Reviewed-by: Sergey Shtylyov <redacted>

Thank you for your review!

    Out of curiosity: have you really experienced the bug or found it by
review?

Our team has really experienced the bug when iperf3 runs on both side(server and client),
and this patch could fix the issue.

Best regards,
Yoshihiro Shimoda

quoted

Best regards,
Yoshihiro Shimoda

MBR, Sergey