Re: [PATCH] USB: Fix the issue of task recovery failure caused by USB status when S4 wakes up

Alan Stern
2024-10-10T14:21:48Z

On Thu, Oct 10, 2024 at 01:59:08PM +0800, duanchenghao wrote:
> Hi Alan,

> Thank you very much for your evaluation of the scheme. I have a
> question regarding why the set_bit operation for the
> HCD_FLAG_WAKEUP_PENDING flag is performed in the top half of an
> interrupt handler, while the clear_bit operation is done in the bottom
> half. This seems to contradict conventional practices. The issue is not
> limited to S4; if other processes freeze the work queue in the bottom
> half, the same problem may arise.

The flag is treated this way because that's what it means: A wakeup is
pending. The kernel first learns about the wakeup when it receives the
wakeup interrupt from the host controller, so that's when it sets the
flag -- in the top half of the interrupt handler. The wakeup procedure
isn't complete until the root hub has been resumed, so the flag remains
set until that resume is finished, in the bottom half.

You say "the same problem may arise", but I don't think it is a problem.
If the system is about to suspend the host controller with wakeups
enabled, and a wakeup request has already been received but the system
has not yet finished acting on it, then the suspend _should_ fail.
After all, if the wakeup interrupt had arrived just after the host
controller was suspended rather than just before, it would have caused
the host controller to be resumed right away -- with exactly the same
effect as if the controller had never been suspended in the first place.

> The solution you described below should be able to resolve the current
> S4 issue, but for now, we haven't identified any other scenarios that
> require the same operation.

Perhaps because there aren't any.

> Based on my understanding, the USB device
> is woken up in the bottom half of the interrupt,

You are failing to distinguish between the host controller and the root
hub. The host controller (which is a PCI device on your system, not a
USB device) is woken up in the top half of the interrupt handler. The
root hub (which is a virtual USB device) is woken up in the bottom half.
Both operations have to occur before the wakeup can be considered fully
complete.

> and both the set_bit
> and clear_bit operations for the HCD_FLAG_WAKEUP_PENDING flag should be
> executed within the same thread in the bottom half. May I ask if there
> are any other reasons why the set_bit is executed in the top half?

Because the root hub's wakeup becomes pending when the host controller
is resumed, in the top half.

Alan Stern