From: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
To: song@kernel.org
Cc: linux-raid@vger.kernel.org,
Artur Paszkiewicz <artur.paszkiewicz@intel.com>
Subject: [PATCH] md: improve io stats accounting
Date: Mon, 1 Jun 2020 18:12:56 +0200 [thread overview]
Message-ID: <20200601161256.27718-1-artur.paszkiewicz@intel.com> (raw)
Use generic io accounting functions to manage io stats. There was an
attempt to do this earlier in commit 18c0b223cf990172 ("md: use generic
io stats accounting functions to simplify io stat accounting"), but it
did not include a call to generic_end_io_acct() and caused issues with
tracking in-flight IOs, so it was later removed in commit
74672d069b298b03 ("md: fix md io stats accounting broken").
This patch attempts to fix this by using both generic_start_io_acct()
and generic_end_io_acct(). To make it possible, in md_make_request() a
bio is cloned with additional data - struct md_io, which includes the io
start_time. A new bioset is introduced for this purpose. We call
generic_start_io_acct() and pass the clone instead of the original to
md_handle_request(). When it completes, we call generic_end_io_acct()
and complete the original bio.
This adds correct statistics about in-flight IOs and IO processing time,
interpreted e.g. in iostat as await, svctm, aqu-sz and %util.
It also fixes a situation where too many IOs where reported if a bio was
re-submitted to the mddev, because io accounting is now performed only
on newly arriving bios.
Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
---
drivers/md/md.c | 65 +++++++++++++++++++++++++++++++++++++++----------
drivers/md/md.h | 1 +
2 files changed, 53 insertions(+), 13 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index f567f536b529..5a9f167ef5b9 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -463,12 +463,32 @@ void md_handle_request(struct mddev *mddev, struct bio *bio)
}
EXPORT_SYMBOL(md_handle_request);
+struct md_io {
+ struct mddev *mddev;
+ struct bio *orig_bio;
+ unsigned long start_time;
+ struct bio orig_bio_clone;
+};
+
+static void md_end_request(struct bio *bio)
+{
+ struct md_io *md_io = bio->bi_private;
+ struct mddev *mddev = md_io->mddev;
+ struct bio *orig_bio = md_io->orig_bio;
+
+ orig_bio->bi_status = bio->bi_status;
+
+ generic_end_io_acct(mddev->queue, bio_op(orig_bio),
+ &mddev->gendisk->part0, md_io->start_time);
+ bio_put(bio);
+
+ bio_endio(orig_bio);
+}
+
static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
{
const int rw = bio_data_dir(bio);
- const int sgrp = op_stat_group(bio_op(bio));
struct mddev *mddev = bio->bi_disk->private_data;
- unsigned int sectors;
if (unlikely(test_bit(MD_BROKEN, &mddev->flags)) && (rw == WRITE)) {
bio_io_error(bio);
@@ -488,21 +508,30 @@ static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
return BLK_QC_T_NONE;
}
- /*
- * save the sectors now since our bio can
- * go away inside make_request
- */
- sectors = bio_sectors(bio);
+ if (bio->bi_pool != &mddev->md_io_bs) {
+ struct bio *clone;
+ struct md_io *md_io;
+
+ clone = bio_clone_fast(bio, GFP_NOIO, &mddev->md_io_bs);
+
+ md_io = container_of(clone, struct md_io, orig_bio_clone);
+ md_io->mddev = mddev;
+ md_io->orig_bio = bio;
+ md_io->start_time = jiffies;
+
+ clone->bi_end_io = md_end_request;
+ clone->bi_private = md_io;
+ bio = clone;
+
+ generic_start_io_acct(mddev->queue, bio_op(bio),
+ bio_sectors(bio), &mddev->gendisk->part0);
+ }
+
/* bio could be mergeable after passing to underlayer */
bio->bi_opf &= ~REQ_NOMERGE;
md_handle_request(mddev, bio);
- part_stat_lock();
- part_stat_inc(&mddev->gendisk->part0, ios[sgrp]);
- part_stat_add(&mddev->gendisk->part0, sectors[sgrp], sectors);
- part_stat_unlock();
-
return BLK_QC_T_NONE;
}
@@ -2338,7 +2367,8 @@ int md_integrity_register(struct mddev *mddev)
bdev_get_integrity(reference->bdev));
pr_debug("md: data integrity enabled on %s\n", mdname(mddev));
- if (bioset_integrity_create(&mddev->bio_set, BIO_POOL_SIZE)) {
+ if (bioset_integrity_create(&mddev->bio_set, BIO_POOL_SIZE) ||
+ bioset_integrity_create(&mddev->md_io_bs, BIO_POOL_SIZE)) {
pr_err("md: failed to create integrity pool for %s\n",
mdname(mddev));
return -EINVAL;
@@ -5545,6 +5575,7 @@ static void md_free(struct kobject *ko)
bioset_exit(&mddev->bio_set);
bioset_exit(&mddev->sync_set);
+ bioset_exit(&mddev->md_io_bs);
kfree(mddev);
}
@@ -5838,6 +5869,12 @@ int md_run(struct mddev *mddev)
if (err)
return err;
}
+ if (!bioset_initialized(&mddev->md_io_bs)) {
+ err = bioset_init(&mddev->md_io_bs, BIO_POOL_SIZE,
+ offsetof(struct md_io, orig_bio_clone), 0);
+ if (err)
+ return err;
+ }
spin_lock(&pers_lock);
pers = find_pers(mddev->level, mddev->clevel);
@@ -6015,6 +6052,7 @@ int md_run(struct mddev *mddev)
abort:
bioset_exit(&mddev->bio_set);
bioset_exit(&mddev->sync_set);
+ bioset_exit(&mddev->md_io_bs);
return err;
}
EXPORT_SYMBOL_GPL(md_run);
@@ -6239,6 +6277,7 @@ void md_stop(struct mddev *mddev)
__md_stop(mddev);
bioset_exit(&mddev->bio_set);
bioset_exit(&mddev->sync_set);
+ bioset_exit(&mddev->md_io_bs);
}
EXPORT_SYMBOL_GPL(md_stop);
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 612814d07d35..74273728b898 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -481,6 +481,7 @@ struct mddev {
struct bio_set sync_set; /* for sync operations like
* metadata and bitmap writes
*/
+ struct bio_set md_io_bs;
/* Generic flush handling.
* The last to finish preflush schedules a worker to submit
--
2.26.0
next reply other threads:[~2020-06-01 16:12 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-01 16:12 Artur Paszkiewicz [this message]
2020-06-01 22:03 ` [PATCH] md: improve io stats accounting kbuild test robot
2020-06-02 6:22 ` Song Liu
2020-06-02 6:31 ` [kbuild-all] " Rong Chen
2020-06-02 6:48 ` Song Liu
2020-06-02 11:47 ` Artur Paszkiewicz
2020-06-02 17:16 ` Song Liu
2020-06-02 17:32 ` John Stoffel
2020-06-02 7:01 ` kbuild test robot
2020-06-08 14:37 ` Guoqing Jiang
2020-07-02 6:30 ` Song Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200601161256.27718-1-artur.paszkiewicz@intel.com \
--to=artur.paszkiewicz@intel.com \
--cc=linux-raid@vger.kernel.org \
--cc=song@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).