[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <20241216135322.4533-1-jinjian.song@fibocom.com>
Date: Mon, 16 Dec 2024 21:53:22 +0800
From: Jinjian Song <jinjian.song@...ocom.com>
To: ryazanov.s.a@...il.com,
Jinjian Song <jinjian.song@...ocom.com>,
chandrashekar.devegowda@...el.com,
chiranjeevi.rapolu@...ux.intel.com,
haijun.liu@...iatek.com,
ricardo.martinez@...ux.intel.com,
loic.poulain@...aro.org,
johannes@...solutions.net,
davem@...emloft.net,
edumazet@...gle.com,
kuba@...nel.org,
pabeni@...hat.com
Cc: andrew+netdev@...n.ch,
angelogioacchino.delregno@...labora.com,
corbet@....net,
danielwinkler@...gle.com,
helgaas@...nel.org,
horms@...nel.org,
korneld@...gle.com,
linux-arm-kernel@...ts.infradead.org,
linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org,
linux-mediatek@...ts.infradead.org,
matthias.bgg@...il.com,
netdev@...r.kernel.org
Subject: Re: [net v2] net: wwan: t7xx: Fix FSM command timeout issue
From: Sergey Ryazanov <ryazanov.s.a@...il.com>
>> 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.
Powered by blists - more mailing lists