cmdprio: add mode to make the logic easier to reason about

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>
Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Link: https://lore.kernel.org/r/20211112095428.158300-8-Niklas.Cassel@wdc.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/engines/cmdprio.c b/engines/cmdprio.c
index d36b6264e07d1dfb735ee2efb3ef2b735b8021a2..8c52dabdf3e86d671f35b825ea9a0807f36cb413 100644 (file)
--- a/engines/cmdprio.c
+++ b/engines/cmdprio.c
@@ -70,20 +70,23 @@ 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:
+               /*
+                * An I/O engine should never call this function if cmdprio
+                * is not is use.
+                */
+               assert(0);
        }
 
        return 0;
@@ -104,10 +107,11 @@ 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]);
 
+       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) {
@@ -134,8 +138,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;
@@ -169,7 +172,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 732b087e5df637633fb6ba6deac6d19f2649a3c8..7e4fcf6c58e4113436b19cc8cf3b1965889c6b0e 100644 (file)
--- 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 68c3311386da328abe7e19b435186b6a3fbe7fdd..1f55ad1756af2e2bf43d6297fe67790c3a9bb73b 100644 (file)
--- 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 dc6ad08e5a08c73c0380a4019645ea5d79f7718e..53fe7428a010cfa07140c108760a8005094a2c7e 100644 (file)
--- 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;