Re: [PATCH v9 06/11] io_uring: introduce attributes for read/write and PI support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/18/24 12:50, Christoph Hellwig wrote:
On Sat, Nov 16, 2024 at 12:32:25AM +0000, Pavel Begunkov wrote:
We can also reuse your idea from your previous iterations and
use the bitmap to list all attributes. Then preamble and
the explicit attr_type field are not needed, type checking
in the loop is removed and packing is better. And just
by looking at the map we can calculate the size of the
array and remove all size checks in the loop.

Can we please stop overdesigning the f**k out of this?  Really,

Please stop it, it doesn't add weight to your argument. The design
requirement has never changed, at least not during this patchset
iterations.

either we're fine using the space in the extended SQE, or
we're fine using a separate opcode, or if we really have to just
make it uring_cmd.  But stop making thing being extensible for
the sake of being extensible.

It's asked to be extendible because there is a good chance it'll need to
be extended, and no, I'm not suggesting anyone to implement the entire
thing, only PI bits is fine.

And no, it doesn't have to be "this or that" while there are other
options suggested for consideration. And the problem with the SQE128
option is not even about SQE128 but how it's placed inside, i.e.
at a fixed spot.

Do we have technical arguments against the direction in the last
suggestion? It's extendible and _very_ simple. The entire (generic)
handling for the bitmask approach for this set would be sth like:

struct sqe {
	u64 attr_type_mask;
	u64 attr_ptr;
};
if (sqe->attr_type_mask) {
	if (sqe->attr_type_mask != TYPE_PI)
		return -EINVAL;

	struct uapi_pi_structure pi;
	copy_from_user(&pi, sqe->attr_ptr, sizeof(pi));
	hanlde_pi(&pi);
}

And the user side:

struct uapi_pi_structure pi = { ... };
sqe->attr_ptr = π
sqe->attr_type_mask = TYPE_PI;

--
Pavel Begunkov




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux