RE: [PATCH 1/1] io_uring: clean up io_uring_register_get_file()

[lizetao <lizetao1@huawei.com>]

Hi,

> -----Original Message-----
> From: Jens Axboe <axboe@kernel.dk>
> Sent: Thursday, January 16, 2025 10:10 PM
> To: lizetao <lizetao1@huawei.com>; Pavel Begunkov <asml.silence@gmail.com>
> Cc: io-uring@vger.kernel.org
> Subject: Re: [PATCH 1/1] io_uring: clean up io_uring_register_get_file()
> 
> On 1/16/25 6:09 AM, lizetao wrote:
> > Hi,
> >
> >> -----Original Message-----
> >> From: Pavel Begunkov <asml.silence@gmail.com>
> >> Sent: Thursday, January 16, 2025 10:53 AM
> >> To: io-uring@vger.kernel.org
> >> Cc: asml.silence@gmail.com
> >> Subject: [PATCH 1/1] io_uring: clean up io_uring_register_get_file()
> >>
> >> Make it always reference the returned file. It's safer, especially
> >> with unregistrations happening under it. And it makes the api cleaner
> >> with no conditional clean ups by the caller.
> >>
> >> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> >> ---
> >>  io_uring/register.c | 6 ++++--
> >>  io_uring/rsrc.c     | 4 ++--
> >>  2 files changed, 6 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/io_uring/register.c b/io_uring/register.c index
> >> 5e48413706ac..a93c979c2f38 100644
> >> --- a/io_uring/register.c
> >> +++ b/io_uring/register.c
> >> @@ -841,6 +841,8 @@ struct file *io_uring_register_get_file(unsigned
> >> int fd, bool registered)
> >>  			return ERR_PTR(-EINVAL);
> >>  		fd = array_index_nospec(fd, IO_RINGFD_REG_MAX);
> >>  		file = tctx->registered_rings[fd];
> >> +		if (file)
> >> +			get_file(file);
> >
> > Should performance be a priority here?
> 
> Performance only really matters for high frequency invocations, of which the
> register part is not. So no, should not matter at all.
Ok. I got it.

Tested-by: Li Zetao <lizetao1@huawei.com>
Reviewed-by: Li Zetao <lizetao1@huawei.com>
> 
> --
> Jens Axboe

---
Li Zetao