[PATCH for-5.15 0/3] small code clean

[PATCH for-5.15 0/3] small code clean

[Hao Xu]

Some small code clean.

Hao Xu (3):
  io_uring: extract io_uring_files_cancel() in io_uring_task_cancel()
  io_uring: remove files pointer in cancellation functions
  io_uring: code clean for completion_lock in io_arm_poll_handler()

 fs/io_uring.c            | 15 ++++++---------
 include/linux/io_uring.h |  9 +++++----
 kernel/exit.c            |  2 +-
 3 files changed, 12 insertions(+), 14 deletions(-)

-- 
2.24.4

[PATCH 1/3] io_uring: extract io_uring_files_cancel() in io_uring_task_cancel()

[Hao Xu]

Extract io_uring_files_cancel() call in io_uring_task_cancel() to make
io_uring_files_cancel() and io_uring_task_cancel() coherent and easy to
read.

Signed-off-by: Hao Xu <haoxu@linux.alibaba.com>
---
 include/linux/io_uring.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/io_uring.h b/include/linux/io_uring.h
index 04b650bcbbe5..ed13304e764c 100644
--- a/include/linux/io_uring.h
+++ b/include/linux/io_uring.h
@@ -17,7 +17,8 @@ static inline void io_uring_files_cancel(struct files_struct *files)
 }
 static inline void io_uring_task_cancel(void)
 {
-	return io_uring_files_cancel(NULL);
+	if (current->io_uring)
+		__io_uring_cancel(NULL);
 }
 static inline void io_uring_free(struct task_struct *tsk)
 {
-- 
2.24.4

[PATCH 2/3] io_uring: remove files pointer in cancellation functions

[Hao Xu]

When doing cancellation, we use a parameter to indicate where it's from
do_exit or exec. So a boolean value is good enough for this, remove the
struct files* as it is not necessary.

Signed-off-by: Hao Xu <haoxu@linux.alibaba.com>
---
 fs/io_uring.c            | 4 ++--
 include/linux/io_uring.h | 8 ++++----
 kernel/exit.c            | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index efd818419014..b29774aa1f09 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -9205,9 +9205,9 @@ static void io_uring_cancel_generic(bool cancel_all, struct io_sq_data *sqd)
 	}
 }
 
