Re: [PATCH 1/2] engines/io_uring: support r/w with metadata

[Vincent Fu <vincentfu@gmail.com>]

On 7/23/25 1:37 PM, Jens Axboe wrote:
>>> +	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.
> 

Thanks for the feedback. How about doing a strcmp at ioengine init time 
and setting a flag that we can check in the hot path?

Vincent