Re: [PATCH v3 2/2] md/cluster: fix deadlock when doing reshape job
From: heming.zhao@suse.com <hidden>
Date: 2020-11-15 04:39:55
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;
}