On Mon, Aug 31, 2020 at 7:36 AM Xiao Ni <xni@xxxxxxxxxx> wrote:
>
>
>
> On 08/31/2020 04:37 PM, Xiao Ni wrote:
> >
> >
> > 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?
>
> 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
