>On Fri, 3 Jun 2016, Chung-Geol Kim wrote:
>
>> Yes, you are right, The presentational errors in order to obtain an 
>> understanding of the process.
>> Therefore, I will be happy to explain again the diagrammatic representation 
>> as shown below.
>> If using usb 3.0 storage(OTG), you can see as below.
>> 
>> ==============================================
>>     At *Insert USB(3.0) Storage
>>     sequence <1> --> <5>
>> ==============================================
>>                                 VOLD       
>> =================================|============
>>                              (uevent)
>>                            ______|___________ 
>>                           |<5>               |
>>                           |      SCSI        |
>>                           |usb_get_hcd       |
>>                           |shared_hcd(kref=3)|
>>                           |__________________|
>>    ___________________     ________|_________ 
>>   |<2>                |   |<4>               |
>>   |dwc3_otg_sm_work   |   |dwc3_otg_sm_work  |
>>   |usb_get_hcd        |   |usb_get_hcd       |
>>   |primary_hcd(kref=2)|   |shared_hcd(kref=2)|
>>   |___________________|   |__________________|
>>    _________|_________     ________|_________ 
>>   |<1>                |   |<3>               |
>>   |New USB BUS #1     |   |New USB BUS #2    |
>>   |usb_create_hcd     |   |usb_create_hcd    |
>>   |primary_hcd(kref=1)|   |shared_hcd(kref=1)|
>>   |                   |   |                  |
>>   |bandXX_mutex(alloc)|<-(Link)-bandXX_mutex |
>>   |___________________|   |__________________|
>> 
>
>When people present diagrams like this, the universal convention is to 
>show earlier times at the top and later times at the bottom.  That way 
>the order in which you read the diagram is the same as the order in 
>which the events are supposed to occur.
>
>Also, the convention is to put events next to each other if they happen 
>at the same time.  In your diagram, <1> and <3> are next to each other 
>but they don't happen at the same time.

Thank you for your kind explanation. It has been modified as shown below.
=================================================
     At *Insert USB(3.0) Storage
     sequence <1> --> <5>
=================================================
   ___________________
  |<1>                |
  |New USB BUS #1     |
  |usb_create_hcd     |
  |primary_hcd(kref=1)|
  |                   |
  |bandXX_mutex(alloc)|<--
  |___________________|  |
   _________|_________   |
  |<2>                |  |
  |dwc3_otg_sm_work   |  |
  |usb_get_hcd        |  |
  |primary_hcd(kref=2)|  |
  |___________________|  |
                         |  __________________
                         | |<3>               |
                         | |New USB BUS #2    |
                         | |usb_create_hcd    |
                         | |shared_hcd(kref=1)|
                         | |                  |
                         --(Link)-bandXX_mutex|
                           |__________________|
                            ________|_________ 
                           |<4>               |
                           |dwc3_otg_sm_work  |
                           |usb_get_hcd       |
                           |shared_hcd(kref=2)|
                           |__________________|
                            _______|__________ 
                           |<5>               |
                           |      SCSI        |
                           |usb_get_hcd       |
                           |shared_hcd(kref=3)|
                           |__________________|
                                    |
                                (uevent)
-----------------------------------|------------
                                 VOLD
=================================================


=================================================
     At *remove USB(3.0) Storage
     sequence <1> --> <5> ((Normal Case))
=================================================
                                   VOLD
------------------------------------|------------
                                (uevent)
                              ______|___________
                             |<1>               |
                             |      SCSI        |
                             |usb_put_hcd       |
                             |shared_hcd(kref=2)|
                             |__________________|
                              ________|_________ 
                             |<2>               |
                             |dwc3_otg_sm_work  |
                             |usb_put_hcd       |
                             |shared_hcd(kref=1)|
                             |__________________|
                              ________|_________ 
                             |<3>               |
                             |New USB BUS #2    |
                             |hcd_release       |
                             |shared_hcd(kref=0)|
                             |                  |
                          -X-(cut off)-bandXX_mutex|
                           | |__________________|
                           |
      ___________________  |
     |<4>                | |
     |dwc3_otg_sm_work   | |
     |usb_put_hcd        | |
     |primary_hcd(kref=1)| |
     |___________________| |
      _________|_________  |
     |<5>                | |
     |New USB BUS #1     | |
     |hcd_release        | |
     |primary_hcd(kref=0)| |
     |                   | |
     |bandXX_mutex(free) |<-
     |___________________| 

=================================================

