On 08/29/2020 06:16 AM, Song Liu wrote:
On Mon, Aug 24, 2020 at 10:43 PM Xiao Ni <xni@xxxxxxxxxx> wrote:
[...]
---
drivers/md/raid10.c | 254 +++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 253 insertions(+), 1 deletion(-)
[...]
+
+static void raid10_end_discard_request(struct bio *bio)
+{
+ struct r10bio *r10_bio = bio->bi_private;
+ struct r10conf *conf = r10_bio->mddev->private;
+ struct md_rdev *rdev = NULL;
+ int dev;
+ int slot, repl;
+
+ /*
+ * We don't care the return value of discard bio
+ */
+ if (!test_bit(R10BIO_Uptodate, &r10_bio->state))
+ set_bit(R10BIO_Uptodate, &r10_bio->state);
We don't need the test_bit(), just do set_bit().
Coly suggested to do test_bit first to avoid write memory. If there are
so many requests and the
requests fail, this way can improve performance very much.
But it doesn't care the return value of discard bio. So it should be ok
that doesn't set R10BIO_Uptodate here.
I'll remove these codes. What do you think?
+
+ dev = find_bio_disk(conf, r10_bio, bio, &slot, &repl);
+ if (repl)
+ rdev = conf->mirrors[dev].replacement;
+ if (!rdev) {
+ /* raid10_remove_disk uses smp_mb to make sure rdev is set to
+ * replacement before setting replacement to NULL. It can read
+ * rdev first without barrier protect even replacment is NULL
+ */
+ smp_rmb();
+ repl = 0;
repl is no longer used, right?
Right, I'll remove this line
+ rdev = conf->mirrors[dev].rdev;
[...]
+
+ if (conf->reshape_progress != MaxSector &&
+ ((bio->bi_iter.bi_sector >= conf->reshape_progress) !=
+ conf->mddev->reshape_backwards))
+ geo = &conf->prev;
Do we need to set R10BIO_Previous here? Also, please run some tests with
reshape in progress.
Thanks,
Song
Thanks for pointing about this. Yes, it needs to set R10BIO_Previous
here. Because it needs to
choose_data_offset when submitting bio to member disks. But it lets me
think about patch 1/5
has a problem too. It uses rdev->data_offset directly in function
md_submit_discard_bio. It's ok
for raid0, because it changes other levels during reshape. For raid10,
it's a bug. Now it's in md-next
branch. Do you want me to resend all patches or a new patch to fix 1/5
problem? Sorry for making
this trouble.
Regards
Xiao
