Re: [PATCH] btrfs: add io_uring interface for encoded reads

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

 



On 8/13/24 01:49, David Sterba wrote:
On Mon, Aug 12, 2024 at 08:17:43PM +0100, Pavel Begunkov wrote:
On 8/12/24 17:58, David Sterba wrote:
On Mon, Aug 12, 2024 at 05:10:15PM +0100, Pavel Begunkov wrote:
And the last point, I'm surprised there are two versions of
btrfs_ioctl_encoded_io_args. Maybe, it's a good moment to fix it if
we're creating a new interface.

E.g. by adding a new structure defined right with u64 and such, use it
in io_uring, and cast to it in the ioctl code when it's x64 (with
a good set of BUILD_BUG_ON sprinkled) and convert structures otherwise?

If you mean the 32bit version of the ioctl struct
(btrfs_ioctl_encoded_io_args_32), I don't think we can fix it. It's been

Right, I meant btrfs_ioctl_encoded_io_args_32. And to clarify, nothing
can be done for the ioctl(2) part, I only suggested to have a single
structure when it comes to io_uring.

there from the beginning and it's not a mistake. I don't remember the
details why and only vaguely remember that I'd asked why we need it.
Similar 64/32 struct is in the send ioctl but that was a mistake due to
a pointer being passed in the structure and that needs to be handled due
to different type width.

Would be interesting to learn why, maybe Omar remembers? Only two
fields are not explicitly sized, both could've been just u64.
The structure iov points to (struct iovec) would've had a compat
flavour, but that doesn't require a separate
btrfs_ioctl_encoded_io_args.

Found it:

"why don't we avoid the send 32bit workaround"
https://lore.kernel.org/linux-btrfs/20190828120650.GZ2752@xxxxxxxxxxxxx/

"because big-endian"
https://lore.kernel.org/linux-btrfs/20190903171458.GA7452@vader/

union {
	void __user *buf;
	__u64 __buf_alignment;
};

Endianness is indeed a problem here, but I don't see the
purpose of aliasing it with a pointer instead of keeping
just u64, which is a common pattern.

struct btrfs_ioctl_encoded_io_args {
	__u64 buf;
	...
};

// user
void *buf = ...;
arg.buf = (__u64)(uintptr_t)buf;

// kernel
void __user *p = u64_to_user_ptr(arg.buf);

--
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