On Fri, Feb 10, 2023 at 11:59:11AM +0530, Ankit Kumar wrote:
> > +.. option:: fdp=bool : [io_uring_cmd]
> > +
> > + Enable Flexible Data Placement mode for write commands.
> > +
> > +.. option:: fdp_pli=int[,int][,int] : [io_uring_cmd]
> I think it probably should just be "fdp_pli=str"
> Kind of similar to the fio "option cpus_allowed=str"
...
> > + Select which Placement ID Index/Indicies this job is allowed to use for
> > + writes. By default, the job will cycle through all available Placement
> > + IDs, so use this to isolate these identifiers to specific jobs..
> Maybe we can also add one line description something like this:
> If you want fio to use placement identifier only at indices 0,2 and 5 specify
> ``fdp_pli=0,2,5``.
Sure, definitely don't need to be sequential. Also, like cpus_allowed, perhaps
ranges are desirable (ex: 0-2,5).
> > +int fio_nvme_iomgmt_ruhs(struct thread_data *td, struct fio_file *f,
> > + struct nvme_fdp_ruh_status *ruhs, __u32 bytes)
> > +{
> > + struct nvme_data *data = FILE_ENG_DATA(f);
> > + int fd, ret;
> > +
> > + fd = open(f->file_name, O_RDONLY | O_LARGEFILE);
> > + if (fd < 0)
> > + return -errno;
> > +
> > + ret = nvme_fdp_reclaim_unit_handle_status(fd, data->nsid, bytes, ruhs);
> > + if (ret) {
> > + log_err("%s: nvme_fdp_reclaim_unit_handle_status failed, err=%d\n",
> > + f->file_name, ret);
> > + errno = ENOTSUP;
> > + }
> > +
> > + close(fd);
> > + return -errno;
> should be "return ret;"
The return needs to be a negative number, and 'ret' can sometimes be positive
or negative.
> > +/*
> > + * Maximum number of RUHS to fetch
> > + * TODO: Revisit this so that we can work with more than 1024 RUH's
> > + */
> For me, I will send a fix once these changes get merged.
Could you help clarify some spec confusion on my side? The Namespace Management
command allows you to specify at most 128 placement handles when you create the
namespace, so why would we need more than that? Under what conditions could the
namespaces' IO Mgmt Recv RUH Status ever return more than 128 descriptors?
And speaking of that, since 128 is the largest value, it seems odd the spec
folks gave this field two bytes.
