Re: g_mass_storage not queueing requests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> writes:
> On Sat, 3 Sep 2016, Felipe Balbi wrote:
>
>> Hi,
>> 
>> Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> writes:
>> > On Fri, 2 Sep 2016, Felipe Balbi wrote:
>> >
>> >> 
>> >> Hi,
>> >> 
>> >> Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> 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?

here are the last few calls

> [   34.122210] ==> bulk_out_complete 484

CBW received

> [   34.122213] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1

set thread_wakeup_needed to 1 and wakeup fsg_main_thread

> [   34.122255] ==> fsg_main_thread 2587 -> get_next_command() -> 0

get_next_command()

> [   34.122328] ==> fsg_main_thread 2608 -> send_status -> 0

CSW

> [   34.122331] ==> get_next_command 2251 -> state != full

now waiting for CBW:

> [   34.122333] ===> sleep_thread 634: caller fsg_main_thread+0xaa1/0xb00 [usb_f_mass_storage]: going to sleep

thread goes to sleep

> [   34.122379] ==> bulk_in_complete 461

We send data to host

> [   34.122381] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1
> [   34.122383] ==> get_next_command 2251 -> state != full

still waiting for CBW

> [   34.122385] ===> sleep_thread 634: caller fsg_main_thread+0xaa1/0xb00 [usb_f_mass_storage]: going to sleep
> [   34.122386] ==> bulk_in_complete 461

send data to host

> [   34.122388] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1
> [   34.122389] ==> get_next_command 2251 -> state != full

still waiting for CBW

> [   34.122391] ===> sleep_thread 634: caller fsg_main_thread+0xaa1/0xb00 [usb_f_mass_storage]: going to sleep
> [   34.122468] ==> bulk_in_complete 461
> [   34.122469] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1
> [   34.122470] ==> get_next_command 2251 -> state != full
> [   34.122473] ===> sleep_thread 634: caller fsg_main_thread+0xaa1/0xb00 [usb_f_mass_storage]: going to sleep
> [   34.122473] ==> bulk_in_complete 461
> [   34.122474] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1
> [   34.122476] ==> get_next_command 2251 -> state != full
> [   34.122479] ===> sleep_thread 634: caller fsg_main_thread+0xaa1/0xb00 [usb_f_mass_storage]: going to sleep
> [   34.122556] ==> bulk_in_complete 461
> [   34.122557] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1
> [   34.122559] ==> get_next_command 2251 -> state != full
> [   34.122561] ===> sleep_thread 634: caller fsg_main_thread+0xaa1/0xb00 [usb_f_mass_storage]: going to sleep
> [   34.122561] ==> bulk_in_complete 461
> [   34.122563] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1
> [   34.122563] ==> get_next_command 2251 -> state != full
> [   34.122565] ===> sleep_thread 634: caller fsg_main_thread+0xaa1/0xb00 [usb_f_mass_storage]: going to sleep
> [   34.122647] ==> bulk_in_complete 461
> [   34.122648] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1
> [   34.122650] ==> get_next_command 2251 -> state != full
> [   34.122653] ===> sleep_thread 634: caller fsg_main_thread+0xaa1/0xb00 [usb_f_mass_storage]: going to sleep
> [   34.122654] ==> bulk_in_complete 461
> [   34.122655] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1
> [   34.122656] ==> get_next_command 2251 -> state != full
> [   34.122658] ===> sleep_thread 634: caller fsg_main_thread+0xaa1/0xb00 [usb_f_mass_storage]: going to sleep
> [   34.122716] ==> bulk_in_complete 461
> [   34.122717] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1

yada, yada, yada. All the other transfers. If you count all
bulk_in_complete prints, you'll see there are 9 of them. 8x 16kiB +
13-byte CSW.

> [   34.122719] ==> bulk_out_complete 484
> [   34.122720] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1

another CBW

> [   34.122721] ==> fsg_main_thread 2587 -> get_next_command() -> 0
> [   34.122802] ==> fsg_main_thread 2608 -> send_status -> 0
> [   34.122805] ==> get_next_command 2251 -> state != full
> [   34.122807] ===> sleep_thread 634: caller fsg_main_thread+0xaa1/0xb00 [usb_f_mass_storage]: going to sleep
> [   34.122850] ==> bulk_in_complete 461
> [   34.122851] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1
> [   34.122855] ==> get_next_command 2251 -> state != full
> [   34.122856] ==> bulk_in_complete 461
> [   34.122857] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1
> [   34.122858] ==> get_next_command 2251 -> state != full
> [   34.122860] ===> sleep_thread 634: caller fsg_main_thread+0xaa1/0xb00 [usb_f_mass_storage]: going to sleep
> [   34.122939] ==> bulk_in_complete 461
> [   34.122940] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1
> [   34.122945] ==> bulk_in_complete 461
> [   34.122947] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1
> [   34.122988] ==> get_next_command 2251 -> state != full
> [   34.123008] ===> sleep_thread 634: caller fsg_main_thread+0xaa1/0xb00 [usb_f_mass_storage]: going to sleep
> [   34.123055] ==> bulk_in_complete 461
> [   34.123070] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1
> [   34.123073] ==> bulk_in_complete 461
> [   34.123074] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1
> [   34.123105] ==> get_next_command 2251 -> state != full
> [   34.123125] ===> sleep_thread 634: caller fsg_main_thread+0xaa1/0xb00 [usb_f_mass_storage]: going to sleep
> [   34.123167] ==> bulk_in_complete 461
> [   34.123169] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1
> [   34.123179] ==> bulk_in_complete 461
> [   34.123181] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1
> [   34.123185] ==> get_next_command 2251 -> state != full
> [   34.123202] ===> sleep_thread 634: caller fsg_main_thread+0xaa1/0xb00 [usb_f_mass_storage]: going to sleep
> [   34.123279] ==> bulk_in_complete 461
> [   34.123280] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1

Another block of 128kiB

> [   34.123281] ==> bulk_out_complete 484

received CBW. Now, bh->state is set to BUF_STATE_FULL, and ...

> [   34.123283] ==> wakeup_thread 403: caller usb_gadget_giveback_request+0x7/0x10: thread_wakeup_needed = 1

wakeup_thread() is called, however ...

> [   34.123283] ==> get_next_command 2251 -> state != full

get_next_command() still sees bh->state != BUF_STATE_FULL.

> [   34.123286] ===> sleep_thread 634: caller fsg_main_thread+0xaa1/0xb00 [usb_f_mass_storage]: going to sleep
> [   34.123289] ===> sleep_thread 634: caller fsg_main_thread+0xaa1/0xb00 [usb_f_mass_storage]: going to sleep

so it goes to sleep and never wakes up again.

-- 
balbi

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux