Re: [PATCH v2 3/3] md-cluster: fix ack_lockres leak in __sendmsg error path

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The newly added error recovery logic silently fails if the original error was a timeout, leaving the lock leaked in EX state and defeating the patch's purpose.

Pre-existing issues:
- [High] An infinite loop and CPU soft lockup occur in the `failed_ack` cleanup path if a previous DLM lock operation timed out.
--

commit aed4dde7a766dd41cf7dc88c61e2ae9c8fd7a612
Author: Cao Guanghui <[email protected]>
    md-cluster: fix ack_lockres leak in __sendmsg error path

In __sendmsg(), if the down-conversion of ack_lockres from EX to CR fails, the
original code jumps to failed_ack and leaves ack_lockres in the EX state. This
commit attempts to fix this by adding a conversion of ack_lockres to CR in the
failed_ack path.

> diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
> index bc4118d8c5a9f..7c6aa759d0c23 100644
> --- a/drivers/md/md-cluster.c
> +++ b/drivers/md/md-cluster.c
> @@ -801,6 +801,8 @@ static int __sendmsg(struct md_cluster_info *cinfo, struct cluster_msg *cmsg)
>  	}
>  
>  failed_ack:
> +	if (error)
> +		dlm_lock_sync(cinfo->ack_lockres, DLM_LOCK_CR);

[Severity: High]
Will this error recovery logic silently fail if the original error was a
timeout?

If dlm_lock_sync() times out when acquiring DLM_LOCK_EX for ack_lockres
earlier in the function, it leaves a pending DLM lock request in the
background. Because of the uncancelled pending request, calling
dlm_lock_sync(cinfo->ack_lockres, DLM_LOCK_CR) here immediately fails with
-EBUSY and its return value is ignored.

When the background EX request eventually completes, the lock is left in the EX
state, which could still lead to a cluster-wide deadlock.

>  	while ((unlock_error = dlm_unlock_sync(cinfo->message_lockres)))
>  		pr_err("md-cluster: failed convert to NL on MESSAGE(%d)\n",
>  			unlock_error);

[Severity: High]
This is a pre-existing issue, but does this create an infinite loop and CPU
soft lockup if a previous DLM lock operation timed out?

If a timeout occurs during dlm_lock_sync(cinfo->message_lockres, DLM_LOCK_CW)
earlier in __sendmsg(), it leaves a pending lock conversion request in the DLM
lockspace. 

Because of the pending request, dlm_unlock_sync() will immediately return
-EBUSY without blocking, causing this to become an infinite loop that floods
the kernel log and soft lockups the CPU core.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3