Thanks for quick response. I'll send a second version of the patch if that's OK :). BR, Alejandro On 11/11/20 22:48, Marc Kleine-Budde wrote: > On 11/6/20 6:33 PM, Alejandro wrote: >> 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), > Yes, but it doesn't export it's symbols. I've removed the function from the > header file and everything still compiles. > >> 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. > I don't see a valid usecase where can_restart_now() is called from an ISR. We > can change this if an uscase pops up later. > > regards, > Marc > >
