Re: [PATCH V2 2/7] md: add accounting_bio for raid0 and raid5

[Song Liu <song@kernel.org>]

On Thu, May 20, 2021 at 5:56 PM Guoqing Jiang <jgq516@gmail.com> wrote:
>
> Let's introduce accounting_bio which checks if md needs clone the bio
> for accounting.
>
> And add relevant function to raid0 and raid5 given both don't have
> their own clone infrastrure, also checks if it is split bio.
>
> Signed-off-by: Guoqing Jiang <jiangguoqing@kylinos.cn>
> ---
>  drivers/md/md.h    |  2 ++
>  drivers/md/raid0.c | 14 ++++++++++++++
>  drivers/md/raid5.c | 17 +++++++++++++++++
>  3 files changed, 33 insertions(+)
>
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 4da240ffe2c5..5125ccf9df06 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -605,6 +605,8 @@ struct md_personality
>         void *(*takeover) (struct mddev *mddev);
>         /* Changes the consistency policy of an active array. */
>         int (*change_consistency_policy)(struct mddev *mddev, const char *buf);
> +       /* check if need to clone bio for accounting in md layer */
> +       bool (*accounting_bio)(struct mddev *mddev, struct bio *bio);
>  };
>
>  struct md_sysfs_entry {
> 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;

We can simply do "return bio->bi_pool != &mddev->bio_set;". And same for
raid5_accouting_bio.

> +
> +       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,
>  };
>
>  static int __init raid0_init (void)
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 841e1c1aa5e6..bcc1ceb69c73 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;
> @@ -8688,6 +8702,7 @@ static struct md_personality raid6_personality =
>         .quiesce        = raid5_quiesce,
>         .takeover       = raid6_takeover,
>         .change_consistency_policy = raid5_change_consistency_policy,
> +       .accounting_bio = raid5_accounting_bio,
>  };
>  static struct md_personality raid5_personality =
>  {
> @@ -8712,6 +8727,7 @@ static struct md_personality raid5_personality =
>         .quiesce        = raid5_quiesce,
>         .takeover       = raid5_takeover,
>         .change_consistency_policy = raid5_change_consistency_policy,
> +       .accounting_bio = raid5_accounting_bio,
>  };
>
>  static struct md_personality raid4_personality =
> @@ -8737,6 +8753,7 @@ static struct md_personality raid4_personality =
>         .quiesce        = raid5_quiesce,
>         .takeover       = raid4_takeover,
>         .change_consistency_policy = raid5_change_consistency_policy,
> +       .accounting_bio = raid5_accounting_bio,
>  };
>
>  static int __init raid5_init(void)
> --
> 2.25.1
>