Re: [PATCH v2 3/8] nvme: add nvme opcodes, structures and helper functions

[Jens Axboe <axboe@kernel.dk>]

On 5/27/22 1:29 AM, Kanchan Joshi wrote:
> On Thu, May 26, 2022 at 08:18:04PM +0530, Ankit Kumar wrote:
> 
>> +void fio_nvme_uring_cmd_prep(struct nvme_uring_cmd *cmd, struct io_u *io_u,
>> +                 struct iovec *iov)
>> +{
>> +    struct nvme_data *data = FILE_ENG_DATA(io_u->file);
>> +    __u64 slba;
>> +    __u32 nlb;
>> +
>> +    slba = io_u->offset / data->lba_size;
>> +    nlb = (io_u->xfer_buflen / data->lba_size) - 1;
>> +
>> +    memset(cmd, 0, sizeof(struct nvme_uring_cmd));
> 
> Is this better or setting remaining fields (which are not populated
> down) to zero.

Since lba_size is a power of 2, it would be a lot more efficient to take
this division by non-constant out of the fast path and init a shift
value:

data->lba_shift = log2(data->lba_size);

and change these to:

slba = io_u->offset >> data->lba_shift;

and ditto for nlb.

>> +    /* cdw10 and cdw11 represent starting lba */
>> +    cmd->cdw10 = slba & 0xffffffff;
>> +    cmd->cdw11 = slba >> 32;
>> +    /* cdw12 represent number of lba's for read/write */
>> +    cmd->cdw12 = nlb;
>> +    if (iov) {
>> +        iov->iov_base = io_u->xfer_buf;
>> +        iov->iov_len = io_u->xfer_buflen;
>> +        cmd->addr = (__u64)(uintptr_t)iov;
>> +        cmd->data_len = 1;
> 
> Is this correct? Do we always get 1 vector to deal with.

Yes

-- 
Jens Axboe