On Thu, Nov 18, 2021 at 03:00:16PM +0000, Xu Yang wrote:
> Hi,
>
> > -----Original Message-----
> > From: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> > Sent: Thursday, November 18, 2021 9:18 PM
> > To: Xu Yang <xu.yang_2@xxxxxxx>
> > Cc: linux@xxxxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; linux-
> > usb@xxxxxxxxxxxxxxx; Jun Li <jun.li@xxxxxxx>; dl-linux-imx <linux-
> > imx@xxxxxxx>
> > Subject: [EXT] Re: [PATCH] usb: typec: tcpm: fix tcpm unregister port but
> > leave a pending timer
> >
> > Caution: EXT Email
> >
> > 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 think we should not take the port lock before kthread_destroy_worker() since
> a deadlock might occur. Considering a work is pending and tcpm_unregister_port
> is called at this time, the worker needs to flush all the works after taking
> the port lock in tcpm_unregister_port(). However, the work can't take the port
> lock anymore.
The point is that you create a race with that code. If the port lock
is not useful in this case, there needs to be something else.
thanks,
--
heikki
