Re: [PATCH] io_uring: reduce/pack size of io_ring_ctx

[Jackie Liu <liuyun01@kylinos.cn>]

> 2019年11月8日 08:35，Jens Axboe <axboe@kernel.dk> 写道：
> 
> On 11/7/19 5:06 PM, Jens Axboe wrote:
>> On 11/7/19 5:00 PM, Jackie Liu wrote:
>>> This patch looks good, but I prefer sqo_thread_started instead of sqo_done,
>>> because we are marking the thread started, not the end of the thread.
>>> 
>>> Anyway, Reviewed-by: Jackie Liu <liuyun01@kylinos.cn>
>> 
>> Yeah, let's retain the old name. I'll make that change and add your
>> reviewed-by, thanks.
> 
> Actually, would you mind if we just make it ->completions[2] instead?
> That saves a kmalloc per ctx setup, I think that's worthwhile enough
> to bundle them together:
> 
> 
> commit 3b830211e99976650d5da0613dfca105c5007f8b
> Author: Jens Axboe <axboe@kernel.dk>
> Date:   Thu Nov 7 17:27:39 2019 -0700
> 
>    io_uring: reduce/pack size of io_ring_ctx
> 
>    With the recent flurry of additions and changes to io_uring, the
>    layout of io_ring_ctx has become a bit stale. We're right now at
>    704 bytes in size on my x86-64 build, or 11 cachelines. This
>    patch does two things:
> 
>    - We have to completion structs embedded, that we only use for
>      quiesce of the ctx (or shutdown) and for sqthread init cases.
>      That 2x32 bytes right there, let's dynamically allocate them.
> 
>    - Reorder the struct a bit with an eye on cachelines, use cases,
>      and holes.
> 
>    With this patch, we're down to 512 bytes, or 8 cachelines.
> 
>    Reviewed-by: Jackie Liu <liuyun01@kylinos.cn>
>    Signed-off-by: Jens Axboe <axboe@kernel.dk>
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 4c488bf6e889..2b784262eaff 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -213,24 +213,13 @@ struct io_ring_ctx {
> 		wait_queue_head_t	inflight_wait;
> 	} ____cacheline_aligned_in_smp;
> 
> +	struct io_rings	*rings;
> +
> 	/* IO offload */
> 	struct io_wq		*io_wq;
> 	struct task_struct	*sqo_thread;	/* if using sq thread polling */
> 	struct mm_struct	*sqo_mm;
> 	wait_queue_head_t	sqo_wait;
> -	struct completion	sqo_thread_started;
> -
> -	struct {
> -		unsigned		cached_cq_tail;
> -		unsigned		cq_entries;
> -		unsigned		cq_mask;
> -		atomic_t		cq_timeouts;
> -		struct wait_queue_head	cq_wait;
> -		struct fasync_struct	*cq_fasync;
> -		struct eventfd_ctx	*cq_ev_fd;
> -	} ____cacheline_aligned_in_smp;
> -
> -	struct io_rings	*rings;
> 
> 	/*
> 	 * If used, fixed file set. Writers must ensure that ->refs is dead,
> @@ -246,7 +235,22 @@ struct io_ring_ctx {
> 
> 	struct user_struct	*user;
> 
> -	struct completion	ctx_done;
> +	/* 0 is for ctx quiesce/reinit/free, 1 is for sqo_thread started */
> +	struct completion	*completions;
> +

I think it's okay, it's clear through comments here.

--
BR, Jackie Liu