Skip to content

Commit 3664847

Browse files
committed
md/raid5: fix a race condition in stripe batch
We have a race condition in below scenario, say have 3 continuous stripes, sh1, sh2 and sh3, sh1 is the stripe_head of sh2 and sh3: CPU1 CPU2 CPU3 handle_stripe(sh3) stripe_add_to_batch_list(sh3) -> lock(sh2, sh3) -> lock batch_lock(sh1) -> add sh3 to batch_list of sh1 -> unlock batch_lock(sh1) clear_batch_ready(sh1) -> lock(sh1) and batch_lock(sh1) -> clear STRIPE_BATCH_READY for all stripes in batch_list -> unlock(sh1) and batch_lock(sh1) ->clear_batch_ready(sh3) -->test_and_clear_bit(STRIPE_BATCH_READY, sh3) --->return 0 as sh->batch == NULL -> sh3->batch_head = sh1 -> unlock (sh2, sh3) In CPU1, handle_stripe will continue handle sh3 even it's in batch stripe list of sh1. By moving sh3->batch_head assignment in to batch_lock, we make it impossible to clear STRIPE_BATCH_READY before batch_head is set. Thanks Stephane for helping debug this tricky issue. Reported-and-tested-by: Stephane Thiell <[email protected]> Cc: [email protected] (v4.1+) Signed-off-by: Shaohua Li <[email protected]>
1 parent e8a27f8 commit 3664847

File tree

1 file changed

+8
-2
lines changed

1 file changed

+8
-2
lines changed

drivers/md/raid5.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -811,15 +811,21 @@ static void stripe_add_to_batch_list(struct r5conf *conf, struct stripe_head *sh
811811
spin_unlock(&head->batch_head->batch_lock);
812812
goto unlock_out;
813813
}
814+
/*
815+
* We must assign batch_head of this stripe within the
816+
* batch_lock, otherwise clear_batch_ready of batch head
817+
* stripe could clear BATCH_READY bit of this stripe and
818+
* this stripe->batch_head doesn't get assigned, which
819+
* could confuse clear_batch_ready for this stripe
820+
*/
821+
sh->batch_head = head->batch_head;
814822

815823
/*
816824
* at this point, head's BATCH_READY could be cleared, but we
817825
* can still add the stripe to batch list
818826
*/
819827
list_add(&sh->batch_list, &head->batch_list);
820828
spin_unlock(&head->batch_head->batch_lock);
821-
822-
sh->batch_head = head->batch_head;
823829
} else {
824830
head->batch_head = head;
825831
sh->batch_head = head->batch_head;

0 commit comments

Comments
 (0)