From: Hao Xu <[email protected]>
Add a helper for poll clean, it will be used in the multishot accept in
the later patches.
Signed-off-by: Hao Xu <[email protected]>
---
fs/io_uring.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index d33777575faf..0a83ecc457d1 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5711,6 +5711,23 @@ static int io_accept_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
return 0;
}
+static inline void __io_poll_clean(struct io_kiocb *req)
+{
+ struct io_ring_ctx *ctx = req->ctx;
+
+ io_poll_remove_entries(req);
+ spin_lock(&ctx->completion_lock);
+ hash_del(&req->hash_node);
+ spin_unlock(&ctx->completion_lock);
+}
+
+#define REQ_F_APOLL_MULTI_POLLED (REQ_F_APOLL_MULTISHOT | REQ_F_POLLED)
+static inline void io_poll_clean(struct io_kiocb *req)
+{
+ if ((req->flags & REQ_F_APOLL_MULTI_POLLED) == REQ_F_APOLL_MULTI_POLLED)
+ __io_poll_clean(req);
+}
+
static int io_accept(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_accept *accept = &req->accept;
@@ -6041,17 +6058,13 @@ static void io_poll_task_func(struct io_kiocb *req, bool *locked)
static void io_apoll_task_func(struct io_kiocb *req, bool *locked)
{
- struct io_ring_ctx *ctx = req->ctx;
int ret;
ret = io_poll_check_events(req, locked);
if (ret > 0)
return;
- io_poll_remove_entries(req);
- spin_lock(&ctx->completion_lock);
- hash_del(&req->hash_node);
- spin_unlock(&ctx->completion_lock);
+ __io_poll_clean(req);
if (!ret)
io_req_task_submit(req, locked);
--
2.36.0
On 5/6/22 1:01 AM, Hao Xu wrote:
> From: Hao Xu <[email protected]>
>
> Add a helper for poll clean, it will be used in the multishot accept in
> the later patches.
Should this just go into io_clean_op()? Didn't look at it thoroughly,
but it'd remove some cases from the next patch if it could.
--
Jens Axboe
在 2022/5/6 下午10:36, Jens Axboe 写道:
> On 5/6/22 1:01 AM, Hao Xu wrote:
>> From: Hao Xu <[email protected]>
>>
>> Add a helper for poll clean, it will be used in the multishot accept in
>> the later patches.
>
> Should this just go into io_clean_op()? Didn't look at it thoroughly,
> but it'd remove some cases from the next patch if it could.
>
Actually this was my first version, here I put it in io_accept() to make
it happen as early as possible. I rethinked about this, seems it's ok to
put it into the io_clean_op().
Hi Hao,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on f2e030dd7aaea5a937a2547dc980fab418fbc5e7]
url: https://github.com/intel-lab-lkp/linux/commits/Hao-Xu/fast-poll-multishot-mode/20220506-150750
base: f2e030dd7aaea5a937a2547dc980fab418fbc5e7
config: mips-pic32mzda_defconfig (https://download.01.org/0day-ci/archive/20220506/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 5e004fb787698440a387750db7f8028e7cb14cfc)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install mips cross compiling tool for clang build
# apt-get install binutils-mips-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/acb232e81643bd097278ebdc17038e6f280e7212
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Hao-Xu/fast-poll-multishot-mode/20220506-150750
git checkout acb232e81643bd097278ebdc17038e6f280e7212
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> fs/io_uring.c:6067:2: error: call to undeclared function '__io_poll_clean'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
__io_poll_clean(req);
^
fs/io_uring.c:6067:2: note: did you mean '__io_fill_cqe'?
fs/io_uring.c:2141:20: note: '__io_fill_cqe' declared here
static inline bool __io_fill_cqe(struct io_ring_ctx *ctx, u64 user_data,
^
1 error generated.
vim +/__io_poll_clean +6067 fs/io_uring.c
6058
6059 static void io_apoll_task_func(struct io_kiocb *req, bool *locked)
6060 {
6061 int ret;
6062
6063 ret = io_poll_check_events(req, locked);
6064 if (ret > 0)
6065 return;
6066
> 6067 __io_poll_clean(req);
6068
6069 if (!ret)
6070 io_req_task_submit(req, locked);
6071 else
6072 io_req_complete_failed(req, ret);
6073 }
6074
--
0-DAY CI Kernel Test Service
https://01.org/lkp
On 5/7/22 07:43, Hao Xu wrote:
> 在 2022/5/7 上午12:22, Pavel Begunkov 写道:
>> On 5/6/22 08:01, Hao Xu wrote:
>>> From: Hao Xu <[email protected]>
>>>
>>> Add a helper for poll clean, it will be used in the multishot accept in
>>> the later patches.
>>>
>>> Signed-off-by: Hao Xu <[email protected]>
>>> ---
>>> fs/io_uring.c | 23 ++++++++++++++++++-----
>>> 1 file changed, 18 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>>> index d33777575faf..0a83ecc457d1 100644
>>> --- a/fs/io_uring.c
>>> +++ b/fs/io_uring.c
>>> @@ -5711,6 +5711,23 @@ static int io_accept_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>> return 0;
>>> }
>>> +static inline void __io_poll_clean(struct io_kiocb *req)
>>> +{
>>> + struct io_ring_ctx *ctx = req->ctx;
>>> +
>>> + io_poll_remove_entries(req);
>>> + spin_lock(&ctx->completion_lock);
>>> + hash_del(&req->hash_node);
>>> + spin_unlock(&ctx->completion_lock);
>>> +}
>>> +
>>> +#define REQ_F_APOLL_MULTI_POLLED (REQ_F_APOLL_MULTISHOT | REQ_F_POLLED)
>>> +static inline void io_poll_clean(struct io_kiocb *req)
>>> +{
>>> + if ((req->flags & REQ_F_APOLL_MULTI_POLLED) == REQ_F_APOLL_MULTI_POLLED)
>>
>> So it triggers for apoll multishot only when REQ_F_POLLED is _not_ set,
>> but if it's not set it did never go through arm_poll / etc. and there is
>> nothing to clean up. What is the catch?
>
> No, it is triggered for apoll multishot only when REQ_F_POLLED is set..
Ok, see it now, probably confused REQ_F_APOLL_MULTI_POLLED on the right
hand side with something else
>> btw, don't see the function used in this patch, better to add it later
>> or at least mark with attribute unused, or some may get build failures.
> Gotcha.
>>
>>
>>> + __io_poll_clean(req);
>>> +}
>>> +
>>> static int io_accept(struct io_kiocb *req, unsigned int issue_flags)
>>> {
>>> struct io_accept *accept = &req->accept;
>>> @@ -6041,17 +6058,13 @@ static void io_poll_task_func(struct io_kiocb *req, bool *locked)
>>> static void io_apoll_task_func(struct io_kiocb *req, bool *locked)
>>> {
>>> - struct io_ring_ctx *ctx = req->ctx;
>>> int ret;
>>> ret = io_poll_check_events(req, locked);
>>> if (ret > 0)
>>> return;
>>> - io_poll_remove_entries(req);
>>> - spin_lock(&ctx->completion_lock);
>>> - hash_del(&req->hash_node);
>>> - spin_unlock(&ctx->completion_lock);
>>> + __io_poll_clean(req);
>>> if (!ret)
>>> io_req_task_submit(req, locked);
>>
>
--
Pavel Begunkov
On 5/6/22 08:01, Hao Xu wrote:
> From: Hao Xu <[email protected]>
>
> Add a helper for poll clean, it will be used in the multishot accept in
> the later patches.
>
> Signed-off-by: Hao Xu <[email protected]>
> ---
> fs/io_uring.c | 23 ++++++++++++++++++-----
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index d33777575faf..0a83ecc457d1 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -5711,6 +5711,23 @@ static int io_accept_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> return 0;
> }
>
> +static inline void __io_poll_clean(struct io_kiocb *req)
> +{
> + struct io_ring_ctx *ctx = req->ctx;
> +
> + io_poll_remove_entries(req);
> + spin_lock(&ctx->completion_lock);
> + hash_del(&req->hash_node);
> + spin_unlock(&ctx->completion_lock);
> +}
> +
> +#define REQ_F_APOLL_MULTI_POLLED (REQ_F_APOLL_MULTISHOT | REQ_F_POLLED)
> +static inline void io_poll_clean(struct io_kiocb *req)
> +{
> + if ((req->flags & REQ_F_APOLL_MULTI_POLLED) == REQ_F_APOLL_MULTI_POLLED)
So it triggers for apoll multishot only when REQ_F_POLLED is _not_ set,
but if it's not set it did never go through arm_poll / etc. and there is
nothing to clean up. What is the catch?
btw, don't see the function used in this patch, better to add it later
or at least mark with attribute unused, or some may get build failures.
> + __io_poll_clean(req);
> +}
> +
> static int io_accept(struct io_kiocb *req, unsigned int issue_flags)
> {
> struct io_accept *accept = &req->accept;
> @@ -6041,17 +6058,13 @@ static void io_poll_task_func(struct io_kiocb *req, bool *locked)
>
> static void io_apoll_task_func(struct io_kiocb *req, bool *locked)
> {
> - struct io_ring_ctx *ctx = req->ctx;
> int ret;
>
> ret = io_poll_check_events(req, locked);
> if (ret > 0)
> return;
>
> - io_poll_remove_entries(req);
> - spin_lock(&ctx->completion_lock);
> - hash_del(&req->hash_node);
> - spin_unlock(&ctx->completion_lock);
> + __io_poll_clean(req);
>
> if (!ret)
> io_req_task_submit(req, locked);
--
Pavel Begunkov
在 2022/5/7 上午12:22, Pavel Begunkov 写道:
> On 5/6/22 08:01, Hao Xu wrote:
>> From: Hao Xu <[email protected]>
>>
>> Add a helper for poll clean, it will be used in the multishot accept in
>> the later patches.
>>
>> Signed-off-by: Hao Xu <[email protected]>
>> ---
>> fs/io_uring.c | 23 ++++++++++++++++++-----
>> 1 file changed, 18 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index d33777575faf..0a83ecc457d1 100644
>> --- a/fs/io_uring.c
>> +++ b/fs/io_uring.c
>> @@ -5711,6 +5711,23 @@ static int io_accept_prep(struct io_kiocb *req,
>> const struct io_uring_sqe *sqe)
>> return 0;
>> }
>> +static inline void __io_poll_clean(struct io_kiocb *req)
>> +{
>> + struct io_ring_ctx *ctx = req->ctx;
>> +
>> + io_poll_remove_entries(req);
>> + spin_lock(&ctx->completion_lock);
>> + hash_del(&req->hash_node);
>> + spin_unlock(&ctx->completion_lock);
>> +}
>> +
>> +#define REQ_F_APOLL_MULTI_POLLED (REQ_F_APOLL_MULTISHOT | REQ_F_POLLED)
>> +static inline void io_poll_clean(struct io_kiocb *req)
>> +{
>> + if ((req->flags & REQ_F_APOLL_MULTI_POLLED) ==
>> REQ_F_APOLL_MULTI_POLLED)
>
> So it triggers for apoll multishot only when REQ_F_POLLED is _not_ set,
> but if it's not set it did never go through arm_poll / etc. and there is
> nothing to clean up. What is the catch?
No, it is triggered for apoll multishot only when REQ_F_POLLED is set..
>
> btw, don't see the function used in this patch, better to add it later
> or at least mark with attribute unused, or some may get build failures.
Gotcha.
>
>
>> + __io_poll_clean(req);
>> +}
>> +
>> static int io_accept(struct io_kiocb *req, unsigned int issue_flags)
>> {
>> struct io_accept *accept = &req->accept;
>> @@ -6041,17 +6058,13 @@ static void io_poll_task_func(struct io_kiocb
>> *req, bool *locked)
>> static void io_apoll_task_func(struct io_kiocb *req, bool *locked)
>> {
>> - struct io_ring_ctx *ctx = req->ctx;
>> int ret;
>> ret = io_poll_check_events(req, locked);
>> if (ret > 0)
>> return;
>> - io_poll_remove_entries(req);
>> - spin_lock(&ctx->completion_lock);
>> - hash_del(&req->hash_node);
>> - spin_unlock(&ctx->completion_lock);
>> + __io_poll_clean(req);
>> if (!ret)
>> io_req_task_submit(req, locked);
>
Hi Hao,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on f2e030dd7aaea5a937a2547dc980fab418fbc5e7]
url: https://github.com/intel-lab-lkp/linux/commits/Hao-Xu/fast-poll-multishot-mode/20220506-150750
base: f2e030dd7aaea5a937a2547dc980fab418fbc5e7
config: m68k-randconfig-r025-20220506 (https://download.01.org/0day-ci/archive/20220506/[email protected]/config)
compiler: m68k-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/acb232e81643bd097278ebdc17038e6f280e7212
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Hao-Xu/fast-poll-multishot-mode/20220506-150750
git checkout acb232e81643bd097278ebdc17038e6f280e7212
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
fs/io_uring.c: In function '__io_submit_flush_completions':
fs/io_uring.c:2785:40: warning: variable 'prev' set but not used [-Wunused-but-set-variable]
2785 | struct io_wq_work_node *node, *prev;
| ^~~~
fs/io_uring.c: In function 'io_apoll_task_func':
>> fs/io_uring.c:6067:9: error: implicit declaration of function '__io_poll_clean'; did you mean '__io_fill_cqe'? [-Werror=implicit-function-declaration]
6067 | __io_poll_clean(req);
| ^~~~~~~~~~~~~~~
| __io_fill_cqe
cc1: some warnings being treated as errors
vim +6067 fs/io_uring.c
6058
6059 static void io_apoll_task_func(struct io_kiocb *req, bool *locked)
6060 {
6061 int ret;
6062
6063 ret = io_poll_check_events(req, locked);
6064 if (ret > 0)
6065 return;
6066
> 6067 __io_poll_clean(req);
6068
6069 if (!ret)
6070 io_req_task_submit(req, locked);
6071 else
6072 io_req_complete_failed(req, ret);
6073 }
6074
--
0-DAY CI Kernel Test Service
https://01.org/lkp