All of lore.kernel.org
 help / color / mirror / Atom feed
From: Niklas Cassel <Niklas.Cassel@wdc.com>
To: "axboe@kernel.dk" <axboe@kernel.dk>
Cc: "fio@vger.kernel.org" <fio@vger.kernel.org>,
	"damien.lemoal@opensource.wdc.com"
	<damien.lemoal@opensource.wdc.com>,
	Niklas Cassel <Niklas.Cassel@wdc.com>
Subject: [PATCH v2 7/8] cmdprio: add mode to make the logic easier to reason about
Date: Tue, 9 Nov 2021 23:38:31 +0000	[thread overview]
Message-ID: <20211109233804.164786-8-Niklas.Cassel@wdc.com> (raw)
In-Reply-To: <20211109233804.164786-1-Niklas.Cassel@wdc.com>

From: Niklas Cassel <niklas.cassel@wdc.com>

Add a new field "mode", in order to know if we are determining IO
priorities according to cmdprio_percentage or to cmdprio_bssplit.

This makes the logic easier to reason about, and allows us to
remove the "use_cmdprio" variable from the ioengines themselves.

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 engines/cmdprio.c  | 45 +++++++++++++++++++++++++++++----------------
 engines/cmdprio.h  | 10 ++++++++--
 engines/io_uring.c |  7 +++----
 engines/libaio.c   |  7 +++----
 4 files changed, 43 insertions(+), 26 deletions(-)

