Skip to content

Commit 16d56e2

Browse files
committed
md/raid1: fix writebehind bio clone
After bio is submitted, we should not clone it as its bi_iter might be invalid by driver. This is the case of behind_master_bio. In certain situration, we could dispatch behind_master_bio immediately for the first disk and then clone it for other disks. https://bugzilla.kernel.org/show_bug.cgi?id=196383 Reported-and-tested-by: Markus <[email protected]> Reviewed-by: Ming Lei <[email protected]> Fix: 841c131(md: raid1: improve write behind) Cc: [email protected] (4.12+) Signed-off-by: Shaohua Li <[email protected]>
1 parent be453e7 commit 16d56e2

File tree

1 file changed

+13
-21
lines changed

1 file changed

+13
-21
lines changed

drivers/md/raid1.c

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -484,10 +484,6 @@ static void raid1_end_write_request(struct bio *bio)
484484
}
485485

486486
if (behind) {
487-
/* we release behind master bio when all write are done */
488-
if (r1_bio->behind_master_bio == bio)
489-
to_put = NULL;
490-
491487
if (test_bit(WriteMostly, &rdev->flags))
492488
atomic_dec(&r1_bio->behind_remaining);
493489

@@ -1080,7 +1076,7 @@ static void unfreeze_array(struct r1conf *conf)
10801076
wake_up(&conf->wait_barrier);
10811077
}
10821078

1083-
static struct bio *alloc_behind_master_bio(struct r1bio *r1_bio,
1079+
static void alloc_behind_master_bio(struct r1bio *r1_bio,
10841080
struct bio *bio)
10851081
{
10861082
int size = bio->bi_iter.bi_size;
@@ -1090,11 +1086,13 @@ static struct bio *alloc_behind_master_bio(struct r1bio *r1_bio,
10901086

10911087
behind_bio = bio_alloc_mddev(GFP_NOIO, vcnt, r1_bio->mddev);
10921088
if (!behind_bio)
1093-
goto fail;
1089+
return;
10941090

10951091
/* discard op, we don't support writezero/writesame yet */
1096-
if (!bio_has_data(bio))
1092+
if (!bio_has_data(bio)) {
1093+
behind_bio->bi_iter.bi_size = size;
10971094
goto skip_copy;
1095+
}
10981096

10991097
while (i < vcnt && size) {
11001098
struct page *page;
@@ -1115,14 +1113,13 @@ static struct bio *alloc_behind_master_bio(struct r1bio *r1_bio,
11151113
r1_bio->behind_master_bio = behind_bio;;
11161114
set_bit(R1BIO_BehindIO, &r1_bio->state);
11171115

1118-
return behind_bio;
1116+
return;
11191117

11201118
free_pages:
11211119
pr_debug("%dB behind alloc failed, doing sync I/O\n",
11221120
bio->bi_iter.bi_size);
11231121
bio_free_pages(behind_bio);
1124-
fail:
1125-
return behind_bio;
1122+
bio_put(behind_bio);
11261123
}
11271124

11281125
struct raid1_plug_cb {
@@ -1475,7 +1472,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
14751472
(atomic_read(&bitmap->behind_writes)
14761473
< mddev->bitmap_info.max_write_behind) &&
14771474
!waitqueue_active(&bitmap->behind_wait)) {
1478-
mbio = alloc_behind_master_bio(r1_bio, bio);
1475+
alloc_behind_master_bio(r1_bio, bio);
14791476
}
14801477

14811478
bitmap_startwrite(bitmap, r1_bio->sector,
@@ -1485,14 +1482,11 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
14851482
first_clone = 0;
14861483
}
14871484

1488-
if (!mbio) {
1489-
if (r1_bio->behind_master_bio)
1490-
mbio = bio_clone_fast(r1_bio->behind_master_bio,
1491-
GFP_NOIO,
1492-
mddev->bio_set);
1493-
else
1494-
mbio = bio_clone_fast(bio, GFP_NOIO, mddev->bio_set);
1495-
}
1485+
if (r1_bio->behind_master_bio)
1486+
mbio = bio_clone_fast(r1_bio->behind_master_bio,
1487+
GFP_NOIO, mddev->bio_set);
1488+
else
1489+
mbio = bio_clone_fast(bio, GFP_NOIO, mddev->bio_set);
14961490

14971491
if (r1_bio->behind_master_bio) {
14981492
if (test_bit(WriteMostly, &conf->mirrors[i].rdev->flags))
@@ -2346,8 +2340,6 @@ static int narrow_write_error(struct r1bio *r1_bio, int i)
23462340
wbio = bio_clone_fast(r1_bio->behind_master_bio,
23472341
GFP_NOIO,
23482342
mddev->bio_set);
2349-
/* We really need a _all clone */
2350-
wbio->bi_iter = (struct bvec_iter){ 0 };
23512343
} else {
23522344
wbio = bio_clone_fast(r1_bio->master_bio, GFP_NOIO,
23532345
mddev->bio_set);

0 commit comments

Comments
 (0)