Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 21 Jul 2023 13:30:31 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Jens Axboe <axboe@kernel.dk>
Cc:     io-uring@vger.kernel.org, linux-kernel@vger.kernel.org,
        andres@anarazel.de
Subject: Re: [PATCH 06/10] io_uring: add support for futex wake and wait
Message-ID: <20230721113031.GG3630545@hirez.programming.kicks-ass.net>
References: <20230720221858.135240-1-axboe@kernel.dk>
 <20230720221858.135240-7-axboe@kernel.dk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230720221858.135240-7-axboe@kernel.dk>
Precedence: bulk

On Thu, Jul 20, 2023 at 04:18:54PM -0600, Jens Axboe wrote:


> +struct io_futex {
> +	struct file	*file;
> +	u32 __user	*uaddr;
> +	unsigned int	futex_val;
> +	unsigned int	futex_flags;
> +	unsigned int	futex_mask;
> +};

So in the futex patches I just posted I went with 'unsigned long'
(syscall) or 'u64' (data structures) for the futex, such that, on 64bit
platforms, we might support 64bit futexes in the future (I still need to
audit the whole futex internals and convert u32 to unsigned long in
order to enable that).

So would something like:

struct io_futex {
	struct file	*file;
	void __user	*uaddr;
	u64		futex_val;
	u64		futex_mask;
	u32		futex_flags;
};

work to match the futex2 syscalls?


> +int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> +{
> +	struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
> +
> +	if (unlikely(sqe->fd || sqe->addr2 || sqe->buf_index || sqe->addr3))
> +		return -EINVAL;
> +
> +	iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
> +	iof->futex_val = READ_ONCE(sqe->len);
> +	iof->futex_mask = READ_ONCE(sqe->file_index);
> +	iof->futex_flags = READ_ONCE(sqe->futex_flags);

sqe->addr,		u64
sqe->len,		u32
sqe->file_index,	u32
sqe->futex_flags,	u32

> +	if (iof->futex_flags & FUTEX_CMD_MASK)

		FUTEX2_MASK

(which would need lifting from syscall.c to kernel/futex/futex.h I
suppose)

> +		return -EINVAL;
> +
> +	return 0;
> +}

> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
> index 36f9c73082de..3bd2d765f593 100644
> --- a/include/uapi/linux/io_uring.h
> +++ b/include/uapi/linux/io_uring.h
> @@ -65,6 +65,7 @@ struct io_uring_sqe {
>  		__u32		xattr_flags;
>  		__u32		msg_ring_flags;
>  		__u32		uring_cmd_flags;
> +		__u32		futex_flags;
>  	};
>  	__u64	user_data;	/* data to be passed back at completion time */
>  	/* pack this to avoid bogus arm OABI complaints */

Perhaps extend it like so?


diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 08720c7bd92f..c1d28bf64d11 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -35,6 +35,7 @@ struct io_uring_sqe {
 	union {
 		__u64	off;	/* offset into file */
 		__u64	addr2;
+		__u64	futex_val;
 		struct {
 			__u32	cmd_op;
 			__u32	__pad1;
@@ -65,6 +66,7 @@ struct io_uring_sqe {
 		__u32		xattr_flags;
 		__u32		msg_ring_flags;
 		__u32		uring_cmd_flags;
+		__u32		futex_flags;
 	};
 	__u64	user_data;	/* data to be passed back at completion time */
 	/* pack this to avoid bogus arm OABI complaints */
@@ -87,6 +89,7 @@ struct io_uring_sqe {
 	union {
 		struct {
 			__u64	addr3;
+			__u64	futex_mask;
 			__u64	__pad2[1];
 		};
 		/*


So that we can write something roughtly like:

	iof->uaddr = sqe->addr;
	iof->val   = sqe->futex_val;
	iof->mask  = sqe->futex_mask;
	iof->flags = sqe->futex_flags;

	if (iof->flags & ~FUTEX2_MASK)
		return -EINVAL;