Re: [PATCH v2] usbip: convert to use faux_device

[Zongmin Zhou <min_halo@163.com>]

On 2025/6/20 12:29, Greg KH wrote:
> On Fri, Jun 20, 2025 at 10:16:16AM +0800, Zongmin Zhou wrote:
>> On 2025/6/19 19:01, Greg KH wrote:
>>> On Wed, Jun 04, 2025 at 02:54:10PM +0800, Zongmin Zhou wrote:
>>>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>
>>>> The vhci driver does not need to create a platform device,
>>>> it only did so because it was simple to do that in order to
>>>> get a place in sysfs to hang some device-specific attributes.
>>>> Now the faux device interface is more appropriate,change it
>>>> over to use the faux bus instead.
>>>>
>>>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>> ---
>>>> Changes in v2:
>>>> - don't change faux create api,just call probe on vhci_hcd_init.
>>>>
>>>>    drivers/usb/usbip/vhci.h             |  4 +-
>>>>    drivers/usb/usbip/vhci_hcd.c         | 86 +++++++++++-----------------
>>>>    drivers/usb/usbip/vhci_sysfs.c       | 68 +++++++++++-----------
>>>>    tools/usb/usbip/libsrc/vhci_driver.h |  2 +-
>>>>    4 files changed, 72 insertions(+), 88 deletions(-)
>>> I get the following build errors from this patch:
>>>
>>> drivers/usb/usbip/vhci_hcd.c:1462:12: error: ‘vhci_hcd_resume’ defined but not used [-Werror=unused-function]
>>>    1462 | static int vhci_hcd_resume(struct faux_device *fdev)
>>>         |            ^~~~~~~~~~~~~~~
>>> drivers/usb/usbip/vhci_hcd.c:1418:12: error: ‘vhci_hcd_suspend’ defined but not used [-Werror=unused-function]
>>>    1418 | static int vhci_hcd_suspend(struct faux_device *fdev, pm_message_t state)
>>>         |            ^~~~~~~~~~~~~~~~
>>> cc1: all warnings being treated as errors
>>>
>>> Are you sure you tested this?
>> I apologize for not enabling -Werror, which resulted in missing this error
>> warning.
>> I have tested usbip feature use the new patch,but not test system
>> suspend/resume.
>> The faux bus type don't add pm function,and vhci-hcd driver can't register
>> it.
>> Maybe have to add suspend/resume for it.like below:
>> static const struct bus_type faux_bus_type = {
>>      .name        = "faux",
>>      .match        = faux_match,
>>      .probe        = faux_probe,
>>      .remove        = faux_remove,
>>      .resume     = faux_resume,
>>      .suspend    = faux_suspend,
>> };
>>
>> Is that right?
>> Your expertise would be greatly valued.
> As this is not real hardware, why do you need the suspend/resume
> callbacks at all?  What is happening here that requires them?
@greg,
The vhci_hcd_suspend/vhci_hcd_resume interfaces are not designed for 
this faux device, but rather to
manipulate the HCD_FLAG_HW_ACCESSIBLE bit in the hcd flags associated 
with the faux device.
For example:
During system standby: clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
During system wakeup: set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)

Previously, these two functions were registered through platform_driver,
but faux bus does not have the relevant interface, so they were not called,
resulting in this compilation warning error.

This raises the question: Should the faux bus implement PM-related 
interface?
I'm uncertain whether these functions are essential for the vhci-hcd 
driver or if they can be safely removed.

However, during system standby/wakeup tests with remote USB devices 
bound to the vhci-hcd driver,
I observed consistent failure scenarios across both the original 
platform bus and faux bus patch implementations.

Failure Modes
a.Failed standby with auto-wakeup(Log excerpt):
[ 1449.065592][T10238] PM: suspend entry (s2idle)
[ 1449.106146][T10238] Filesystems sync: 0.040 seconds
[ 1449.216189][T10238] Freezing user space processes
[ 1449.219474][T10238] Freezing user space processes completed (elapsed 
0.002 seconds)
[ 1449.219887][T10238] OOM killer disabled.
[ 1449.220090][T10238] Freezing remaining freezable tasks
[ 1469.222372][T10238] Freezing remaining freezable tasks failed after 
20.002 seconds (0 tasks refusing to freeze, wq_busy=1):
[ 1469.225038][T10238] Showing freezable workqueues that are still busy:
[ 1469.226176][T10238] workqueue events_freezable_pwr_efficient: flags=0x86
[ 1469.227453][T10238]   pwq 20: cpus=0-3 node=0 flags=0x4 nice=0 
active=1 refcnt=2
[ 1469.227463][T10238]     in-flight: 268:disk_events_workfn
[ 1469.233559][T10238] Restarting kernel threads ... done.
[ 1469.235119][T10238] OOM killer enabled.
[ 1469.235849][T10238] Restarting tasks ... done.
[ 1469.240121][T10238] random: crng reseeded on system resumption
[ 1469.241176][T10238] PM: suspend exit

b.Failed standby with black screen freeze:
[ 1820.667073][T11454] PM: suspend entry (s2idle)

@Shuah，
I wonder if you has encountered this issue? When a USB device is 
attached to vhci-hcd,
is it not possible to put the system into standby mode?

Thanks.

> thanks,
>
> greg k-h