On 6/11/20 11:25, Marc Kleine-Budde wrote: > On 11/5/20 10:51 PM, Alejandro wrote: >> From: Alejandro Concepcion Rodriguez<alejandro@xxxxxxxx> >> >> netif_rx() is meant to be called from interrupt contexts. can_restart() >> may be called by can_restart_work(), which is called from a worqueue, so >> it may run in process context. Use netif_rx_any_context() which invokes >> the correct code path depending on context. >> >> Co-developed-by: Loris Fauster<loris.fauster@xxxxxxxxxxxxx> >> Signed-off-by: Loris Fauster<loris.fauster@xxxxxxxxxxxxx> >> Signed-off-by: Alejandro Concepcion Rodriguez<alejandro@xxxxxxxx> > I think we either call can_restart() from a netlink callback via > can_restart_now() or via the can_restart_work(). So we should always use > netif_rx_ni(skb), right? Right, I think that currently it is as you say. However, it seems that can_restart_now() has public visibility (/linux/can/dev.h), and even though it doesn't seem to be used by other CAN drivers for now, I guess it could potentially be used in future. netif_rx_any_context() should avoid issues if can_restart_now() is called from an ISR later. > >> --- >> drivers/net/can/dev.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c >> index b70ded3760f2..83114f8e8c24 100644 >> --- a/drivers/net/can/dev.c >> +++ b/drivers/net/can/dev.c >> @@ -584,7 +584,7 @@ static void can_restart(struct net_device *dev) >> >> cf->can_id |= CAN_ERR_RESTARTED; >> >> - netif_rx(skb); >> + netif_rx_any_context(skb); >> >> stats->rx_packets++; >> stats->rx_bytes += cf->can_dlc; >> > Marc > BR, Alejandro