diff --git a/engines/cmdprio.c b/engines/cmdprio.c
index 9cad76b5..174bed7c 100644
--- a/engines/cmdprio.c
+++ b/engines/cmdprio.c
@@ -70,20 +70,19 @@ int fio_cmdprio_bssplit_parse(struct thread_data *td, const char *input,
 static int fio_cmdprio_percentage(struct cmdprio *cmdprio, struct io_u *io_u)
 {
 	enum fio_ddir ddir = io_u->ddir;
-	unsigned int p = cmdprio->percentage[ddir];
 	int i;
 
-	/*
-	 * If cmdprio_percentage option was specified, then use that
-	 * percentage. Otherwise, use cmdprio_bssplit percentages depending
-	 * on the IO size.
-	 */
-	if (p)
-		return p;
-
-	for (i = 0; i < cmdprio->bssplit_nr[ddir]; i++) {
-		if (cmdprio->bssplit[ddir][i].bs == io_u->buflen)
-			return cmdprio->bssplit[ddir][i].perc;
+	switch (cmdprio->mode) {
+	case CMDPRIO_MODE_PERC:
+		return cmdprio->percentage[ddir];
+	case CMDPRIO_MODE_BSSPLIT:
+		for (i = 0; i < cmdprio->bssplit_nr[ddir]; i++) {
+			if (cmdprio->bssplit[ddir][i].bs == io_u->buflen)
+				return cmdprio->bssplit[ddir][i].perc;
+		}
+		break;
+	default:
+		assert(0);
 	}
 
 	return 0;
@@ -99,10 +98,20 @@ bool fio_cmdprio_set_ioprio(struct thread_data *td, struct cmdprio *cmdprio,
 			    struct io_u *io_u)
 {
 	enum fio_ddir ddir = io_u->ddir;
-	unsigned int p = fio_cmdprio_percentage(cmdprio, io_u);
+	unsigned int p;
 	unsigned int cmdprio_value =
 		ioprio_value(cmdprio->class[ddir], cmdprio->level[ddir]);
 
+	if (cmdprio->mode == CMDPRIO_MODE_NONE) {
+		/*
+		 * An I/O engine should never call this function if cmdprio
+		 * is not is use.
+		 */
+		assert(0);
+		return false;
+	}
+
+	p = fio_cmdprio_percentage(cmdprio, io_u);
 	if (p && rand_between(&td->prio_state, 0, 99) < p) {
 		io_u->ioprio = cmdprio_value;
 		if (!td->ioprio || cmdprio_value < td->ioprio) {
@@ -128,8 +137,7 @@ bool fio_cmdprio_set_ioprio(struct thread_data *td, struct cmdprio *cmdprio,
 	return false;
 }
 
-int fio_cmdprio_init(struct thread_data *td, struct cmdprio *cmdprio,
-		     bool *has_cmdprio)
+int fio_cmdprio_init(struct thread_data *td, struct cmdprio *cmdprio)
 {
 	struct thread_options *to = &td->o;
 	bool has_cmdprio_percentage = false;
@@ -163,7 +171,12 @@ int fio_cmdprio_init(struct thread_data *td, struct cmdprio *cmdprio,
 		return 1;
 	}
 
-	*has_cmdprio = has_cmdprio_percentage || has_cmdprio_bssplit;
+	if (has_cmdprio_bssplit)
+		cmdprio->mode = CMDPRIO_MODE_BSSPLIT;
+	else if (has_cmdprio_percentage)
+		cmdprio->mode = CMDPRIO_MODE_PERC;
+	else
+		cmdprio->mode = CMDPRIO_MODE_NONE;
 
 	return 0;
 }
diff --git a/engines/cmdprio.h b/engines/cmdprio.h
index 732b087e..7e4fcf6c 100644
--- a/engines/cmdprio.h
+++ b/engines/cmdprio.h
@@ -11,12 +11,19 @@
 /* read and writes only, no trim */
 #define CMDPRIO_RWDIR_CNT 2
 
+enum {
+	CMDPRIO_MODE_NONE,
+	CMDPRIO_MODE_PERC,
+	CMDPRIO_MODE_BSSPLIT,
+};
+
 struct cmdprio {
 	unsigned int percentage[CMDPRIO_RWDIR_CNT];
 	unsigned int class[CMDPRIO_RWDIR_CNT];
 	unsigned int level[CMDPRIO_RWDIR_CNT];
 	unsigned int bssplit_nr[CMDPRIO_RWDIR_CNT];
 	struct bssplit *bssplit[CMDPRIO_RWDIR_CNT];
+	unsigned int mode;
 };
 
 int fio_cmdprio_bssplit_parse(struct thread_data *td, const char *input,
@@ -25,7 +32,6 @@ int fio_cmdprio_bssplit_parse(struct thread_data *td, const char *input,
 bool fio_cmdprio_set_ioprio(struct thread_data *td, struct cmdprio *cmdprio,
 			    struct io_u *io_u);
 
-int fio_cmdprio_init(struct thread_data *td, struct cmdprio *cmdprio,
-		     bool *has_cmdprio);
+int fio_cmdprio_init(struct thread_data *td, struct cmdprio *cmdprio);
 
 #endif
diff --git a/engines/io_uring.c b/engines/io_uring.c
index 230f08a5..979d07d4 100644
--- a/engines/io_uring.c
+++ b/engines/io_uring.c
@@ -68,8 +68,6 @@ struct ioring_data {
 	int prepped;
 
 	struct ioring_mmap mmap[3];
-
-	bool use_cmdprio;
 };
 
 struct ioring_options {
@@ -472,6 +470,7 @@ static enum fio_q_status fio_ioring_queue(struct thread_data *td,
 {
 	struct ioring_data *ld = td->io_ops_data;
 	struct io_sq_ring *ring = &ld->sq_ring;
+	struct ioring_options *o = td->eo;
 	unsigned tail, next_tail;
 
 	fio_ro_check(td, io_u);
@@ -494,7 +493,7 @@ static enum fio_q_status fio_ioring_queue(struct thread_data *td,
 	if (next_tail == atomic_load_acquire(ring->head))
 		return FIO_Q_BUSY;
 
-	if (ld->use_cmdprio)
+	if (o->cmdprio.mode != CMDPRIO_MODE_NONE)
 		fio_ioring_cmdprio_prep(td, io_u);
 
 	ring->array[tail & ld->sq_ring_mask] = io_u->index;
@@ -831,7 +830,7 @@ static int fio_ioring_init(struct thread_data *td)
 
 	td->io_ops_data = ld;
 
-	ret = fio_cmdprio_init(td, cmdprio, &ld->use_cmdprio);
+	ret = fio_cmdprio_init(td, cmdprio);
 	if (ret) {
 		td_verror(td, EINVAL, "fio_ioring_init");
 		return 1;
diff --git a/engines/libaio.c b/engines/libaio.c
index dc6ad08e..53fe7428 100644
--- a/engines/libaio.c
+++ b/engines/libaio.c
@@ -51,8 +51,6 @@ struct libaio_data {
 	unsigned int queued;
 	unsigned int head;
 	unsigned int tail;
-
-	bool use_cmdprio;
 };
 
 struct libaio_options {
@@ -320,6 +318,7 @@ static enum fio_q_status fio_libaio_queue(struct thread_data *td,
 					  struct io_u *io_u)
 {
 	struct libaio_data *ld = td->io_ops_data;
+	struct libaio_options *o = td->eo;
 
 	fio_ro_check(td, io_u);
 
@@ -350,7 +349,7 @@ static enum fio_q_status fio_libaio_queue(struct thread_data *td,
 		return FIO_Q_COMPLETED;
 	}
 
-	if (ld->use_cmdprio)
+	if (o->cmdprio.mode != CMDPRIO_MODE_NONE)
 		fio_libaio_cmdprio_prep(td, io_u);
 
 	ld->iocbs[ld->head] = &io_u->iocb;
@@ -507,7 +506,7 @@ static int fio_libaio_init(struct thread_data *td)
 
 	td->io_ops_data = ld;
 
-	ret = fio_cmdprio_init(td, cmdprio, &ld->use_cmdprio);
+	ret = fio_cmdprio_init(td, cmdprio);
 	if (ret) {
 		td_verror(td, EINVAL, "fio_libaio_init");
 		return 1;
-- 
2.33.1

  parent reply	other threads:[~2021-11-09 23:38 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-09 23:38 [PATCH v2 0/8] cmdprio cleanup series Niklas Cassel
2021-11-09 23:38 ` [PATCH v2 1/8] docs: update cmdprio_percentage documentation Niklas Cassel
2021-11-09 23:38 ` [PATCH v2 2/8] cmdprio: move cmdprio function definitions to a new cmdprio.c file Niklas Cassel
2021-11-09 23:38 ` [PATCH v2 3/8] cmdprio: do not allocate memory for unused data direction Niklas Cassel
2021-11-09 23:38 ` [PATCH v2 4/8] io_uring: set async IO priority to td->ioprio in fio_ioring_prep() Niklas Cassel
2021-11-12  0:41   ` Damien Le Moal
2021-11-09 23:38 ` [PATCH v2 5/8] libaio,io_uring: rename prio_prep() to include cmdprio in the name Niklas Cassel
2021-11-09 23:38 ` Niklas Cassel [this message]
2021-11-12  0:58   ` [PATCH v2 7/8] cmdprio: add mode to make the logic easier to reason about Damien Le Moal
2021-11-09 23:38 ` [PATCH v2 6/8] libaio,io_uring: move common cmdprio_prep() code to cmdprio Niklas Cassel
2021-11-12  0:52   ` Damien Le Moal
2021-11-09 23:38 ` [PATCH v2 8/8] libaio,io_uring: make it possible to cleanup cmdprio malloced data Niklas Cassel
2021-11-12  1:12   ` Damien Le Moal

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=20211109233804.164786-8-Niklas.Cassel@wdc.com \
    --to=niklas.cassel@wdc.com \
    --cc=axboe@kernel.dk \
    --cc=damien.lemoal@opensource.wdc.com \
    --cc=fio@vger.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.