Hello!
On 29.06.2020 8:24, Yoshihiro Shimoda wrote:
From: Yoshihiro Shimoda, Sent: Tuesday, May 26, 2020 6:47 PM
According to the report of [1], this driver is possible to cause
the following error in ravb_tx_timeout_work().
ravb e6800000.ethernet ethernet: failed to switch device to config mode
This error means that the hardware could not change the state
from "Operation" to "Configuration" while some tx queue is operating.
After that, ravb_config() in ravb_dmac_init() will fail, and then
any descriptors will be not allocaled anymore so that NULL porinter
dereference happens after that on ravb_start_xmit().
Such a case is possible to be caused because this driver supports
two queues (NC and BE) and the ravb_stop_dma() is possible to return
without any stopping process if TCCR or CSR register indicates
the hardware is operating for TX.
To fix the issue, just try to wake the subqueue on
ravb_tx_timeout_work() if the descriptors are not full instead
of stop all transfers (all queues of TX and RX).
[1]
https://lore.kernel.org/linux-renesas-soc/20200518045452.2390-1-dirk.behme@xxxxxxxxxxxx/
Reported-by: Dirk Behme <dirk.behme@xxxxxxxxxxxx>
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
---
I'm guessing that this issue is possible to happen if:
- ravb_start_xmit() calls netif_stop_subqueue(), and
- ravb_poll() will not be called with some reason, and
- netif_wake_subqueue() will be not called, and then
- dev_watchdog() in net/sched/sch_generic.c calls ndo_tx_timeout().
However, unfortunately, I didn't reproduce the issue yet.
To be honest, I'm also guessing other queues (SR) of this hardware
which out-of tree driver manages are possible to reproduce this issue,
but I didn't try such environment for now...
So, I marked RFC on this patch now.
I'm afraid, but do you have any comments about this patch?
I agree that we should now reset only the stuck queue, not both but I
doubt your solution is good enough. Let me have another look...
Thank you for your comment! I hope this solution is good enough...
I'm sorry again and again. But, do you have any time to look this patch?
Yes, in the sense of reviewing -- I don't consider it complete. And no, in
the sense of looking into the issue myself... Can we do a per-queue tear-down
and re-init (not necessarily all in 1 patch)?
Best regards,
Yoshihiro Shimoda
MBR, Sergei
