On Thu, 4 Aug 2016, Viresh Kumar wrote:
> On 04-08-16, 11:49, Viresh Kumar wrote:
> > On 04-08-16, 14:47, Alan Stern wrote:
> > > On Thu, 4 Aug 2016, Viresh Kumar wrote:
> > >
> > > > On 04-08-16, 11:20, Alan Stern wrote:
> > > > > Yes, it could lead to this deadlock.
> > > > >
> > > > > > Should we rather have device_trylock instead of device_lock
> > > > >
> > > > > No.
> > > > >
> > > > > > > @@ -1031,10 +1031,20 @@ static void hub_activate(struct usb_hub
> > > > > > > unsigned delay;
> > > > > > >
> > > > > > > /* Continue a partial initialization */
> > > > > > > - if (type == HUB_INIT2)
> > > > > > > - goto init2;
> > > > > > > - if (type == HUB_INIT3)
> > > > > > > + if (type == HUB_INIT2 || type == HUB_INIT3) {
> > > > > > > + device_lock(hub->intfdev);
> > > > > >
> > > > > > device_trylock?? If failed to acquire then treat that as disconnect?
> > > > >
> > > > > No, that's not the right answer. I believe the correct solution is to
> > > > > remove the cancel_delayed_work_sync() call in hub_quiesce().
> > > > >
> > > > > Viresh, do you agree?
> > > >
> > > > Hmm, so I think we have two different problems here and maybe we should fix them
> > > > separately. But surely, my patch isn't enough and we need to do it the way you
> > > > suggested, i.e. let the work die by itself.
> > > >
> > > > What about another patch on top of my patch to fix the deadlock?
> > >
> > > Or another patch in place of yours to fix both problems. Has your
> > > patch been merged yet? I don't see it in any of the branches in
> > > https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git, and it's
> > > not in the current mainline.
> >
> > I don't think Greg has applied it yet. Do you want me to send the new patch?
>
> I have sent a V2 now.
Okay. I'll add my two changes on top of yours.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
