[PATCH] io_uring: don't hold fixed_file_data's lock when registering files

[PATCH] io_uring: don't hold fixed_file_data's lock when registering files

[Xiaoguang Wang]

While registering new files by IORING_REGISTER_FILES, there're not
valid fixed_file_ref_node at the moment, so it's unnecessary to hold
fixed_file_data's lock when registering files.

Signed-off-by: Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com>
---
 fs/io_uring.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 186072861af9..75841f3cb2f7 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -7257,9 +7257,7 @@ static int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg,
 	}
 
 	ctx->file_data->cur_refs = &ref_node->refs;
-	spin_lock(&ctx->file_data->lock);
 	list_add(&ref_node->node, &ctx->file_data->ref_list);
-	spin_unlock(&ctx->file_data->lock);
 	percpu_ref_get(&ctx->file_data->refs);
 	return ret;
 }
-- 
2.17.2

Re: [PATCH] io_uring: don't hold fixed_file_data's lock when registering files

[Jens Axboe]

On 9/2/20 5:32 AM, Xiaoguang Wang wrote:
> While registering new files by IORING_REGISTER_FILES, there're not
> valid fixed_file_ref_node at the moment, so it's unnecessary to hold
> fixed_file_data's lock when registering files.

Even if that were the case (I haven't looked too closely at it yet),
it would a) need a big comment explaining why, and b) some justification
on why this would be a change we'd want to make.

On b, are you seeing any tangible differences with this?

-- 
Jens Axboe

Re: [PATCH] io_uring: don't hold fixed_file_data's lock when registering files

[Xiaoguang Wang]

hi，

> On 9/2/20 5:32 AM, Xiaoguang Wang wrote:
>> While registering new files by IORING_REGISTER_FILES, there're not
>> valid fixed_file_ref_node at the moment, so it's unnecessary to hold
>> fixed_file_data's lock when registering files.
> 
> Even if that were the case (I haven't looked too closely at it yet),
> it would a) need a big comment explaining why, and b) some justification
> on why this would be a change we'd want to make.
> 
> On b, are you seeing any tangible differences with this?
No, just found this by reading source codes.

Regards,
Xiaoguang Wang

> 

Re: [PATCH] io_uring: don't hold fixed_file_data's lock when registering files

[Jens Axboe]

On 9/2/20 10:35 PM, Xiaoguang Wang wrote:
> hi，
> 
>> On 9/2/20 5:32 AM, Xiaoguang Wang wrote:
>>> While registering new files by IORING_REGISTER_FILES, there're not
>>> valid fixed_file_ref_node at the moment, so it's unnecessary to hold
>>> fixed_file_data's lock when registering files.
>>
>> Even if that were the case (I haven't looked too closely at it yet),
>> it would a) need a big comment explaining why, and b) some justification
>> on why this would be a change we'd want to make.
>>
>> On b, are you seeing any tangible differences with this?
> No, just found this by reading source codes.

OK, then I suggest we leave it as it is. The data race analyzers wouldn't
be very happy with it either.

-- 
Jens Axboe