On 26/11/2024 02:05, Sergey Ryazanov wrote:
Hi Antonio,
the question was addressed to Sabrina, but since I've already touched this topic in the another patch, let me put my 2c here.
On 16.11.2024 02:33, Antonio Quartulli wrote:
On 31/10/2024 16:25, Sabrina Dubroca wrote:
2024-10-29, 11:47:25 +0100, Antonio Quartulli wrote:
+static void ovpn_socket_release_work(struct work_struct *work) +{ + struct ovpn_socket *sock = container_of(work, struct ovpn_socket, work);
+ ovpn_socket_detach(sock->sock); + kfree_rcu(sock, rcu); +}
+static void ovpn_socket_schedule_release(struct ovpn_socket *sock) +{ + INIT_WORK(&sock->work, ovpn_socket_release_work); + schedule_work(&sock->work);
How does module unloading know that it has to wait for this work to complete? Will ovpn_cleanup get stuck until some refcount gets released by this work?
No, we have no such mechanism. Any idea how other modules do?
Actually this makes me wonder how module unloading coordinates with the code being executed. Unload may happen at any time - how do we prevent killing the code in the middle of something (regardless of scheduled workers)?
Right question! There is a workqueue flushing API intended for synchronization with work(s) execution.
Here, the system workqueue was used, so technically a flush_scheduled_work() call somewhere in the module_exit handler would be enough.
On another hand, flushing the system workqueue considered a not so good practice. It's recommended to use a local workqueue. You can find a good example of switching from the system to a local workqueue in cc271ab86606 ("wwan_hwsim: Avoid flush_scheduled_work() usage").
And if the workqueue is definitely empty at a time of module unloading, e.g. due to flushing on netdev removing, there no requirement to flush it again.
ACK. I wanted to avoid using a local workqueue, but if we have pending work that needs flushing I indeed see no other way.
Regards,
-- Sergey