On Wed, May 19, 2021 at 1:09 AM Guoqing Jiang <jgq516@xxxxxxxxx> wrote:
>
>
>
> On 5/19/21 3:26 PM, Artur Paszkiewicz wrote:
> > On 19.05.2021 03:30, Guoqing Jiang wrote:
> >> Hmm, raid0 allocates split bio from mddev->bio_set, but raid5 is
> >> different, it splits bio from r5conf->bio_split. So either let raid5 also
> >> splits bio from mddev->bio_set, or add an additional checking for
> >> raid5. Thoughts?
> > It looks like raid5 has a different bio set for that because it uses
> > mddev->bio_set for something else - allocating a bio for rdev. So I
> > think it can be changed to split from mddev->bio_set and have a private
> > bio set for the rdev bio allocation.
>
>
> Instead of add a flag, I tried this way:
>
> diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
> index e5d7411cba9b..d309b639b5d9 100644
> --- a/drivers/md/raid0.c
> +++ b/drivers/md/raid0.c
> @@ -748,6 +748,19 @@ static void raid0_quiesce(struct mddev *mddev, int
> quiesce)
> {
> }
>
> +/*
> + * Don't account the bio if it was split from mddev->bio_set.
> + */
> +static bool raid0_accounting_bio(struct mddev *mddev, struct bio *bio)
> +{
> + bool ret = true;
> +
> + if (bio->bi_pool == &mddev->bio_set)
> + ret = false;
> +
> + return ret;
> +}
> +
> static struct md_personality raid0_personality=
> {
> .name = "raid0",
> @@ -760,6 +773,7 @@ static struct md_personality raid0_personality=
> .size = raid0_size,
> .takeover = raid0_takeover,
> .quiesce = raid0_quiesce,
> + .accounting_bio = raid0_accounting_bio,
> };
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 841e1c1aa5e6..070ccf55c534 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -8596,6 +8596,20 @@ static void *raid6_takeover(struct mddev *mddev)
> return setup_conf(mddev);
> }
>
> +/*
> + * Don't account the bio if it was split from r5conf->bio_split.
> + */
> +static bool raid5_accounting_bio(struct mddev *mddev, struct bio *bio)
> +{
> + bool ret = true;
> + struct r5conf *conf = mddev->private;
> +
> + if (bio->bi_pool == &conf->bio_split)
> + ret = false;
> +
> + return ret;
> +}
> +
> static int raid5_change_consistency_policy(struct mddev *mddev, const
> char *buf)
> {
> struct r5conf *conf;
> @@ -8687,6 +8701,7 @@ static struct md_personality raid6_personality =
> .finish_reshape = raid5_finish_reshape,
> .quiesce = raid5_quiesce,
> .takeover = raid6_takeover,
> + .accounting_bio = raid5_accounting_bio,
I like the accounting_bio idea. Let's go with it.
Thanks,
Song
