* [PATCH 1/3] io_uring: extract io_uring_files_cancel() in io_uring_task_cancel()
2021-08-12 4:14 [PATCH for-5.15 0/3] small code clean Hao Xu
@ 2021-08-12 4:14 ` Hao Xu
2021-08-12 4:14 ` [PATCH 2/3] io_uring: remove files pointer in cancellation functions Hao Xu
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Hao Xu @ 2021-08-12 4:14 UTC (permalink / raw)
To: Jens Axboe; +Cc: io-uring, Pavel Begunkov, Joseph Qi
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 <[email protected]>
---
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/3] io_uring: remove files pointer in cancellation functions
2021-08-12 4:14 [PATCH for-5.15 0/3] small code clean Hao Xu
2021-08-12 4:14 ` [PATCH 1/3] io_uring: extract io_uring_files_cancel() in io_uring_task_cancel() Hao Xu
@ 2021-08-12 4:14 ` Hao Xu
2021-08-12 4:14 ` [PATCH 3/3] io_uring: code clean for completion_lock in io_arm_poll_handler() Hao Xu
2021-08-12 17:28 ` [PATCH for-5.15 0/3] small code clean Jens Axboe
3 siblings, 0 replies; 9+ messages in thread
From: Hao Xu @ 2021-08-12 4:14 UTC (permalink / raw)
To: Jens Axboe; +Cc: io-uring, Pavel Begunkov, Joseph Qi
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 <[email protected]>
---
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/3] io_uring: code clean for completion_lock in io_arm_poll_handler()
2021-08-12 4:14 [PATCH for-5.15 0/3] small code clean Hao Xu
2021-08-12 4:14 ` [PATCH 1/3] io_uring: extract io_uring_files_cancel() in io_uring_task_cancel() Hao Xu
2021-08-12 4:14 ` [PATCH 2/3] io_uring: remove files pointer in cancellation functions Hao Xu
@ 2021-08-12 4:14 ` Hao Xu
2021-08-12 7:34 ` Joseph Qi
2021-08-12 7:47 ` [PATCH v2] " Hao Xu
2021-08-12 17:28 ` [PATCH for-5.15 0/3] small code clean Jens Axboe
3 siblings, 2 replies; 9+ messages in thread
From: Hao Xu @ 2021-08-12 4:14 UTC (permalink / raw)
To: Jens Axboe; +Cc: io-uring, Pavel Begunkov, Joseph Qi
We can merge two spin_unlock() operations to one since we removed some
code not long ago.
Signed-off-by: Hao Xu <[email protected]>
---
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] io_uring: code clean for completion_lock in io_arm_poll_handler()
2021-08-12 4:14 ` [PATCH 3/3] io_uring: code clean for completion_lock in io_arm_poll_handler() Hao Xu
@ 2021-08-12 7:34 ` Joseph Qi
2021-08-12 7:47 ` [PATCH v2] " Hao Xu
1 sibling, 0 replies; 9+ messages in thread
From: Joseph Qi @ 2021-08-12 7:34 UTC (permalink / raw)
To: Hao Xu, Jens Axboe; +Cc: io-uring, Pavel Begunkov
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 <[email protected]>
> ---
> 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;
>
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v2] io_uring: code clean for completion_lock in io_arm_poll_handler()
2021-08-12 4:14 ` [PATCH 3/3] io_uring: code clean for completion_lock in io_arm_poll_handler() Hao Xu
2021-08-12 7:34 ` Joseph Qi
@ 2021-08-12 7:47 ` Hao Xu
1 sibling, 0 replies; 9+ messages in thread
From: Hao Xu @ 2021-08-12 7:47 UTC (permalink / raw)
To: Jens Axboe; +Cc: io-uring, Pavel Begunkov, Joseph Qi
We can merge two spin_unlock() operations to one since we removed some
code not long ago.
Signed-off-by: Hao Xu <[email protected]>
---
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH for-5.15 0/3] small code clean
2021-08-12 4:14 [PATCH for-5.15 0/3] small code clean Hao Xu
` (2 preceding siblings ...)
2021-08-12 4:14 ` [PATCH 3/3] io_uring: code clean for completion_lock in io_arm_poll_handler() Hao Xu
@ 2021-08-12 17:28 ` Jens Axboe
2021-08-13 7:58 ` Hao Xu
3 siblings, 1 reply; 9+ messages in thread
From: Jens Axboe @ 2021-08-12 17:28 UTC (permalink / raw)
To: Hao Xu; +Cc: io-uring, Pavel Begunkov, Joseph Qi
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH for-5.15 0/3] small code clean
2021-08-12 17:28 ` [PATCH for-5.15 0/3] small code clean Jens Axboe
@ 2021-08-13 7:58 ` Hao Xu
2021-08-13 13:56 ` Jens Axboe
0 siblings, 1 reply; 9+ messages in thread
From: Hao Xu @ 2021-08-13 7:58 UTC (permalink / raw)
To: Jens Axboe; +Cc: io-uring, Pavel Begunkov, Joseph Qi
在 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.
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH for-5.15 0/3] small code clean
2021-08-13 7:58 ` Hao Xu
@ 2021-08-13 13:56 ` Jens Axboe
0 siblings, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2021-08-13 13:56 UTC (permalink / raw)
To: Hao Xu; +Cc: io-uring, Pavel Begunkov, Joseph Qi
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
^ permalink raw reply [flat|nested] 9+ messages in thread