Re: [PATCH next] usb: usbfs: Add reset_resume for usbfs

[Hongyu Xie <xy521521@gmail.com>]

From: Hongyu Xie <xiehongyu1@kylinos.cn>



On 2024/7/16 20:44, Oliver Neukum wrote:
> On 15.07.24 16:22, Alan Stern wrote:
>> On Mon, Jul 15, 2024 at 10:53:14AM +0200, Oliver Neukum wrote:
>>>
>>>
>>> On 11.07.24 16:41, Alan Stern wrote:
> 
>>>> Agreed, but the solution is pretty simple.  Because the device was
>>>> suspended, the userspace driver must have enabled suspend via the
>>>> USBDEVFS_ALLOW_SUSPEND ioctl.
>>>
>>> The whole system could have been suspended, in particularly to S4.
>>
>> You are right.  I was thinking of runtime suspend, not system suspend.
>> My mistake.
> 
> This is at the intersection of several scenarios. That is a good part of
> what makes this difficult.
> In principal I think we could get away with checking for a flag to be set
> at reset_resume() before each operation. Elegant this is not. Yet, it seems
> to me like the race necessarily exists and is unsolvable in user space.
> 

 From what I know, that CONFIG_USB_DEFAULT_PERSIST is enabled by 
default. Then udev->persist_enabled is set to 1 and this causing 
udev->reset_resume set to 1 during init2 in hub_activate.
During resume,
usb_resume_both
   usb_resume_device
     generic_resume
       usb_port_resume
         finish_port_resume
           usb_reset_and_verify_device (if udev->reset_resume is 1)
             hub_port_init
               hub_port_reset
   usb_resume_interface (udev->reset_resume is 1 but usbfs doesn't have 
reset_resume implementation so set intf->needs_binding to 1, and it will 
be rebind in usb_resume_complete)

Even before usbfs->reset_resume is called (if there is one), the USB 
device has already been reset and in a good state.

After all that thaw_processes is called and user-space application runs 
again.

So I still don't understand why "the race necessarily exists". Can you 
show me an example that usbfs->reset_resume causes race?

> Furthermore in the long run, if we want to use D3cold in runtime power
> management, it looks to me like we would want a second permission ioctl
> for that.
> 
>      Regards
>          Oliver
> 
Regards
Hongyu Xie