Re: [PATCH 2/5] md: the latest try for improve io stats accounting

[Guoqing Jiang <jgq516@gmail.com>]

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,

Thanks,
Guoqing