Messages in this thread | | | Subject | Re: [PATCH md-6.9 02/10] md: record nonrot rdevs while adding/removing rdevs to conf | From | Yu Kuai <> | Date | Mon, 26 Feb 2024 21:25:17 +0800 |
| |
Hi,
在 2024/02/26 21:12, Xiao Ni 写道: > Hi Kuai > > I added some logs to check and add_bound_rdev can't be called when > creating raid device. Maybe move rdev_update_nonrot to > bind_rdev_to_array?
bind_rdev_to_array() is used to add new rdev to the array, then 'pers->hot_add_disk' is used to add the rdev to conf. For new spares, bind_rdev_to_array() will be called while 'pers->hot_add_disk' won't. Hence rdev_update_nonrot() is used where 'pers->hot_add_disk' succeed in this patch.
Perhaps it's better to move related code to raid1/raid10_add_disk() and the new counter in raid1/raid10 conf?
Thanks, Kuai
> > Regards > Xiao > > On Thu, Feb 22, 2024 at 4:04 PM Yu Kuai <yukuai1@huaweicloud.com> wrote: >> >> From: Yu Kuai <yukuai3@huawei.com> >> >> For raid1/raid10, each read will iterate all the rdevs from conf and >> check if any rdev is non-rotational, then choose rdev with minimal IO >> inflight if so, or rdev with closest distance otherwise. >> >> Disk nonrot info can be changed through sysfs entry: >> >> /sys/block/[disk_name]/queue/rotational >> >> However, consider that this should only be used for testing, and user >> really shouldn't do this in real life. Record the number of non-rotational >> disks in mddev, to avoid checking each rdev in IO fast path and simplify >> read_balance() a little bit. >> >> Co-developed-by: Paul Luse <paul.e.luse@linux.intel.com> >> Signed-off-by: Paul Luse <paul.e.luse@linux.intel.com> >> Signed-off-by: Yu Kuai <yukuai3@huawei.com> >> --- >> drivers/md/md.c | 28 ++++++++++++++++++++++++++-- >> drivers/md/md.h | 2 ++ >> drivers/md/raid1.c | 9 ++------- >> drivers/md/raid10.c | 8 ++------ >> 4 files changed, 32 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/md/md.c b/drivers/md/md.c >> index e2a5f513dbb7..9e671eec9309 100644 >> --- a/drivers/md/md.c >> +++ b/drivers/md/md.c >> @@ -146,6 +146,24 @@ static inline int speed_max(struct mddev *mddev) >> mddev->sync_speed_max : sysctl_speed_limit_max; >> } >> >> +static void rdev_update_nonrot(struct md_rdev *rdev) >> +{ >> + if (!bdev_nonrot(rdev->bdev)) >> + return; >> + >> + set_bit(Nonrot, &rdev->flags); >> + WRITE_ONCE(rdev->mddev->nonrot_disks, rdev->mddev->nonrot_disks + 1); >> +} >> + >> +static void rdev_clear_nonrot(struct md_rdev *rdev) >> +{ >> + if (!test_bit(Nonrot, &rdev->flags)) >> + return; >> + >> + clear_bit(Nonrot, &rdev->flags); >> + WRITE_ONCE(rdev->mddev->nonrot_disks, rdev->mddev->nonrot_disks - 1); >> +} >> + >> static void rdev_uninit_serial(struct md_rdev *rdev) >> { >> if (!test_and_clear_bit(CollisionCheck, &rdev->flags)) >> @@ -2922,6 +2940,8 @@ static int add_bound_rdev(struct md_rdev *rdev) >> md_kick_rdev_from_array(rdev); >> return err; >> } >> + >> + rdev_update_nonrot(rdev); >> } >> sysfs_notify_dirent_safe(rdev->sysfs_state); >> >> @@ -3271,8 +3291,10 @@ slot_store(struct md_rdev *rdev, const char *buf, size_t len) >> if (err) { >> rdev->raid_disk = -1; >> return err; >> - } else >> - sysfs_notify_dirent_safe(rdev->sysfs_state); >> + } >> + >> + rdev_update_nonrot(rdev); >> + sysfs_notify_dirent_safe(rdev->sysfs_state); >> /* failure here is OK */; >> sysfs_link_rdev(rdev->mddev, rdev); >> /* don't wakeup anyone, leave that to userspace. */ >> @@ -9266,6 +9288,7 @@ static int remove_and_add_spares(struct mddev *mddev, >> rdev_for_each(rdev, mddev) { >> if ((this == NULL || rdev == this) && rdev_removeable(rdev) && >> !mddev->pers->hot_remove_disk(mddev, rdev)) { >> + rdev_clear_nonrot(rdev); >> sysfs_unlink_rdev(mddev, rdev); >> rdev->saved_raid_disk = rdev->raid_disk; >> rdev->raid_disk = -1; >> @@ -9289,6 +9312,7 @@ static int remove_and_add_spares(struct mddev *mddev, >> if (!test_bit(Journal, &rdev->flags)) >> rdev->recovery_offset = 0; >> if (mddev->pers->hot_add_disk(mddev, rdev) == 0) { >> + rdev_update_nonrot(rdev); >> /* failure here is OK */ >> sysfs_link_rdev(mddev, rdev); >> if (!test_bit(Journal, &rdev->flags)) >> diff --git a/drivers/md/md.h b/drivers/md/md.h >> index a49ab04ab707..54aa951f2bba 100644 >> --- a/drivers/md/md.h >> +++ b/drivers/md/md.h >> @@ -207,6 +207,7 @@ enum flag_bits { >> * check if there is collision between raid1 >> * serial bios. >> */ >> + Nonrot, /* non-rotational device (SSD) */ >> }; >> >> static inline int is_badblock(struct md_rdev *rdev, sector_t s, int sectors, >> @@ -312,6 +313,7 @@ struct mddev { >> unsigned long flags; >> unsigned long sb_flags; >> >> + int nonrot_disks; >> int suspended; >> struct mutex suspend_mutex; >> struct percpu_ref active_io; >> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c >> index a145fe48b9ce..c60ea58ae8c5 100644 >> --- a/drivers/md/raid1.c >> +++ b/drivers/md/raid1.c >> @@ -599,7 +599,6 @@ static int read_balance(struct r1conf *conf, struct r1bio *r1_bio, int *max_sect >> int sectors; >> int best_good_sectors; >> int best_disk, best_dist_disk, best_pending_disk; >> - int has_nonrot_disk; >> int disk; >> sector_t best_dist; >> unsigned int min_pending; >> @@ -620,7 +619,6 @@ static int read_balance(struct r1conf *conf, struct r1bio *r1_bio, int *max_sect >> best_pending_disk = -1; >> min_pending = UINT_MAX; >> best_good_sectors = 0; >> - has_nonrot_disk = 0; >> choose_next_idle = 0; >> clear_bit(R1BIO_FailFast, &r1_bio->state); >> >> @@ -637,7 +635,6 @@ static int read_balance(struct r1conf *conf, struct r1bio *r1_bio, int *max_sect >> sector_t first_bad; >> int bad_sectors; >> unsigned int pending; >> - bool nonrot; >> >> rdev = conf->mirrors[disk].rdev; >> if (r1_bio->bios[disk] == IO_BLOCKED >> @@ -703,8 +700,6 @@ static int read_balance(struct r1conf *conf, struct r1bio *r1_bio, int *max_sect >> /* At least two disks to choose from so failfast is OK */ >> set_bit(R1BIO_FailFast, &r1_bio->state); >> >> - nonrot = bdev_nonrot(rdev->bdev); >> - has_nonrot_disk |= nonrot; >> pending = atomic_read(&rdev->nr_pending); >> dist = abs(this_sector - conf->mirrors[disk].head_position); >> if (choose_first) { >> @@ -731,7 +726,7 @@ static int read_balance(struct r1conf *conf, struct r1bio *r1_bio, int *max_sect >> * small, but not a big deal since when the second disk >> * starts IO, the first disk is likely still busy. >> */ >> - if (nonrot && opt_iosize > 0 && >> + if (test_bit(Nonrot, &rdev->flags) && opt_iosize > 0 && >> mirror->seq_start != MaxSector && >> mirror->next_seq_sect > opt_iosize && >> mirror->next_seq_sect - opt_iosize >= >> @@ -763,7 +758,7 @@ static int read_balance(struct r1conf *conf, struct r1bio *r1_bio, int *max_sect >> * mixed ratation/non-rotational disks depending on workload. >> */ >> if (best_disk == -1) { >> - if (has_nonrot_disk || min_pending == 0) >> + if (conf->mddev->nonrot_disks || min_pending == 0) >> best_disk = best_pending_disk; >> else >> best_disk = best_dist_disk; >> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c >> index d5a7a621f0f0..1f6693e40e12 100644 >> --- a/drivers/md/raid10.c >> +++ b/drivers/md/raid10.c >> @@ -735,7 +735,6 @@ static struct md_rdev *read_balance(struct r10conf *conf, >> struct md_rdev *best_dist_rdev, *best_pending_rdev, *rdev = NULL; >> int do_balance; >> int best_dist_slot, best_pending_slot; >> - bool has_nonrot_disk = false; >> unsigned int min_pending; >> struct geom *geo = &conf->geo; >> >> @@ -766,7 +765,6 @@ static struct md_rdev *read_balance(struct r10conf *conf, >> int bad_sectors; >> sector_t dev_sector; >> unsigned int pending; >> - bool nonrot; >> >> if (r10_bio->devs[slot].bio == IO_BLOCKED) >> continue; >> @@ -818,10 +816,8 @@ static struct md_rdev *read_balance(struct r10conf *conf, >> if (!do_balance) >> break; >> >> - nonrot = bdev_nonrot(rdev->bdev); >> - has_nonrot_disk |= nonrot; >> pending = atomic_read(&rdev->nr_pending); >> - if (min_pending > pending && nonrot) { >> + if (min_pending > pending && test_bit(Nonrot, &rdev->flags)) { >> min_pending = pending; >> best_pending_slot = slot; >> best_pending_rdev = rdev; >> @@ -851,7 +847,7 @@ static struct md_rdev *read_balance(struct r10conf *conf, >> } >> } >> if (slot >= conf->copies) { >> - if (has_nonrot_disk) { >> + if (conf->mddev->nonrot_disks) { >> slot = best_pending_slot; >> rdev = best_pending_rdev; >> } else { >> -- >> 2.39.2 >> >> > > . >
| |