On Sat, 3 Sep 2016, Felipe Balbi wrote:

> Hi,
> 
> Alan Stern <st...@rowland.harvard.edu> writes:
> > On Fri, 2 Sep 2016, Felipe Balbi wrote:
> >
> >> 
> >> Hi,
> >> 
> >> Alan Stern <st...@rowland.harvard.edu> writes:
> >> 
> >> [...]
> >> 
> >> > No, that would be too late.  The barrier needs to go between the write 
> >> > to common->thead_wakeup_needed and the call to wake_up_process().  
> >> 
> >> alright, I tested this but it doesn't work. It still hangs :-(
> >
> > Okay, I was wrong.  But see my recent reply to Paul; maybe the 
> > smp_wmb() in wakeup_thread() needs to be smp_mb().
> 
> okay, this seems to be working so far:
> 
> modified   drivers/usb/gadget/function/f_mass_storage.c
> @@ -395,7 +395,7 @@ static int fsg_set_halt(struct fsg_dev *fsg, struct 
> usb_ep *ep)
>  /* Caller must hold fsg->lock */
>  static void wakeup_thread(struct fsg_common *common)
>  {
> -     smp_wmb();      /* ensure the write of bh->state is complete */
> +     smp_mb();       /* ensure the write of bh->state is complete */
>       /* Tell the main thread that something has happened */
>       common->thread_wakeup_needed = 1;
>       if (common->thread_task)
> @@ -626,7 +626,7 @@ static int sleep_thread(struct fsg_common *common, bool 
> can_freeze)
>       }
>       __set_current_state(TASK_RUNNING);
>       common->thread_wakeup_needed = 0;
> -     smp_rmb();      /* ensure the latest bh->state is visible */
> +     smp_mb();       /* ensure the latest bh->state is visible */
>       return rc;
>  }
>  
> I'll keep it running over the weekend.

Can you also check the question I mentioned earlier?  That is, if you
revert to the original code then when the thread hangs, what does the
call to received_cbw() in get_next_command() return?  Or does that
routine not get called at all because the preceding sleep_thread() is
the one that doesn't return?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to