-void __io_uring_cancel(struct files_struct *files)
+void __io_uring_cancel(bool cancel_all)
 {
-	io_uring_cancel_generic(!files, NULL);
+	io_uring_cancel_generic(cancel_all, NULL);
 }
 
 static void *io_uring_validate_mmap_request(struct file *file,
diff --git a/include/linux/io_uring.h b/include/linux/io_uring.h
index ed13304e764c..2b42f9c012fb 100644
--- a/include/linux/io_uring.h
+++ b/include/linux/io_uring.h
@@ -7,18 +7,18 @@
 
 #if defined(CONFIG_IO_URING)
 struct sock *io_uring_get_socket(struct file *file);
-void __io_uring_cancel(struct files_struct *files);
+void __io_uring_cancel(bool cancel_all);
 void __io_uring_free(struct task_struct *tsk);
 
-static inline void io_uring_files_cancel(struct files_struct *files)
+static inline void io_uring_files_cancel(void)
 {
 	if (current->io_uring)
-		__io_uring_cancel(files);
+		__io_uring_cancel(false);
 }
 static inline void io_uring_task_cancel(void)
 {
 	if (current->io_uring)
-		__io_uring_cancel(NULL);
+		__io_uring_cancel(true);
 }
 static inline void io_uring_free(struct task_struct *tsk)
 {
diff --git a/kernel/exit.c b/kernel/exit.c
index 9a89e7f36acb..91a43e57a32e 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -777,7 +777,7 @@ void __noreturn do_exit(long code)
 		schedule();
 	}
 
-	io_uring_files_cancel(tsk->files);
+	io_uring_files_cancel();
 	exit_signals(tsk);  /* sets PF_EXITING */
 
 	/* sync mm's RSS info before statistics gathering */
-- 
2.24.4

[PATCH 3/3] io_uring: code clean for completion_lock in io_arm_poll_handler()

[Hao Xu]

We can merge two spin_unlock() operations to one since we removed some
code not long ago.

Signed-off-by: Hao Xu <haoxu@linux.alibaba.com>
---
 fs/io_uring.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index b29774aa1f09..9cbc66b52643 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5231,13 +5231,10 @@ static int io_arm_poll_handler(struct io_kiocb *req)
 
 	ret = __io_arm_poll_handler(req, &apoll->poll, &ipt, mask,
 					io_async_wake);
-	if (ret || ipt.error) {
-		spin_unlock(&ctx->completion_lock);
-		if (ret)
-			return IO_APOLL_READY;
-		return IO_APOLL_ABORTED;
-	}
-	spin_unlock(&ctx->completion_lock);
+	spin_unlock_irq(&ctx->completion_lock);
+	if (ret || ipt.error)
+		return ret ? IO_APOLL_READY : IO_APOLL_ABORTED;
+
 	trace_io_uring_poll_arm(ctx, req, req->opcode, req->user_data,
 				mask, apoll->poll.events);
 	return IO_APOLL_OK;
-- 
2.24.4

Re: [PATCH 3/3] io_uring: code clean for completion_lock in io_arm_poll_handler()

[Joseph Qi]

On 8/12/21 12:14 PM, Hao Xu wrote:
> We can merge two spin_unlock() operations to one since we removed some
> code not long ago.
> 
> Signed-off-by: Hao Xu <haoxu@linux.alibaba.com>
> ---
>  fs/io_uring.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index b29774aa1f09..9cbc66b52643 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -5231,13 +5231,10 @@ static int io_arm_poll_handler(struct io_kiocb *req)
>  
>  	ret = __io_arm_poll_handler(req, &apoll->poll, &ipt, mask,
>  					io_async_wake);
> -	if (ret || ipt.error) {
> -		spin_unlock(&ctx->completion_lock);
> -		if (ret)
> -			return IO_APOLL_READY;
> -		return IO_APOLL_ABORTED;
> -	}
> -	spin_unlock(&ctx->completion_lock);
> +	spin_unlock_irq(&ctx->completion_lock);

This looks weird.
You replace spin_unlock() with spin_unlock_irq() without any spin_lock()
changes.

Thanks,
Joseph 

> +	if (ret || ipt.error)
> +		return ret ? IO_APOLL_READY : IO_APOLL_ABORTED;
> +
>  	trace_io_uring_poll_arm(ctx, req, req->opcode, req->user_data,
>  				mask, apoll->poll.events);
>  	return IO_APOLL_OK;
> 

[PATCH v2] io_uring: code clean for completion_lock in io_arm_poll_handler()

[Hao Xu]

We can merge two spin_unlock() operations to one since we removed some
code not long ago.

Signed-off-by: Hao Xu <haoxu@linux.alibaba.com>
---
 fs/io_uring.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index b29774aa1f09..7f56394e6c40 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5231,13 +5231,10 @@ static int io_arm_poll_handler(struct io_kiocb *req)
 
 	ret = __io_arm_poll_handler(req, &apoll->poll, &ipt, mask,
 					io_async_wake);
-	if (ret || ipt.error) {
-		spin_unlock(&ctx->completion_lock);
-		if (ret)
-			return IO_APOLL_READY;
-		return IO_APOLL_ABORTED;
-	}
 	spin_unlock(&ctx->completion_lock);
+	if (ret || ipt.error)
+		return ret ? IO_APOLL_READY : IO_APOLL_ABORTED;
+
 	trace_io_uring_poll_arm(ctx, req, req->opcode, req->user_data,
 				mask, apoll->poll.events);
 	return IO_APOLL_OK;
-- 
2.24.4

Re: [PATCH for-5.15 0/3] small code clean

[Jens Axboe]

On 8/11/21 10:14 PM, Hao Xu wrote:
> Some small code clean.
> 
> Hao Xu (3):
>   io_uring: extract io_uring_files_cancel() in io_uring_task_cancel()
>   io_uring: remove files pointer in cancellation functions
>   io_uring: code clean for completion_lock in io_arm_poll_handler()
> 
>  fs/io_uring.c            | 15 ++++++---------
>  include/linux/io_uring.h |  9 +++++----
>  kernel/exit.c            |  2 +-
>  3 files changed, 12 insertions(+), 14 deletions(-)

This looks good, except 3/3 which I think was rebased and then not 
re-tested after doing so... That's a black mark.

Anyway, v2 looks fine, applied for 5.15.

-- 
Jens Axboe

Re: [PATCH for-5.15 0/3] small code clean

[Hao Xu]

在 2021/8/13 上午1:28, Jens Axboe 写道:
> On 8/11/21 10:14 PM, Hao Xu wrote:
>> Some small code clean.
>>
>> Hao Xu (3):
>>    io_uring: extract io_uring_files_cancel() in io_uring_task_cancel()
>>    io_uring: remove files pointer in cancellation functions
>>    io_uring: code clean for completion_lock in io_arm_poll_handler()
>>
>>   fs/io_uring.c            | 15 ++++++---------
>>   include/linux/io_uring.h |  9 +++++----
>>   kernel/exit.c            |  2 +-
>>   3 files changed, 12 insertions(+), 14 deletions(-)
> 
> This looks good, except 3/3 which I think was rebased and then not
> re-tested after doing so... That's a black mark.
> 
Actually I re-tested it after rebasing code and addressing the conflict
But the liburing tests results seems all good. Anyway, I'll check the
code more carefully when resolving conflict.
> Anyway, v2 looks fine, applied for 5.15.
> 

Re: [PATCH for-5.15 0/3] small code clean

[Jens Axboe]

On 8/13/21 1:58 AM, Hao Xu wrote:
> 在 2021/8/13 上午1:28, Jens Axboe 写道:
>> On 8/11/21 10:14 PM, Hao Xu wrote:
>>> Some small code clean.
>>>
>>> Hao Xu (3):
>>>    io_uring: extract io_uring_files_cancel() in io_uring_task_cancel()
>>>    io_uring: remove files pointer in cancellation functions
>>>    io_uring: code clean for completion_lock in io_arm_poll_handler()
>>>
>>>   fs/io_uring.c            | 15 ++++++---------
>>>   include/linux/io_uring.h |  9 +++++----
>>>   kernel/exit.c            |  2 +-
>>>   3 files changed, 12 insertions(+), 14 deletions(-)
>>
>> This looks good, except 3/3 which I think was rebased and then not
>> re-tested after doing so... That's a black mark.
>>
> Actually I re-tested it after rebasing code and addressing the conflict
> But the liburing tests results seems all good. Anyway, I'll check the
> code more carefully when resolving conflict.

Probably would only complain if you had PROVE_LOCKING enabled in
your config.

The last patch broke !CONFIG_IO_URING too, I fixed that up.

-- 
Jens Axboe