From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-m2835.qiye.163.com (mail-m2835.qiye.163.com [103.74.28.35]) by mail19.linbit.com (LINBIT Mail Daemon) with ESMTP id 224BE420625 for ; Wed, 15 Sep 2021 13:07:50 +0200 (CEST) From: Rui Xu To: philipp.reisner@linbit.com, drbd-dev@lists.linbit.com, joel.colledge@linbit.com Date: Wed, 15 Sep 2021 19:07:43 +0800 Message-Id: <20210915110743.1271706-1-rui.xu@easystack.cn> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Cc: Rui Xu , dongsheng.yang@easystack.cn Subject: [Drbd-dev] [PATCH v3] drbd: fix a race condition in update_sync_bits() and receive_bitmap() List-Id: "*Coordination* of development, patches, contributions -- *Questions* \(even to developers\) go to drbd-user, please." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , There was a race condition involving update_sync_bits() and receive_bitmap(). Consider this scenario: Primary: node-3, Secondary: node-1, node-2 (1) Network failure occurs on node-1 (2) node-1 network recovers (3) node-1 connects to node-2, and starts resync (node-1 is SyncTarget, node-2 is SyncSource) (4) Before resync in (3) finishes, node-1 connects to node-3 and starts resync (node-1 is PausedSyncT, node-3 is PausedSyncS) The following sequence can occur on node-1 while it is syncing from node-2: * ack_receiver thread processes P_PEERS_IN_SYNC * ack_receiver: call update_sync_bits() * ack_receiver: clear the last bitmap bits for node-3 * ack_receiver: set rs_is_done to 1 * receiver thread processes P_*BITMAP * receiver: call receive_bitmap() * receiver: set bitmap bits for node-3 * receiver: set the repl_state towards node-3 to PausedSyncT * ack_receiver: set RS_DONE flag This causes the resync from node-3 to node-1 to finish even though bitmap bits are still set, and node-1 will finish the resync as PausedSyncT, but the resync from node-2 to node-1 is still ongoing, at last, it will finish as a unstable resync, and leaving the disk state end into INCONSISTENT finally. Fix this by evaluating is_sync_target_state before getting the bitmap total weight in update_sync_bits. Signed-off-by: Rui Xu Signed-off-by: Joel Colledge --- changelog: -From v1: fix typo in commit message -From v2: add a brief comment to the code and added the bug description in commit message drbd/drbd_actlog.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/drbd/drbd_actlog.c b/drbd/drbd_actlog.c index 841e5149..01e64823 100644 --- a/drbd/drbd_actlog.c +++ b/drbd/drbd_actlog.c @@ -1044,11 +1044,11 @@ static bool lazy_bitmap_update_due(struct drbd_peer_device *peer_device) } static void maybe_schedule_on_disk_bitmap_update(struct drbd_peer_device *peer_device, - bool rs_done) + bool rs_done, bool is_sync_target) { if (rs_done) { if (peer_device->connection->agreed_pro_version <= 95 || - is_sync_target_state(peer_device, NOW)) + is_sync_target) set_bit(RS_DONE, &peer_device->flags); /* If sync source: rather wait for explicit notification via @@ -1105,11 +1105,18 @@ static int update_sync_bits(struct drbd_peer_device *peer_device, } if (count) { if (mode == SET_IN_SYNC) { + /* Evaluate is_sync_target_state before getting the bm total + * weight to avoid a scenario: still_to_go is 0 when sync + * state is not sync_target_state, but sync state change to + * sync_target_state later and finish the resync even though + * bitmap bits are still set. + */ + bool is_sync_target = is_sync_target_state(peer_device, NOW); unsigned long still_to_go = drbd_bm_total_weight(peer_device); bool rs_is_done = (still_to_go <= peer_device->rs_failed); drbd_advance_rs_marks(peer_device, still_to_go); if (cleared || rs_is_done) - maybe_schedule_on_disk_bitmap_update(peer_device, rs_is_done); + maybe_schedule_on_disk_bitmap_update(peer_device, rs_is_done, is_sync_target); } else if (mode == RECORD_RS_FAILED) { peer_device->rs_failed += count; } else /* if (mode == SET_OUT_OF_SYNC) */ { -- 2.25.1