Re: [PATCH V5 4/5] md/raid10: improve raid10 discard request

On Mon, Aug 31, 2020 at 7:36 AM Xiao Ni [off-list ref] wrote:

On 08/31/2020 04:37 PM, Xiao Ni wrote:

quoted

On 08/29/2020 06:16 AM, Song Liu wrote:

quoted

On Mon, Aug 24, 2020 at 10:43 PM Xiao Ni [off-list ref] wrote:
[...]

quoted

---
  drivers/md/raid10.c | 254
+++++++++++++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 253 insertions(+), 1 deletion(-)

[...]

quoted

+
+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?

Hi Song

Sorry, for this problem, it still needs to set R10BIO_Uptodate. Because
in function raid_end_bio_io it needs to use this
flag to justify whether set BLK_STS_IOERR or not. So is it ok to test
this bit first before setting this bit here?

Let's keep the test_bit() and set_bit().

Please send fixes on top of 1/5. I will handle them when I apply the patch.

Thanks,
Song