[PATCH] md/raid1: protect sequential read hints for read balance

"Chen Cheng" <[email protected]>
Newsgroups gmane.linux.raid,gmane.linux.kernel
Message-ID <[email protected]>
From: Chen Cheng <[email protected]>

The patch just suppress KCSAN noise. No functional change.

KCSAN reports a race, point to update_read_sectors() update next_seq_sect vs.
read next_seq_sect.

Protect next_seq_sect and seq_start with READ_ONCE/WRITE_ONCE, otherwise,
read balance see stale sequential-read hints.

KCSAN report:
==============
 BUG: KCSAN: data-race in raid1_read_request / raid1_read_request

 write to 0xffff8e3a2d6736d0 of 8 bytes by task 593784 on cpu 10:
  raid1_read_request+0xe5a/0x19f0
  raid1_make_request+0xdf/0x1990
  md_handle_request+0x4a2/0xa40
  [...]

 read to 0xffff8e3a2d6736d0 of 8 bytes by task 593776 on cpu 11:
  raid1_read_request+0xe3f/0x19f0
  raid1_make_request+0xdf/0x1990
  md_handle_request+0x4a2/0xa40
  [...]

 value changed: 0x0000000000356368 -> 0x0000000000356370

Signed-off-by: Chen Cheng <[email protected]>
---
 drivers/md/raid1.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 4cdf4484cab6..29b58583e381 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -602,13 +602,13 @@ static void update_read_sectors(struct r1conf *conf, int disk,
 				sector_t this_sector, int len)
 {
 	struct raid1_info *info = &conf->mirrors[disk];
 
 	atomic_inc(&info->rdev->nr_pending);
-	if (info->next_seq_sect != this_sector)
-		info->seq_start = this_sector;
-	info->next_seq_sect = this_sector + len;
+	if (READ_ONCE(info->next_seq_sect) != this_sector)
+		WRITE_ONCE(info->seq_start, this_sector);
+	WRITE_ONCE(info->next_seq_sect, this_sector + len);
 }
 
 static int choose_first_rdev(struct r1conf *conf, struct r1bio *r1_bio,
 			     int *max_sectors)
 {
@@ -733,31 +733,33 @@ static int choose_slow_rdev(struct r1conf *conf, struct r1bio *r1_bio,
 	return bb_disk;
 }
 
 static bool is_sequential(struct r1conf *conf, int disk, struct r1bio *r1_bio)
 {
-	/* TODO: address issues with this check and concurrency. */
-	return conf->mirrors[disk].next_seq_sect == r1_bio->sector ||
+	return READ_ONCE(conf->mirrors[disk].next_seq_sect) == r1_bio->sector ||
 	       READ_ONCE(conf->mirrors[disk].head_position) == r1_bio->sector;
 }
 
 /*
  * If buffered sequential IO size exceeds optimal iosize, check if there is idle
  * disk. If yes, choose the idle disk.
  */
 static bool should_choose_next(struct r1conf *conf, int disk)
 {
 	struct raid1_info *mirror = &conf->mirrors[disk];
+	sector_t seq_start, next_seq_sect;
 	int opt_iosize;
 
 	if (!test_bit(Nonrot, &mirror->rdev->flags))
 		return false;
 
 	opt_iosize = bdev_io_opt(mirror->rdev->bdev) >> 9;
-	return opt_iosize > 0 && mirror->seq_start != MaxSector &&
-	       mirror->next_seq_sect > opt_iosize &&
-	       mirror->next_seq_sect - opt_iosize >= mirror->seq_start;
+	seq_start = READ_ONCE(mirror->seq_start);
+	next_seq_sect = READ_ONCE(mirror->next_seq_sect);
+	return opt_iosize > 0 && seq_start != MaxSector &&
+	       next_seq_sect > opt_iosize &&
+	       next_seq_sect - opt_iosize >= seq_start;
 }
 
 static bool rdev_readable(struct md_rdev *rdev, struct r1bio *r1_bio)
 {
 	if (!rdev || test_bit(Faulty, &rdev->flags))
-- 
2.54.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.