From: Jens Axboe <axboe@kernel.dk>
To: Vincent Fu <vincentfu@gmail.com>,
anuj1072538@gmail.com, hch@infradead.org, joshi.k@samsung.com,
anuj20.g@samsung.com, fio@vger.kernel.org
Cc: Vincent Fu <vincent.fu@samsung.com>
Subject: Re: [PATCH 1/2] engines/io_uring: support r/w with metadata
Date: Wed, 23 Jul 2025 11:37:05 -0600 [thread overview]
Message-ID: <e548d5f8-9ebe-46cf-9f43-79be601d2057@kernel.dk> (raw)
In-Reply-To: <15cb4581-7067-40fa-9a1b-8883c9993d2c@kernel.dk>
>> + if (!strcmp(td->io_ops->name, "io_uring") && o->md_per_io_size) {
>> + struct nvme_data *data = FILE_ENG_DATA(io_u->file);
>> + struct nvme_cmd_ext_io_opts ext_opts = {0};
>> +
>> + if (data->pi_type) {
>> + if (o->pi_act)
>> + ext_opts.io_flags |= NVME_IO_PRINFO_PRACT;
>> +
>> + ext_opts.io_flags |= o->prchk;
>> + ext_opts.apptag = o->apptag;
>> + ext_opts.apptag_mask = o->apptag_mask;
>> + }
>> + fio_nvme_generate_guard(io_u, &ext_opts);
>> + }
>
> Ehh a strcmp() in the hot path?! First of all, that's a big no-no.
> Secondly, if this really was required, you'd add something to put that
> strcmp() in the slow path and flag it. Lastly, thankfully this should be
> much better as:
>
> if (td->io_ops == &ioengine_uring ...)
>
> instead.
Eh I guess dynamically loaded engines would need special treatment. I'll
take a look. In any case, strcmp() is just too ugly to live, actually
quite a few in there and the io_uring engine is the only one that thinks
this is necessary.
--
Jens Axboe
next prev parent reply other threads:[~2025-07-23 17:37 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-23 17:04 [PATCH 0/2] io_uring r/w with metadata Vincent Fu
2025-07-23 17:04 ` [PATCH 1/2] engines/io_uring: support " Vincent Fu
2025-07-23 17:23 ` Jens Axboe
2025-07-23 17:37 ` Jens Axboe [this message]
2025-07-23 18:28 ` Vincent Fu
2025-07-23 19:23 ` Jens Axboe
2025-07-23 17:04 ` [PATCH 2/2] t/io_uring_pi: test script for io_uring PI 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=e548d5f8-9ebe-46cf-9f43-79be601d2057@kernel.dk \
--to=axboe@kernel.dk \
--cc=anuj1072538@gmail.com \
--cc=anuj20.g@samsung.com \
--cc=fio@vger.kernel.org \
--cc=hch@infradead.org \
--cc=joshi.k@samsung.com \
--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