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

[Alan Stern]

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