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 -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Attachment:
signature.asc
Description: OpenPGP digital signature
