On 8/24/23 10:28 AM, Pavel Begunkov wrote: > On 8/19/23 16:03, Jens Axboe wrote: >> On 8/15/23 11:31 AM, Pavel Begunkov wrote: >>> io_kiocb::cqe stores the completion info which we'll memcpy to >>> userspace, and we rely on callbacks and other later steps to populate >>> it with right values. We have never had problems with that, but it would >>> still be safer to zero it on allocation. >>> >>> Signed-off-by: Pavel Begunkov <asml.silence@xxxxxxxxx> >>> --- >>> io_uring/io_uring.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c >>> index e189158ebbdd..4d27655be3a6 100644 >>> --- a/io_uring/io_uring.c >>> +++ b/io_uring/io_uring.c >>> @@ -1056,7 +1056,7 @@ static void io_preinit_req(struct io_kiocb *req, struct io_ring_ctx *ctx) >>> req->link = NULL; >>> req->async_data = NULL; >>> /* not necessary, but safer to zero */ >>> - req->cqe.res = 0; >>> + memset(&req->cqe, 0, sizeof(req->cqe)); >>> } >>> static void io_flush_cached_locked_reqs(struct io_ring_ctx *ctx, >> >> I think this is a good idea, but I wonder if we should open-clear it >> instead. I've had cases in the past where that's more efficient than >> calling memset. > > I don't think it ever happens for 16 byte memcpy, and in either > case it's a cache refill, quite a slow path. I believe memcpy is > better here. Yeah I think it's fine as-is - just checked here and either approach yields the same. -- Jens Axboe