>
>> ==============================================
>>     At *remove USB(3.0) Storage 
>>     sequence <1> --> <5> ((Normal Case))
>> ==============================================
>>                                 VOLD       
>> =================================|============
>>                              (uevent)
>>                            ______|___________ 
>>                           |<1>               |
>>                           |      SCSI        |
>>                           |usb_put_hcd       |
>>                           |shared_hcd(kref=2)|
>>                           |__________________|
>>    ___________________     ________|_________ 
>>   |<4>                |   |<2>               |
>>   |dwc3_otg_sm_work   |   |dwc3_otg_sm_work  |
>>   |usb_put_hcd        |   |usb_put_hcd       |
>>   |primary_hcd(kref=1)|   |shared_hcd(kref=1)|
>>   |___________________|   |__________________|
>>    _________|_________     ________|_________ 
>>   |<5>                |   |<3>               |
>>   |New USB BUS #1     |   |New USB BUS #2    |
>>   |hcd_release        |   |hcd_release       |
>>   |primary_hcd(kref=0)|   |shared_hcd(kref=0)|
>>   |                   |   |                  |
>>   |bandXX_mutex(free) | -X-cut off)-bandXX_mutex|
>>   |___________________|   |__________________|
>> 
>> ----------------------------------------------
>
>> >The real bug here is that the shared_hcd is released after the 
>> >primary_hcd.  That's what you need to fix.
>> 
>> NO, It 's only depend on vold(scsi) release time.
>> If the vold later released and is being released first hcd,
>> Double free happened at <5> as below.
>> 
>> ==============================================
>>     At *remove USB(3.0) Storage 
>>     sequence <1> --> <5> ((Problem Case))
>> ==============================================
>>                                 VOLD       
>> =================================|============
>>                              (uevent)
>>                            ______|___________ 
>>                           |<5>               |
>>                           |      SCSI        |
>>                           |usb_put_hcd       |
>>                           |shared_hcd(kref=0)|
>>                           |*hcd_release      |
>>                           |bandXX_mutex(free*)|<- double free
>>                           |__________________|
>>    ___________________     ________|_________ 
>>   |<3>                |   |<1>               |
>>   |dwc3_otg_sm_work   |   |dwc3_otg_sm_work  |
>>   |usb_put_hcd        |   |usb_put_hcd       |
>>   |primary_hcd(kref=1)|   |shared_hcd(kref=2)|
>>   |___________________|   |__________________|
>>    _________|_________     ________|_________ 
>>   |<4>                |   |<2>               |
>>   |New USB BUS #1     |   |New USB BUS #2    |
>>   |hcd_release        |   |                  |
>>   |primary_hcd(kref=0)|   |shared_hcd(kref=1)|
>>   |                   |   |                  |
>>   |bandXX_mutex(free) |<-(Link)-bandXX_mutex |
>>   |___________________|   |__________________|
>> 
>> ----------------------------------------------
>
>That's the same as what I said.  In your diagram, vold releases the 
>shared_hcd (in <5>) after dwc3_otg_sm_work releases the primary_hcd 
>(in <4>).
>


=================================================
     At *remove USB(3.0) Storage 
     sequence <1> --> <5> ((Problem Case))
=================================================
                                  VOLD
------------------------------------|------------
                                 (uevent)
                            ________|_________
                           |<1>               |
                           |dwc3_otg_sm_work  |
                           |usb_put_hcd       |
                           |shared_hcd(kref=2)|
                           |__________________|
                            ________|_________ 
                           |<2>               |
                           |New USB BUS #2    |
                           |                  |
                           |shared_hcd(kref=1)|
                           |                  |
                         --(Link)-bandXX_mutex|
                         | |__________________|
                         | 
    ___________________  |
   |<3>                | |
   |dwc3_otg_sm_work   | |
   |usb_put_hcd        | |
   |primary_hcd(kref=1)| |
   |___________________| |
    _________|_________  |
   |<4>                | |
   |New USB BUS #1     | |
   |hcd_release        | |
   |primary_hcd(kref=0)| |
   |                   | |
   |bandXX_mutex(free) |<-
   |___________________| 
                               (( VOLD ))
                            ______|___________
                           |<5>               |
                           |      SCSI        |
                           |usb_put_hcd       |
                           |shared_hcd(kref=0)|
                           |*hcd_release      |
                           |bandXX_mutex(free*)|<- double free
                           |__________________|

=================================================

>If you change the code so that the shared_hcd takes a reference to the 
>primary_hcd and drops that reference when the shared_hcd is released, 
>this will never occur.
>
>Alan Stern

Reply via email to