Re: Possible race condition in f_mass_storage gadget during deinitialization. Kernel warning issued

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

 



Hi,

Adrian Ambrożewicz <adrian.ambrozewicz@xxxxxxxxx> writes:
> Hello,
>
> I'm consistently observing kernel warnings related to Mass Storage USB
> Gadget de-initialization flow. After investigation I believe that I've
> found root cause of these warnings, however I'm unable to estimate the
> impact. I would like to put the issue into discussion about possible
> side-effects.
>
> My usage model is to emulate file system using f_mass_storage gadget.
> Whenever I pull the USB cable out I see warning related to
> "dwc2_hsotg_init_fifo" which is commented in the following way: "/*
> Reset fifo map if not correctly cleared during previous session */".

which kernel are you using? Have you tried latest mainline? (currently
at v4.18).

> Assumption I've made were the following:
> 1) disconnect handler notifies all gadgets with call_gadget(.., disconnect)
> 2) gadgets are then responsible to clear the ep fifos with usb_ep_disable()
> 3) disconnect handler initializes the fifo maps to clear state
>
> Unfortunately in my case the warning occurs every time when using
> f_mass_storage gadget. By comparison with f_hid gadget I've come up
> with conclusion that it's caused by race condition in the way the
> "disable" flow is implemented in mass storage.
>
> "Correct" flow implemented by HID gadget is the following:
> * dwc2_hsotg_irq (USBRst)
> ** dwc2_hsotg_disconnect
> *** call_gadget(disconnect)
> **** hidg_disable
> ***** usb_ep_disable // fifos utilized by device are cleared in fifo_map
> ** dwc_hsotg_core_init_disconnected
> *** dwc2_hsotg_init_fifo // fifo_map is empty - no warning here
>
> By comparison here is flow observed in Mass Storage gadget. Race
> condition is introduced by utilizing separate worker thread to handle
> events:
> THREAD #1
> * dwc2_hsotg_irq (USBRst)
> ** dwc2_hsotg_disconnect
> *** call_gadget(disconnect)
> **** fsg_disable()
> ***** raise_exception(FSG_STATE_CONFIG_CHANGE) // Race between Thread
> #1 and Thread #2 starts here.
> ** dwc_hsotg_core_init_disconnected
> *** dwc2_hsotg_init_fifo // fifo_map is object of the race between
> this function, and call to usb_ep_disable()
>
> THREAD #2
> * handle_exception(FSG_STATE_CONFIG_CHANGE)
> ** do_set_interface(NULL)
> *** usb_ep_disable()
>
> My questions are the following:
> - is this known issue?
> - is it risky? What are possible outcomes?
> - If this race condition is dangerous what would be proper fix ?
> Should fsg_disable() call be blocked until handle_exception() finishes
> the cleanup? I know that it's just a WA for "misaligned" Mass Storage
> gadget architecture, but are there alternatives?
>
> I would really appreciate professional insight on that matter,

Let's see if Minas has anything to say about this.

-- 
balbi



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

  Powered by Linux