On 16/03/2023 14:40, Tudor Cretu wrote:
[...]
@@ -726,7 +726,7 @@ static __cold void io_uring_drop_tctx_refs(struct task_struct *task) } } -static bool io_cqring_event_overflow(struct io_ring_ctx *ctx, u64 user_data, +static bool io_cqring_event_overflow(struct io_ring_ctx *ctx, __kernel_uintptr_t user_data,
Looking at how user_data is used, I see that it is passed on to a couple of trace functions (here and in io_fill_cqe_aux()). Should we extend the trace events accordingly? Would that be useful at all? We don't necessarily need an answer now, maybe that's to be investigated later.
We could propagate the capability there, what format specifier would be appropriate for a __kernel_uintptr_t though instead of %llx? I don't think printing capability metadata would be really worth it, but I don't like the idea of keeping user_data as a u64 there either. So, maybe I just propagate to __kernel_uintptr_t in the trace functions as well and keep %llx?
I agree printing capability metadata is unlikely to be very helpful. I would say it's fine to keep the trace functions as-is, I would only change them to take __kernel_uintptr_t if they actually printed more than just a u64.
[...]
diff --git a/io_uring/poll.c b/io_uring/poll.c index d9bf1767867e6..4914048fed0f8 100644 --- a/io_uring/poll.c +++ b/io_uring/poll.c @@ -22,8 +22,8 @@ struct io_poll_update { struct file *file; - u64 old_user_data; - u64 new_user_data; + __kernel_uintptr_t old_user_data; + __kernel_uintptr_t new_user_data;
Looking at how new_user_data is used, I see that it is set to sqe->off, which is clearly not what we want in PCuABI (since off is a __u64). I suppose we should have the same change as in msg_ring.c and set it to sqe->addr2 instead?
That's right. Btw, the man pages mention the flag IORING_POLL_UPDATE_USER_DATA at the IORING_OP_POLL_ADD section, but liburing confirms that the flag is used only with IORING_OP_POLL_REMOVE, so that was a bit confusing.
Oh yes that's clearly a mistake in the man page, IORING_POLL_UPDATE_* don't make sense for IORING_OP_POLL_ADD! Since the man pages actually come from liburing itself, would be a nice patch to add there (and eventually upstream).
Kevin