Re: [net v2] net: wwan: t7xx: Fix FSM command timeout issue

[Jinjian Song]

From: Sergey Ryazanov <ryazanov.s.a@xxxxxxxxx>

> > Fixes: d785ed945de6 ("net: wwan: t7xx: PCIe reset rescan")
>
> The completion waiting was introduced in a different commit. I believe, 
> the fix tag should be 13e920d93e37 ("net: wwan: t7xx: Add core components")

Got it.

[...]
> >   	if (cmd->flag & FSM_CMD_FLAG_WAIT_FOR_COMPLETION) {
> >   		*cmd->ret = result;
>
> The memory for the result storage is allocated on the stack as well. And 
> writing it unconditionally can cause unexpected consequences.

Got it.

[...]
> >   		wait_ret = wait_for_completion_timeout(&done,
> >   						       msecs_to_jiffies(FSM_CMD_TIMEOUT_MS));
> > -		if (!wait_ret)
> > +		if (!wait_ret) {
> > +			cmd->done = NULL;
>
> We cannot access the command memory here, since fsm_finish_command() 
> could release it already.

Got it.

[...]
> Here we have an ownership transfer problem and a driver author has tried 
> to solve it, but as noticed, we are still experiencing issues in case of 
> timeout.
>
> The command completion routine should not release the command memory 
> unconditionally. Looks like the references counting approach should help 
> us here. E.g.
> 1. grab a reference before we put a command into the queue
> 1.1. grab an extra reference if we are going to wait the completion
> 2. release the reference as soon as we are done with the command execution
> 3. in case of completion waiting release the reference as soon as we are 
> done with waiting due to completion or timeout
>
> Could you try the following patch? Please note, besides the reference 
> counter introduction it also moves completion and result storage inside 
> the command structure as advised by the completion documentation.

Yes, please let me try the following patch.
[...]

Thanks.

Jinjian,
Best Regards.