Re: [PATCH v3 2/2] md/cluster: fix deadlock when doing reshape job

["heming.zhao@suse.com" <heming.zhao@suse.com>]

On 11/15/20 12:30 PM, Zhao Heming wrote:
> ... ...
> How to fix:
> 
> Use the same way of metadata_update_start. lock_comm &
> metadata_update_start are two equal users that want to get token lock.
> lock_comm could do the same steps like metadata_update_start.
> The patch moves setting MD_CLUSTER_HOLDING_MUTEX_FOR_RECVD from
> lock_token to lock_comm. It enlarge a little bit window of
> MD_CLUSTER_HOLDING_MUTEX_FOR_RECVD, but it is safe & can break deadlock
> perfectly.
> 
> At last, thanks for Xiao's solution.
> 
> 

for easy review, I paste key fix code from v3 patch.

legacy code (original upstream code):

```
static int lock_token(struct md_cluster_info *cinfo, bool mddev_locked)
{
    int error, set_bit = 0;
    struct mddev *mddev = cinfo->mddev;

    /*
     * If resync thread run after raid1d thread, then process_metadata_update
     * could not continue if raid1d held reconfig_mutex (and raid1d is blocked
     * since another node already got EX on Token and waitting the EX of Ack),
     * so let resync wake up thread in case flag is set.
     */
    if (mddev_locked && !test_bit(MD_CLUSTER_HOLDING_MUTEX_FOR_RECVD,
                      &cinfo->state)) {
        error = test_and_set_bit_lock(MD_CLUSTER_HOLDING_MUTEX_FOR_RECVD,
                          &cinfo->state);
        WARN_ON_ONCE(error);
        md_wakeup_thread(mddev->thread);
        set_bit = 1;
    }
    error = dlm_lock_sync(cinfo->token_lockres, DLM_LOCK_EX);
    if (set_bit)
        clear_bit_unlock(MD_CLUSTER_HOLDING_MUTEX_FOR_RECVD, &cinfo->state);

    if (error)
        pr_err("md-cluster(%s:%d): failed to get EX on TOKEN (%d)\n",
                __func__, __LINE__, error);

    /* Lock the receive sequence */
    mutex_lock(&cinfo->recv_mutex);
    return error;
}

/* lock_comm()
 * Sets the MD_CLUSTER_SEND_LOCK bit to lock the send channel.
 */
static int lock_comm(struct md_cluster_info *cinfo, bool mddev_locked)
{
    wait_event(cinfo->wait,
           !test_and_set_bit(MD_CLUSTER_SEND_LOCK, &cinfo->state));

    return lock_token(cinfo, mddev_locked);
}
```

v3 patch:
```
static int lock_token(struct md_cluster_info *cinfo)
{
    int error;

    error = dlm_lock_sync(cinfo->token_lockres, DLM_LOCK_EX);
    if (error) {
        pr_err("md-cluster(%s:%d): failed to get EX on TOKEN (%d)\n",
                __func__, __LINE__, error);
    } else {
        /* Lock the receive sequence */
        mutex_lock(&cinfo->recv_mutex);
    }
    return error;
}

/* lock_comm()
 * Sets the MD_CLUSTER_SEND_LOCK bit to lock the send channel.
 */
static int lock_comm(struct md_cluster_info *cinfo, bool mddev_locked)
{
    int rv, set_bit = 0;
    struct mddev *mddev = cinfo->mddev;

    /*
     * If resync thread run after raid1d thread, then process_metadata_update
     * could not continue if raid1d held reconfig_mutex (and raid1d is blocked
     * since another node already got EX on Token and waitting the EX of Ack),
     * so let resync wake up thread in case flag is set.
     */
    if (mddev_locked && !test_bit(MD_CLUSTER_HOLDING_MUTEX_FOR_RECVD,
                      &cinfo->state)) {
        rv = test_and_set_bit_lock(MD_CLUSTER_HOLDING_MUTEX_FOR_RECVD,
                          &cinfo->state);
        WARN_ON_ONCE(rv);
        md_wakeup_thread(mddev->thread);
        set_bit = 1;
    }

    wait_event(cinfo->wait,
               !test_and_set_bit(MD_CLUSTER_SEND_LOCK, &cinfo->state));
    rv = lock_token(cinfo);
    if (set_bit)
        clear_bit_unlock(MD_CLUSTER_HOLDING_MUTEX_FOR_RECVD, &cinfo->state);
    return rv;
}
```