On 11/26/24 13:54, Anuj Gupta wrote:
On Tue, Nov 26, 2024 at 01:01:03PM +0000, Pavel Begunkov wrote:
On 11/25/24 07:06, Anuj Gupta wrote:
...
+ /* type specific struct here */
+ struct io_uring_attr_pi pi;
+};
This also looks PI specific but with a generic name. Or are
attribute structures are supposed to be unionised?
Yes, attribute structures would be unionised here. This is done so that
"attr_type" always remains at the top. When there are multiple attributes
this structure would look something like this:
/* attribute information along with type */
struct io_uring_attr {
enum io_uring_attr_type attr_type;
/* type specific struct here */
union {
struct io_uring_attr_pi pi;
struct io_uring_attr_x x;
struct io_uring_attr_y y;
};
};
And then on the application side for sending attribute x, one would do:
io_uring_attr attr;
attr.type = TYPE_X;
prepare_attr(&attr.x);
Hmm, I have doubts it's going to work well because the union
members have different sizes. Adding a new type could grow
struct io_uring_attr, which is already bad for uapi. And it
can't be stacked:
io_uring_attr attrs[2] = {..., ...}
sqe->attr_ptr = &attrs;
...
This example would be incorrect. Even if it's just one attribute
the user would be wasting space on stack. The only use for it I
see is having ephemeral pointers during parsing, ala
void parse(voud *attributes, offset) {
struct io_uring_attr *attr = attributes + offset;
if (attr->type == PI) {
process_pi(&attr->pi);
// or potentially fill_pi() in userspace
}
}
But I don't think it's worth it. I'd say, if you're leaving
the structure, let's rename it to struct io_uring_attr_type_pi
or something similar. We can always add a new one later, it
doesn't change the ABI.
--
Pavel Begunkov
