On 11/18/21 5:18 AM, Heikki Krogerus wrote:
Hi,
On Thu, Nov 18, 2021 at 05:23:52PM +0800, Xu Yang wrote:
@@ -6428,6 +6432,9 @@ void tcpm_unregister_port(struct tcpm_port *port)
{
int i;
You need to take the port lock here, no?
mutex_lock(&port->lock);
+ kthread_destroy_worker(port->wq);
+ port->wq = NULL;
mutex_unlock(&port->lock);
I don't think the timer code runs under the lock, so that won't help.
But, yes, the code is inherently racy. At the very least, port->wq
would have to be set to NULL first, but even that would have to be
synchronized. I wonder how other drivers handle that situation.
Guenter
hrtimer_cancel(&port->send_discover_timer);
hrtimer_cancel(&port->enable_frs_timer);
hrtimer_cancel(&port->vdm_state_machine_timer);
@@ -6439,7 +6446,6 @@ void tcpm_unregister_port(struct tcpm_port *port)
typec_unregister_port(port->typec_port);
usb_role_switch_put(port->role_sw);
tcpm_debugfs_exit(port);
- kthread_destroy_worker(port->wq);
}
EXPORT_SYMBOL_GPL(tcpm_unregister_port);
thanks,
