Hi Xiao,
Thanks for the results.
On Wed, Nov 16, 2022 at 6:03 PM Xiao Ni <xni@xxxxxxxxxx> wrote:
>
> Hi Song
>
> The performance is good. Please check the result below.
>
> And for the patch itself, do you think we should add a smp_mb
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 4d0139cae8b5..3696e3825e27 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -8650,9 +8650,11 @@ static void md_end_io_acct(struct bio *bio)
> bio_put(bio);
> bio_endio(orig_bio);
>
> - if (atomic_dec_and_test(&mddev->io_acct_cnt))
> + if (atomic_dec_and_test(&mddev->io_acct_cnt)) {
> + smp_mb();
> if (unlikely(test_bit(MD_QUIESCE, &mddev->flags)))
> wake_up(&mddev->wait_io_acct);
> + }
> }
>
> /*
> diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
> index 9d4831ca802c..1818f79bfdf7 100644
> --- a/drivers/md/raid0.c
> +++ b/drivers/md/raid0.c
> @@ -757,6 +757,7 @@ static void raid0_quiesce(struct mddev *mddev, int quiesce)
> * to member disks to avoid memory alloc and performance decrease
> */
> set_bit(MD_QUIESCE, &mddev->flags);
> + smp_mb();
> wait_event(mddev->wait_io_acct, !atomic_read(&mddev->io_acct_cnt));
> clear_bit(MD_QUIESCE, &mddev->flags);
> }
>
> Test result:
I think there is some noise in the result?
>
> without patch with patch
> psync read 100MB/s 101MB/s job:1 bs:4k
For example, this is a small improvement, but
> 1015MB/s 1016MB/s job:1 bs:128k
> 1359MB/s 1358MB/s job:1 bs:256k
> 1394MB/s 1393MB/s job:40 bs:4k
> 4959MB/s 4873MB/s job:40 bs:128k
> 6166MB/s 6157MB/s job:40 bs:256k
>
> without patch with patch
> psync write 286MB/s 275MB/s job:1 bs:4k
this is a big regression (~4%).
> 1810MB/s 1808MB/s job:1 bs:128k
> 1814MB/s 1814MB/s job:1 bs:256k
> 1802MB/s 1801MB/s job:40 bs:4k
> 1814MB/s 1814MB/s job:40 bs:128k
> 1814MB/s 1814MB/s job:40 bs:256k
>
> without patch
> psync randread 39.3MB/s 39.7MB/s job:1 bs:4k
> 791MB/s 783MB/s job:1 bs:128k
> 1183MiB/s 1217MB/s job:1 bs:256k
> 1183MiB/s 1235MB/s job:40 bs:4k
> 3768MB/s 3705MB/s job:40 bs:128k
And some regression for 128kB but improvement for 4kB.
> 4410MB/s 4418MB/s job:40 bs:256k
So I am not quite convinced by these results.
Also, do we really need an extra counter here? Can we use
mddev->active_io instead?
Thanks,
Song
[...]
