Re: [PATCH] SUNRPC: Set tk_rpc_status when RPC_TASK_SIGNALLED is detected

[yangerkun]

在 2025/1/17 4:52, Trond Myklebust 写道:
> On Thu, 2025-01-16 at 19:43 +0800, yangerkun wrote:
> > Hi,
> >
> > Thanks for the patch.
> >
> > Before 39494194f93b("SUNRPC: Fix races with rpc_killall_tasks()",
> > every
> > time we set RPC_TASK_SIGNALLED, when we go through __rpc_execute,
> > this
> > rpc_task will immediate break and exist.
> >
> > However after that, __rpc_execute won't judge RPC_TASK_SIGNNALED, so
> > for
> > the case like you point out below, even after your commit
> > rpc_check_timeout will help break and exist eventually, but this
> > rpc_task has already do some work. I prefer reintroduce judging
> > RPC_TASK_SIGNNALED in __rpc_execute to help exist immediatly.
>
> Better yet... Let's get rid of the RPC_TASK_SIGNALLED flag altogether
> and just replace
>
> #define RPC_TASK_SIGNALLED(task) (READ_ONCE(task->tk_rpc_status) == -ERESTARTSYS)

Hi,

Thanks for your reply! Yeah, if all the places where tk_rpc_status is
updated are by calling rpc_task_set_rpc_status, we can use
task->tk_rpc_status == -ERESTARTSYS to determine whether rpc_task is
RPC_TASK_SIGNALLED. But for the case like Li has provided,
__rpc_restart_call won't do this, and will overwrite tk_rpc_status 
unconditionally. This won't be a stable solution. Maybe it's better to
change __rpc_restart_call calling rpc_task_set_rpc_status too? And
__rpc_execute will be enough to help solve this case.

> There is no good reason to try to maintain two completely separate
> sources of truth to describe the same task state.