mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2024-08-27 19:29:37 +00:00
md/cluster: fix deadlock when node is doing resync job
commitbca5b06580
upstream. md-cluster uses MD_CLUSTER_SEND_LOCK to make node can exclusively send msg. During sending msg, node can concurrently receive msg from another node. When node does resync job, grab token_lockres:EX may trigger a deadlock: ``` nodeA nodeB -------------------- -------------------- a. send METADATA_UPDATED held token_lockres:EX b. md_do_sync resync_info_update send RESYNCING + set MD_CLUSTER_SEND_LOCK + wait for holding token_lockres:EX c. mdadm /dev/md0 --remove /dev/sdg + held reconfig_mutex + send REMOVE + wait_event(MD_CLUSTER_SEND_LOCK) d. recv_daemon //METADATA_UPDATED from A process_metadata_update + (mddev_trylock(mddev) || MD_CLUSTER_HOLDING_MUTEX_FOR_RECVD) //this time, both return false forever ``` Explaination: a. A send METADATA_UPDATED This will block another node to send msg b. B does sync jobs, which will send RESYNCING at intervals. This will be block for holding token_lockres:EX lock. c. B do "mdadm --remove", which will send REMOVE. This will be blocked by step <b>: MD_CLUSTER_SEND_LOCK is 1. d. B recv METADATA_UPDATED msg, which send from A in step <a>. This will be blocked by step <c>: holding mddev lock, it makes wait_event can't hold mddev lock. (btw, MD_CLUSTER_HOLDING_MUTEX_FOR_RECVD keep ZERO in this scenario.) There is a similar deadlock in commit0ba959774e
("md-cluster: use sync way to handle METADATA_UPDATED msg") In that commit, step c is "update sb". This patch step c is "mdadm --remove". For fixing this issue, we can refer the solution of function: metadata_update_start. Which does the same grab lock_token action. lock_comm can use the same steps to avoid deadlock. By moving 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. Repro steps (I only triggered 3 times with hundreds tests): two nodes share 3 iSCSI luns: sdg/sdh/sdi. Each lun size is 1GB. ``` ssh root@node2 "mdadm -S --scan" mdadm -S --scan for i in {g,h,i};do dd if=/dev/zero of=/dev/sd$i oflag=direct bs=1M \ count=20; done mdadm -C /dev/md0 -b clustered -e 1.2 -n 2 -l mirror /dev/sdg /dev/sdh \ --bitmap-chunk=1M ssh root@node2 "mdadm -A /dev/md0 /dev/sdg /dev/sdh" sleep 5 mkfs.xfs /dev/md0 mdadm --manage --add /dev/md0 /dev/sdi mdadm --wait /dev/md0 mdadm --grow --raid-devices=3 /dev/md0 mdadm /dev/md0 --fail /dev/sdg mdadm /dev/md0 --remove /dev/sdg mdadm --grow --raid-devices=2 /dev/md0 ``` test script will hung when executing "mdadm --remove". ``` # dump stacks by "echo t > /proc/sysrq-trigger" md0_cluster_rec D 0 5329 2 0x80004000 Call Trace: __schedule+0x1f6/0x560 ? _cond_resched+0x2d/0x40 ? schedule+0x4a/0xb0 ? process_metadata_update.isra.0+0xdb/0x140 [md_cluster] ? wait_woken+0x80/0x80 ? process_recvd_msg+0x113/0x1d0 [md_cluster] ? recv_daemon+0x9e/0x120 [md_cluster] ? md_thread+0x94/0x160 [md_mod] ? wait_woken+0x80/0x80 ? md_congested+0x30/0x30 [md_mod] ? kthread+0x115/0x140 ? __kthread_bind_mask+0x60/0x60 ? ret_from_fork+0x1f/0x40 mdadm D 0 5423 1 0x00004004 Call Trace: __schedule+0x1f6/0x560 ? __schedule+0x1fe/0x560 ? schedule+0x4a/0xb0 ? lock_comm.isra.0+0x7b/0xb0 [md_cluster] ? wait_woken+0x80/0x80 ? remove_disk+0x4f/0x90 [md_cluster] ? hot_remove_disk+0xb1/0x1b0 [md_mod] ? md_ioctl+0x50c/0xba0 [md_mod] ? wait_woken+0x80/0x80 ? blkdev_ioctl+0xa2/0x2a0 ? block_ioctl+0x39/0x40 ? ksys_ioctl+0x82/0xc0 ? __x64_sys_ioctl+0x16/0x20 ? do_syscall_64+0x5f/0x150 ? entry_SYSCALL_64_after_hwframe+0x44/0xa9 md0_resync D 0 5425 2 0x80004000 Call Trace: __schedule+0x1f6/0x560 ? schedule+0x4a/0xb0 ? dlm_lock_sync+0xa1/0xd0 [md_cluster] ? wait_woken+0x80/0x80 ? lock_token+0x2d/0x90 [md_cluster] ? resync_info_update+0x95/0x100 [md_cluster] ? raid1_sync_request+0x7d3/0xa40 [raid1] ? md_do_sync.cold+0x737/0xc8f [md_mod] ? md_thread+0x94/0x160 [md_mod] ? md_congested+0x30/0x30 [md_mod] ? kthread+0x115/0x140 ? __kthread_bind_mask+0x60/0x60 ? ret_from_fork+0x1f/0x40 ``` At last, thanks for Xiao's solution. Cc: stable@vger.kernel.org Signed-off-by: Zhao Heming <heming.zhao@suse.com> Suggested-by: Xiao Ni <xni@redhat.com> Reviewed-by: Xiao Ni <xni@redhat.com> Signed-off-by: Song Liu <songliubraving@fb.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
7523d14708
commit
2fb550de75
2 changed files with 42 additions and 31 deletions
|
@ -664,9 +664,27 @@ static void recv_daemon(struct md_thread *thread)
|
|||
* Takes the lock on the TOKEN lock resource so no other
|
||||
* node can communicate while the operation is underway.
|
||||
*/
|
||||
static int lock_token(struct md_cluster_info *cinfo, bool mddev_locked)
|
||||
static int lock_token(struct md_cluster_info *cinfo)
|
||||
{
|
||||
int error, set_bit = 0;
|
||||
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;
|
||||
|
||||
/*
|
||||
|
@ -677,34 +695,19 @@ static int lock_token(struct md_cluster_info *cinfo, bool mddev_locked)
|
|||
*/
|
||||
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,
|
||||
rv = test_and_set_bit_lock(MD_CLUSTER_HOLDING_MUTEX_FOR_RECVD,
|
||||
&cinfo->state);
|
||||
WARN_ON_ONCE(error);
|
||||
WARN_ON_ONCE(rv);
|
||||
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);
|
||||
rv = lock_token(cinfo);
|
||||
if (set_bit)
|
||||
clear_bit_unlock(MD_CLUSTER_HOLDING_MUTEX_FOR_RECVD, &cinfo->state);
|
||||
return rv;
|
||||
}
|
||||
|
||||
static void unlock_comm(struct md_cluster_info *cinfo)
|
||||
|
@ -784,9 +787,11 @@ static int sendmsg(struct md_cluster_info *cinfo, struct cluster_msg *cmsg,
|
|||
{
|
||||
int ret;
|
||||
|
||||
lock_comm(cinfo, mddev_locked);
|
||||
ret = __sendmsg(cinfo, cmsg);
|
||||
unlock_comm(cinfo);
|
||||
ret = lock_comm(cinfo, mddev_locked);
|
||||
if (!ret) {
|
||||
ret = __sendmsg(cinfo, cmsg);
|
||||
unlock_comm(cinfo);
|
||||
}
|
||||
return ret;
|
||||
}
|
||||
|
||||
|
@ -1061,7 +1066,7 @@ static int metadata_update_start(struct mddev *mddev)
|
|||
return 0;
|
||||
}
|
||||
|
||||
ret = lock_token(cinfo, 1);
|
||||
ret = lock_token(cinfo);
|
||||
clear_bit_unlock(MD_CLUSTER_HOLDING_MUTEX_FOR_RECVD, &cinfo->state);
|
||||
return ret;
|
||||
}
|
||||
|
@ -1255,7 +1260,10 @@ static void update_size(struct mddev *mddev, sector_t old_dev_sectors)
|
|||
int raid_slot = -1;
|
||||
|
||||
md_update_sb(mddev, 1);
|
||||
lock_comm(cinfo, 1);
|
||||
if (lock_comm(cinfo, 1)) {
|
||||
pr_err("%s: lock_comm failed\n", __func__);
|
||||
return;
|
||||
}
|
||||
|
||||
memset(&cmsg, 0, sizeof(cmsg));
|
||||
cmsg.type = cpu_to_le32(METADATA_UPDATED);
|
||||
|
@ -1407,7 +1415,8 @@ static int add_new_disk(struct mddev *mddev, struct md_rdev *rdev)
|
|||
cmsg.type = cpu_to_le32(NEWDISK);
|
||||
memcpy(cmsg.uuid, uuid, 16);
|
||||
cmsg.raid_slot = cpu_to_le32(rdev->desc_nr);
|
||||
lock_comm(cinfo, 1);
|
||||
if (lock_comm(cinfo, 1))
|
||||
return -EAGAIN;
|
||||
ret = __sendmsg(cinfo, &cmsg);
|
||||
if (ret) {
|
||||
unlock_comm(cinfo);
|
||||
|
|
|
@ -6721,8 +6721,10 @@ static int hot_remove_disk(struct mddev *mddev, dev_t dev)
|
|||
goto busy;
|
||||
|
||||
kick_rdev:
|
||||
if (mddev_is_clustered(mddev))
|
||||
md_cluster_ops->remove_disk(mddev, rdev);
|
||||
if (mddev_is_clustered(mddev)) {
|
||||
if (md_cluster_ops->remove_disk(mddev, rdev))
|
||||
goto busy;
|
||||
}
|
||||
|
||||
md_kick_rdev_from_array(rdev);
|
||||
set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
|
||||
|
|
Loading…
Reference in a new issue