From: Ankit Kumar <ankit1455@gmail.com>
To: Vincent Fu <vincentfu@gmail.com>
Cc: axboe@kernel.dk, ankit.kumar@samsung.com, kbusch@kernel.org,
fio@vger.kernel.org, Vincent Fu <vincent.fu@samsung.com>
Subject: Re: [PATCH 2/9] fio: create over-arching data placement option
Date: Tue, 23 Apr 2024 14:06:49 +0530 [thread overview]
Message-ID: <CAKi7+wcrvg4_ouCU2HUEWjA-TKwbTkc0xHjq5fGiufYw=hQWig@mail.gmail.com> (raw)
In-Reply-To: <20240422175522.4614-3-vincent.fu@samsung.com>
- .lname = "FDP Placement ID select",
+ .name = "dataplacement",
Wondering if instead of dataplacement we should use data_placement. I
am ok either way.
On Mon, Apr 22, 2024 at 11:26 PM Vincent Fu <vincentfu@gmail.com> wrote:
>
> Since FDP and streams are similar, we should have an over-arching data
> placement option that encompasses both of these frameworks instead of
> having separate sets of similar options for FDP and streams.
>
> With a common set of options, users will be able to select the data
> placement strategy (fdp or streams), the placement identifiers to use,
> and the algorithm for selecting from the list of placement identifiers.
>
> The original set of FDP options is retained for backward compatibility.
>
> No functional change.
>
> Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
> ---
> cconv.c | 18 +++++++++-------
> dataplacement.c | 14 ++++++-------
> dataplacement.h | 11 ++++++++--
> filesetup.c | 2 +-
> init.c | 10 ++++++++-
> io_u.c | 2 +-
> options.c | 54 +++++++++++++++++++++++++++++++++++-------------
> server.h | 2 +-
> thread_options.h | 15 +++++++-------
> 9 files changed, 86 insertions(+), 42 deletions(-)
>
> diff --git a/cconv.c b/cconv.c
> index ead47248..16112248 100644
> --- a/cconv.c
> +++ b/cconv.c
> @@ -354,10 +354,11 @@ int convert_thread_options_to_cpu(struct thread_options *o,
> o->merge_blktrace_iters[i].u.f = fio_uint64_to_double(le64_to_cpu(top->merge_blktrace_iters[i].u.i));
>
> o->fdp = le32_to_cpu(top->fdp);
> - o->fdp_pli_select = le32_to_cpu(top->fdp_pli_select);
> - o->fdp_nrpli = le32_to_cpu(top->fdp_nrpli);
> - for (i = 0; i < o->fdp_nrpli; i++)
> - o->fdp_plis[i] = le32_to_cpu(top->fdp_plis[i]);
> + o->dp_type = le32_to_cpu(top->dp_type);
> + o->dp_id_select = le32_to_cpu(top->dp_id_select);
> + o->dp_nr_ids = le32_to_cpu(top->dp_nr_ids);
> + for (i = 0; i < o->dp_nr_ids; i++)
> + o->dp_ids[i] = le32_to_cpu(top->dp_ids[i]);
> #if 0
> uint8_t cpumask[FIO_TOP_STR_MAX];
> uint8_t verify_cpumask[FIO_TOP_STR_MAX];
> @@ -652,10 +653,11 @@ void convert_thread_options_to_net(struct thread_options_pack *top,
> top->merge_blktrace_iters[i].u.i = __cpu_to_le64(fio_double_to_uint64(o->merge_blktrace_iters[i].u.f));
>
> top->fdp = cpu_to_le32(o->fdp);
> - top->fdp_pli_select = cpu_to_le32(o->fdp_pli_select);
> - top->fdp_nrpli = cpu_to_le32(o->fdp_nrpli);
> - for (i = 0; i < o->fdp_nrpli; i++)
> - top->fdp_plis[i] = cpu_to_le32(o->fdp_plis[i]);
> + top->dp_type = cpu_to_le32(o->dp_type);
> + top->dp_id_select = cpu_to_le32(o->dp_id_select);
> + top->dp_nr_ids = cpu_to_le32(o->dp_nr_ids);
> + for (i = 0; i < o->dp_nr_ids; i++)
> + top->dp_ids[i] = cpu_to_le32(o->dp_ids[i]);
> #if 0
> uint8_t cpumask[FIO_TOP_STR_MAX];
> uint8_t verify_cpumask[FIO_TOP_STR_MAX];
> diff --git a/dataplacement.c b/dataplacement.c
> index 7518d193..a7170863 100644
> --- a/dataplacement.c
> +++ b/dataplacement.c
> @@ -59,13 +59,13 @@ static int init_ruh_info(struct thread_data *td, struct fio_file *f)
> if (ruhs->nr_ruhs > FDP_MAX_RUHS)
> ruhs->nr_ruhs = FDP_MAX_RUHS;
>
> - if (td->o.fdp_nrpli == 0) {
> + if (td->o.dp_nr_ids == 0) {
> f->ruhs_info = ruhs;
> return 0;
> }
>
> - for (i = 0; i < td->o.fdp_nrpli; i++) {
> - if (td->o.fdp_plis[i] >= ruhs->nr_ruhs) {
> + for (i = 0; i < td->o.dp_nr_ids; i++) {
> + if (td->o.dp_ids[i] >= ruhs->nr_ruhs) {
> ret = -EINVAL;
> goto out;
> }
> @@ -77,9 +77,9 @@ static int init_ruh_info(struct thread_data *td, struct fio_file *f)
> goto out;
> }
>
> - tmp->nr_ruhs = td->o.fdp_nrpli;
> - for (i = 0; i < td->o.fdp_nrpli; i++)
> - tmp->plis[i] = ruhs->plis[td->o.fdp_plis[i]];
> + tmp->nr_ruhs = td->o.dp_nr_ids;
> + for (i = 0; i < td->o.dp_nr_ids; i++)
> + tmp->plis[i] = ruhs->plis[td->o.dp_ids[i]];
> f->ruhs_info = tmp;
> out:
> sfree(ruhs);
> @@ -119,7 +119,7 @@ void dp_fill_dspec_data(struct thread_data *td, struct io_u *io_u)
> return;
> }
>
> - if (td->o.fdp_pli_select == FIO_FDP_RR) {
> + if (td->o.dp_id_select == FIO_DP_RR) {
> if (ruhs->pli_loc >= ruhs->nr_ruhs)
> ruhs->pli_loc = 0;
>
> diff --git a/dataplacement.h b/dataplacement.h
> index 72bd4c08..b6ceb5bc 100644
> --- a/dataplacement.h
> +++ b/dataplacement.h
> @@ -5,15 +5,22 @@
>
> #define FDP_DIR_DTYPE 2
> #define FDP_MAX_RUHS 128
> +#define FIO_MAX_DP_IDS 16
>
> /*
> * How fio chooses what placement identifier to use next. Choice of
> * uniformly random, or roundrobin.
> */
> +enum {
> + FIO_DP_RANDOM = 0x1,
> + FIO_DP_RR = 0x2,
> +};
> +
>
> enum {
> - FIO_FDP_RANDOM = 0x1,
> - FIO_FDP_RR = 0x2,
> + FIO_DP_NONE = 0x0,
> + FIO_DP_FDP = 0x1,
> + FIO_DP_STREAMS = 0x2,
> };
>
> struct fio_ruhs_info {
> diff --git a/filesetup.c b/filesetup.c
> index 8923f2b3..6fbfced5 100644
> --- a/filesetup.c
> +++ b/filesetup.c
> @@ -1411,7 +1411,7 @@ done:
>
> td_restore_runstate(td, old_state);
>
> - if (td->o.fdp) {
> + if (td->o.dp_type == FIO_DP_FDP) {
> err = dp_init(td);
> if (err)
> goto err_out;
> diff --git a/init.c b/init.c
> index 7a0b14a3..ff3e9a90 100644
> --- a/init.c
> +++ b/init.c
> @@ -1015,7 +1015,15 @@ static int fixup_options(struct thread_data *td)
> ret |= 1;
> }
>
> -
> + if (td->o.fdp) {
> + if (fio_option_is_set(&td->o, dp_type) &&
> + (td->o.dp_type == FIO_DP_STREAMS || td->o.dp_type == FIO_DP_NONE)) {
> + log_err("fio: fdp=1 is not compatible with dataplacement={streams, none}\n");
> + ret |= 1;
> + } else {
> + td->o.dp_type = FIO_DP_FDP;
> + }
> + }
> return ret;
> }
>
> diff --git a/io_u.c b/io_u.c
> index 735d9005..e9b94c48 100644
> --- a/io_u.c
> +++ b/io_u.c
> @@ -1065,7 +1065,7 @@ static int fill_io_u(struct thread_data *td, struct io_u *io_u)
> }
> }
>
> - if (td->o.fdp)
> + if (td->o.dp_type == FIO_DP_FDP)
> dp_fill_dspec_data(td, io_u);
>
> if (io_u->offset + io_u->buflen > io_u->file->real_file_size) {
> diff --git a/options.c b/options.c
> index de935efc..09a24652 100644
> --- a/options.c
> +++ b/options.c
> @@ -270,12 +270,12 @@ static int str_fdp_pli_cb(void *data, const char *input)
> strip_blank_front(&str);
> strip_blank_end(str);
>
> - while ((v = strsep(&str, ",")) != NULL && i < FIO_MAX_PLIS)
> - td->o.fdp_plis[i++] = strtoll(v, NULL, 0);
> + while ((v = strsep(&str, ",")) != NULL && i < FIO_MAX_DP_IDS)
> + td->o.dp_ids[i++] = strtoll(v, NULL, 0);
> free(p);
>
> - qsort(td->o.fdp_plis, i, sizeof(*td->o.fdp_plis), fio_fdp_cmp);
> - td->o.fdp_nrpli = i;
> + qsort(td->o.dp_ids, i, sizeof(*td->o.dp_ids), fio_fdp_cmp);
> + td->o.dp_nr_ids = i;
>
> return 0;
> }
> @@ -3710,32 +3710,58 @@ struct fio_option fio_options[FIO_MAX_OPTS] = {
> .group = FIO_OPT_G_INVALID,
> },
> {
> - .name = "fdp_pli_select",
> - .lname = "FDP Placement ID select",
> + .name = "dataplacement",
> + .lname = "Data Placement interface",
> .type = FIO_OPT_STR,
> - .off1 = offsetof(struct thread_options, fdp_pli_select),
> - .help = "Select which FDP placement ID to use next",
> + .off1 = offsetof(struct thread_options, dp_type),
> + .help = "Data Placement interface to use",
> + .def = "none",
> + .category = FIO_OPT_C_IO,
> + .group = FIO_OPT_G_INVALID,
> + .posval = {
> + { .ival = "none",
> + .oval = FIO_DP_NONE,
> + .help = "Do not specify a data placement interface",
> + },
> + { .ival = "fdp",
> + .oval = FIO_DP_FDP,
> + .help = "Use Flexible Data Placement interface",
> + },
> + { .ival = "streams",
> + .oval = FIO_DP_STREAMS,
> + .help = "Use Streams interface",
> + },
> + },
> + },
> + {
> + .name = "plid_select",
> + .alias = "fdp_pli_select",
> + .lname = "Data Placement ID selection strategy",
> + .type = FIO_OPT_STR,
> + .off1 = offsetof(struct thread_options, dp_id_select),
> + .help = "Strategy for selecting next Data Placement ID",
> .def = "roundrobin",
> .category = FIO_OPT_C_IO,
> .group = FIO_OPT_G_INVALID,
> .posval = {
> { .ival = "random",
> - .oval = FIO_FDP_RANDOM,
> + .oval = FIO_DP_RANDOM,
> .help = "Choose a Placement ID at random (uniform)",
> },
> { .ival = "roundrobin",
> - .oval = FIO_FDP_RR,
> + .oval = FIO_DP_RR,
> .help = "Round robin select Placement IDs",
> },
> },
> },
> {
> - .name = "fdp_pli",
> - .lname = "FDP Placement ID indicies",
> + .name = "plids",
> + .alias = "fdp_pli",
> + .lname = "Stream IDs/Data Placement ID indices",
> .type = FIO_OPT_STR,
> .cb = str_fdp_pli_cb,
> - .off1 = offsetof(struct thread_options, fdp_plis),
> - .help = "Sets which placement ids to use (defaults to all)",
> + .off1 = offsetof(struct thread_options, dp_ids),
> + .help = "Sets which Data Placement ids to use (defaults to all for FDP)",
> .hide = 1,
> .category = FIO_OPT_C_IO,
> .group = FIO_OPT_G_INVALID,
> diff --git a/server.h b/server.h
> index 6d2659b0..83ce449b 100644
> --- a/server.h
> +++ b/server.h
> @@ -51,7 +51,7 @@ struct fio_net_cmd_reply {
> };
>
> enum {
> - FIO_SERVER_VER = 103,
> + FIO_SERVER_VER = 104,
>
> FIO_SERVER_MAX_FRAGMENT_PDU = 1024,
> FIO_SERVER_MAX_CMD_MB = 2048,
> diff --git a/thread_options.h b/thread_options.h
> index c2e71518..a36b7909 100644
> --- a/thread_options.h
> +++ b/thread_options.h
> @@ -391,11 +391,11 @@ struct thread_options {
> fio_fp64_t zrt;
> fio_fp64_t zrf;
>
> -#define FIO_MAX_PLIS 16
> unsigned int fdp;
> - unsigned int fdp_pli_select;
> - unsigned int fdp_plis[FIO_MAX_PLIS];
> - unsigned int fdp_nrpli;
> + unsigned int dp_type;
> + unsigned int dp_id_select;
> + unsigned int dp_ids[FIO_MAX_DP_IDS];
> + unsigned int dp_nr_ids;
>
> unsigned int log_entries;
> unsigned int log_prio;
> @@ -709,9 +709,10 @@ struct thread_options_pack {
> uint32_t log_prio;
>
> uint32_t fdp;
> - uint32_t fdp_pli_select;
> - uint32_t fdp_plis[FIO_MAX_PLIS];
> - uint32_t fdp_nrpli;
> + uint32_t dp_type;
> + uint32_t dp_id_select;
> + uint32_t dp_ids[FIO_MAX_DP_IDS];
> + uint32_t dp_nr_ids;
>
> uint32_t num_range;
> /*
> --
> 2.43.0
>
>
next prev parent reply other threads:[~2024-04-23 8:46 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-22 17:49 [PATCH 0/9] FDP tweaks, NVMe streams support Vincent Fu
2024-04-22 17:49 ` [PATCH 1/9] fio: rename fdp.[c,h] to dataplacement.[c,h] Vincent Fu
2024-04-22 17:49 ` [PATCH 2/9] fio: create over-arching data placement option Vincent Fu
2024-04-23 8:36 ` Ankit Kumar [this message]
2024-04-23 15:27 ` Vincent Fu
2024-04-22 17:49 ` [PATCH 3/9] t/nvmept_fdp.py: test script for FDP Vincent Fu
2024-04-22 17:49 ` [PATCH 4/9] fio: support NVMe streams Vincent Fu
2024-04-22 17:49 ` [PATCH 5/9] options: reject placement IDs larger than the max Vincent Fu
2024-04-22 17:49 ` [PATCH 6/9] options: parse placement IDs as unsigned values Vincent Fu
2024-04-22 17:49 ` [PATCH 7/9] dataplacement: add a debug print for IOs Vincent Fu
2024-04-22 17:49 ` [PATCH 8/9] t/nvmept_streams: test NVMe streams support Vincent Fu
2024-04-22 17:49 ` [PATCH 9/9] docs: update for new data placement options Vincent Fu
2024-04-23 8:49 ` [PATCH 0/9] FDP tweaks, NVMe streams support Ankit Kumar
2024-04-23 15:16 ` Vincent Fu
2024-04-24 18:48 ` Vincent Fu
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='CAKi7+wcrvg4_ouCU2HUEWjA-TKwbTkc0xHjq5fGiufYw=hQWig@mail.gmail.com' \
--to=ankit1455@gmail.com \
--cc=ankit.kumar@samsung.com \
--cc=axboe@kernel.dk \
--cc=fio@vger.kernel.org \
--cc=kbusch@kernel.org \
--cc=vincent.fu@samsung.com \
--cc=vincentfu@gmail.com \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).