[PATCH v3 0/4] fast poll multishot mode

[PATCH v3 0/4] fast poll multishot mode

[Hao Xu]

Let multishot support multishot mode, currently only add accept as its
first comsumer.
theoretical analysis:
  1) when connections come in fast
    - singleshot:
              add accept sqe(userpsace) --> accept inline
                              ^                 |
                              |-----------------|
    - multishot:
             add accept sqe(userspace) --> accept inline
                                              ^     |
                                              |--*--|

    we do accept repeatedly in * place until get EAGAIN

  2) when connections come in at a low pressure
    similar thing like 1), we reduce a lot of userspace-kernel context
    switch and useless vfs_poll()


tests:
Did some tests, which goes in this way:

  server    client(multiple)
  accept    connect
  read      write
  write     read
  close     close

Basically, raise up a number of clients(on same machine with server) to
connect to the server, and then write some data to it, the server will
write those data back to the client after it receives them, and then
close the connection after write return. Then the client will read the
data and then close the connection. Here I test 10000 clients connect
one server, data size 128 bytes. And each client has a go routine for
it, so they come to the server in short time.
test 20 times before/after this patchset, time spent:(unit cycle, which
is the return value of clock())
before:
  1930136+1940725+1907981+1947601+1923812+1928226+1911087+1905897+1941075
  +1934374+1906614+1912504+1949110+1908790+1909951+1941672+1969525+1934984
  +1934226+1914385)/20.0 = 1927633.75
after:
  1858905+1917104+1895455+1963963+1892706+1889208+1874175+1904753+1874112
  +1874985+1882706+1884642+1864694+1906508+1916150+1924250+1869060+1889506
  +1871324+1940803)/20.0 = 1894750.45

(1927633.75 - 1894750.45) / 1927633.75 = 1.65%


A liburing test is here:
https://github.com/HowHsu/liburing/blob/multishot_accept/test/accept.c

v1->v2:
 - re-implement it against the reworked poll code

v2->v3:
 - fold in code tweak and clean from Jens
 - use io_issue_sqe rather than io_queue_sqe, since the former one
   return the internal error back which makes more sense
 - remove io_poll_clean() and its friends since they are not needed


Hao Xu (4):
  io_uring: add IORING_ACCEPT_MULTISHOT for accept
  io_uring: add REQ_F_APOLL_MULTISHOT for requests
  io_uring: let fast poll support multishot
  io_uring: implement multishot mode for accept

 fs/io_uring.c                 | 94 +++++++++++++++++++++++++++--------
 include/uapi/linux/io_uring.h |  5 ++
 2 files changed, 79 insertions(+), 20 deletions(-)


base-commit: 0a194603ba7ee67b4e39ec0ee5cda70a356ea618
-- 
2.36.0

Re: [PATCH v3 0/4] fast poll multishot mode

[Hao Xu]

Something wrong with my git sendemail, trying to fix it. Please ignore 
this...

在 2022/5/7 下午8:38, Hao Xu 写道:
> Let multishot support multishot mode, currently only add accept as its
> first comsumer.
> theoretical analysis:
>    1) when connections come in fast
>      - singleshot:
>                add accept sqe(userpsace) --> accept inline
>                                ^                 |
>                                |-----------------|
>      - multishot:
>               add accept sqe(userspace) --> accept inline
>                                                ^     |
>                                                |--*--|
> 
>      we do accept repeatedly in * place until get EAGAIN
> 
>    2) when connections come in at a low pressure
>      similar thing like 1), we reduce a lot of userspace-kernel context
>      switch and useless vfs_poll()
> 
> 
> tests:
> Did some tests, which goes in this way:
> 
>    server    client(multiple)
>    accept    connect
>    read      write
>    write     read
>    close     close
> 
> Basically, raise up a number of clients(on same machine with server) to
> connect to the server, and then write some data to it, the server will
> write those data back to the client after it receives them, and then
> close the connection after write return. Then the client will read the
> data and then close the connection. Here I test 10000 clients connect
> one server, data size 128 bytes. And each client has a go routine for
> it, so they come to the server in short time.
> test 20 times before/after this patchset, time spent:(unit cycle, which
> is the return value of clock())
> before:
>    1930136+1940725+1907981+1947601+1923812+1928226+1911087+1905897+1941075
>    +1934374+1906614+1912504+1949110+1908790+1909951+1941672+1969525+1934984
>    +1934226+1914385)/20.0 = 1927633.75
> after:
>    1858905+1917104+1895455+1963963+1892706+1889208+1874175+1904753+1874112
>    +1874985+1882706+1884642+1864694+1906508+1916150+1924250+1869060+1889506
>    +1871324+1940803)/20.0 = 1894750.45
> 
> (1927633.75 - 1894750.45) / 1927633.75 = 1.65%
> 
> 
> A liburing test is here:
> https://github.com/HowHsu/liburing/blob/multishot_accept/test/accept.c
> 
> v1->v2:
>   - re-implement it against the reworked poll code
> 
> v2->v3:
>   - fold in code tweak and clean from Jens
>   - use io_issue_sqe rather than io_queue_sqe, since the former one
>     return the internal error back which makes more sense
>   - remove io_poll_clean() and its friends since they are not needed
> 
> 
> Hao Xu (4):
>    io_uring: add IORING_ACCEPT_MULTISHOT for accept
>    io_uring: add REQ_F_APOLL_MULTISHOT for requests
>    io_uring: let fast poll support multishot
>    io_uring: implement multishot mode for accept
> 
>   fs/io_uring.c                 | 94 +++++++++++++++++++++++++++--------
>   include/uapi/linux/io_uring.h |  5 ++
>   2 files changed, 79 insertions(+), 20 deletions(-)
> 
> 
> base-commit: 0a194603ba7ee67b4e39ec0ee5cda70a356ea618

[PATCH v3 0/4] fast poll multishot mode

[Hao Xu]

Let multishot support multishot mode, currently only add accept as its
first comsumer.
theoretical analysis:
  1) when connections come in fast
    - singleshot:
              add accept sqe(userpsace) --> accept inline
                              ^                 |
                              |-----------------|
    - multishot:
             add accept sqe(userspace) --> accept inline
                                              ^     |
                                              |--*--|

    we do accept repeatedly in * place until get EAGAIN

  2) when connections come in at a low pressure
    similar thing like 1), we reduce a lot of userspace-kernel context
    switch and useless vfs_poll()


tests:
Did some tests, which goes in this way:

  server    client(multiple)
  accept    connect
  read      write
  write     read
  close     close

Basically, raise up a number of clients(on same machine with server) to
connect to the server, and then write some data to it, the server will
write those data back to the client after it receives them, and then
close the connection after write return. Then the client will read the
data and then close the connection. Here I test 10000 clients connect
one server, data size 128 bytes. And each client has a go routine for
it, so they come to the server in short time.
test 20 times before/after this patchset, time spent:(unit cycle, which
is the return value of clock())
before:
  1930136+1940725+1907981+1947601+1923812+1928226+1911087+1905897+1941075
  +1934374+1906614+1912504+1949110+1908790+1909951+1941672+1969525+1934984
  +1934226+1914385)/20.0 = 1927633.75
after:
  1858905+1917104+1895455+1963963+1892706+1889208+1874175+1904753+1874112
  +1874985+1882706+1884642+1864694+1906508+1916150+1924250+1869060+1889506
  +1871324+1940803)/20.0 = 1894750.45

(1927633.75 - 1894750.45) / 1927633.75 = 1.65%


A liburing test is here:
https://github.com/HowHsu/liburing/blob/multishot_accept/test/accept.c

v1->v2:
 - re-implement it against the reworked poll code

v2->v3:
 - fold in code tweak and clean from Jens
 - use io_issue_sqe rather than io_queue_sqe, since the former one
   return the internal error back which makes more sense
 - remove io_poll_clean() and its friends since they are not needed


Hao Xu (4):
  io_uring: add IORING_ACCEPT_MULTISHOT for accept
  io_uring: add REQ_F_APOLL_MULTISHOT for requests
  io_uring: let fast poll support multishot
  io_uring: implement multishot mode for accept

 fs/io_uring.c                 | 94 +++++++++++++++++++++++++++--------
 include/uapi/linux/io_uring.h |  5 ++
 2 files changed, 79 insertions(+), 20 deletions(-)


base-commit: 0a194603ba7ee67b4e39ec0ee5cda70a356ea618
-- 
2.36.0

[PATCH 1/4] io_uring: add IORING_ACCEPT_MULTISHOT for accept

[Hao Xu]

From: Hao Xu <howeyxu@tencent.com>

add an accept_flag IORING_ACCEPT_MULTISHOT for accept, which is to
support multishot.

Signed-off-by: Hao Xu <howeyxu@tencent.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 include/uapi/linux/io_uring.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 06621a278cb6..f4d9ca62a5a6 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -223,6 +223,11 @@ enum {
  */
 #define IORING_RECVSEND_POLL_FIRST	(1U << 0)
 
+/*
+ * accept flags stored in accept_flags
+ */
+#define IORING_ACCEPT_MULTISHOT	(1U << 15)
+
 /*
  * IO completion data structure (Completion Queue Entry)
  */
-- 
2.36.0

[PATCH 2/4] io_uring: add REQ_F_APOLL_MULTISHOT for requests

[Hao Xu]

From: Hao Xu <howeyxu@tencent.com>

Add a flag to indicate multishot mode for fast poll. currently only
accept use it, but there may be more operations leveraging it in the
future. Also add a mask IO_APOLL_MULTI_POLLED which stands for
REQ_F_APOLL_MULTI | REQ_F_POLLED, to make the code short and cleaner.

Signed-off-by: Hao Xu <howeyxu@tencent.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io_uring.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index b6d491c9a25f..c2ee184ac693 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -116,6 +116,8 @@
 #define IO_REQ_CLEAN_SLOW_FLAGS (REQ_F_REFCOUNT | REQ_F_LINK | REQ_F_HARDLINK |\
 				 IO_REQ_CLEAN_FLAGS)
 
+#define IO_APOLL_MULTI_POLLED (REQ_F_APOLL_MULTISHOT | REQ_F_POLLED)
+
 #define IO_TCTX_REFS_CACHE_NR	(1U << 10)
 
 struct io_uring {
@@ -810,6 +812,7 @@ enum {
 	REQ_F_SINGLE_POLL_BIT,
 	REQ_F_DOUBLE_POLL_BIT,
 	REQ_F_PARTIAL_IO_BIT,
+	REQ_F_APOLL_MULTISHOT_BIT,
 	/* keep async read/write and isreg together and in order */
 	REQ_F_SUPPORT_NOWAIT_BIT,
 	REQ_F_ISREG_BIT,
@@ -874,6 +877,8 @@ enum {
 	REQ_F_DOUBLE_POLL	= BIT(REQ_F_DOUBLE_POLL_BIT),
 	/* request has already done partial IO */
 	REQ_F_PARTIAL_IO	= BIT(REQ_F_PARTIAL_IO_BIT),
+	/* fast poll multishot mode */
+	REQ_F_APOLL_MULTISHOT	= BIT(REQ_F_APOLL_MULTISHOT_BIT),
 };
 
 struct async_poll {
-- 
2.36.0

[PATCH 3/4] io_uring: let fast poll support multishot

[Hao Xu]

From: Hao Xu <howeyxu@tencent.com>

For operations like accept, multishot is a useful feature, since we can
reduce a number of accept sqe. Let's integrate it to fast poll, it may
be good for other operations in the future.

Signed-off-by: Hao Xu <howeyxu@tencent.com>
[fold in cleaning up]
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io_uring.c | 47 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 15 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index c2ee184ac693..e0d12af04cd1 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5955,6 +5955,7 @@ static void io_poll_remove_entries(struct io_kiocb *req)
 	rcu_read_unlock();
 }
 
+static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags);
 /*
  * All poll tw should go through this. Checks for poll events, manages
  * references, does rewait, etc.
@@ -5963,10 +5964,10 @@ static void io_poll_remove_entries(struct io_kiocb *req)
  * either spurious wakeup or multishot CQE is served. 0 when it's done with
  * the request, then the mask is stored in req->cqe.res.
  */
-static int io_poll_check_events(struct io_kiocb *req, bool locked)
+static int io_poll_check_events(struct io_kiocb *req, bool *locked)
 {
 	struct io_ring_ctx *ctx = req->ctx;
-	int v;
+	int v, ret;
 
 	/* req->task == current here, checking PF_EXITING is safe */
 	if (unlikely(req->task->flags & PF_EXITING))
@@ -5990,23 +5991,37 @@ static int io_poll_check_events(struct io_kiocb *req, bool locked)
 			req->cqe.res = vfs_poll(req->file, &pt) & req->apoll_events;
 		}
 
-		/* multishot, just fill an CQE and proceed */
-		if (req->cqe.res && !(req->apoll_events & EPOLLONESHOT)) {
-			__poll_t mask = mangle_poll(req->cqe.res & req->apoll_events);
+		if ((unlikely(!req->cqe.res)))
+			continue;
+		if (req->apoll_events & EPOLLONESHOT)
+			return 0;
+
+		/* multishot, just fill a CQE and proceed */
+		if (!(req->flags & REQ_F_APOLL_MULTISHOT)) {
+			__poll_t mask = mangle_poll(req->cqe.res &
+						    req->apoll_events);
 			bool filled;
 
 			spin_lock(&ctx->completion_lock);
-			filled = io_fill_cqe_aux(ctx, req->cqe.user_data, mask,
-						 IORING_CQE_F_MORE);
+			filled = io_fill_cqe_aux(ctx, req->cqe.user_data,
+						 mask, IORING_CQE_F_MORE);
 			io_commit_cqring(ctx);
 			spin_unlock(&ctx->completion_lock);
-			if (unlikely(!filled))
-				return -ECANCELED;
-			io_cqring_ev_posted(ctx);
-		} else if (req->cqe.res) {
-			return 0;
+			if (filled) {
+				io_cqring_ev_posted(ctx);
+				continue;
+			}
+			return -ECANCELED;
 		}
 
+		io_tw_lock(req->ctx, locked);
+		if (unlikely(req->task->flags & PF_EXITING))
+			return -EFAULT;
+		ret = io_issue_sqe(req,
+				   IO_URING_F_NONBLOCK|IO_URING_F_COMPLETE_DEFER);
+		if (ret)
+			return ret;
+
 		/*
 		 * Release all references, retry if someone tried to restart
 		 * task_work while we were executing it.
@@ -6021,7 +6036,7 @@ static void io_poll_task_func(struct io_kiocb *req, bool *locked)
 	struct io_ring_ctx *ctx = req->ctx;
 	int ret;
 
-	ret = io_poll_check_events(req, *locked);
+	ret = io_poll_check_events(req, locked);
 	if (ret > 0)
 		return;
 
@@ -6046,7 +6061,7 @@ 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);
+	ret = io_poll_check_events(req, locked);
 	if (ret > 0)
 		return;
 
@@ -6286,7 +6301,7 @@ static int io_arm_poll_handler(struct io_kiocb *req, unsigned issue_flags)
 	struct io_ring_ctx *ctx = req->ctx;
 	struct async_poll *apoll;
 	struct io_poll_table ipt;
-	__poll_t mask = EPOLLONESHOT | POLLERR | POLLPRI;
+	__poll_t mask = POLLERR | POLLPRI;
 	int ret;
 
 	if (!def->pollin && !def->pollout)
@@ -6295,6 +6310,8 @@ static int io_arm_poll_handler(struct io_kiocb *req, unsigned issue_flags)
 		return IO_APOLL_ABORTED;
 	if ((req->flags & (REQ_F_POLLED|REQ_F_PARTIAL_IO)) == REQ_F_POLLED)
 		return IO_APOLL_ABORTED;
+	if (!(req->flags & REQ_F_APOLL_MULTISHOT))
+		mask |= EPOLLONESHOT;
 
 	if (def->pollin) {
 		mask |= POLLIN | POLLRDNORM;
-- 
2.36.0

Re: [PATCH 1/4] io_uring: add IORING_ACCEPT_MULTISHOT for accept

[Jens Axboe]

On 5/7/22 8:06 AM, Hao Xu wrote:
> From: Hao Xu <howeyxu@tencent.com>
> 
> add an accept_flag IORING_ACCEPT_MULTISHOT for accept, which is to
> support multishot.
> 
> Signed-off-by: Hao Xu <howeyxu@tencent.com>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>

Heh, don't add my SOB. Guessing this came from the folding in?

> ---
>  include/uapi/linux/io_uring.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
> index 06621a278cb6..f4d9ca62a5a6 100644
> --- a/include/uapi/linux/io_uring.h
> +++ b/include/uapi/linux/io_uring.h
> @@ -223,6 +223,11 @@ enum {
>   */
>  #define IORING_RECVSEND_POLL_FIRST	(1U << 0)
>  
> +/*
> + * accept flags stored in accept_flags
> + */
> +#define IORING_ACCEPT_MULTISHOT	(1U << 15)

Looks like the git send-email is still acting up, this looks like
v2?

-- 
Jens Axboe

Re: [PATCH v3 0/4] fast poll multishot mode

[Jens Axboe]

On 5/7/22 8:06 AM, Hao Xu wrote:
> Let multishot support multishot mode, currently only add accept as its
> first comsumer.

consumer

> theoretical analysis:
>   1) when connections come in fast
>     - singleshot:
>               add accept sqe(userpsace) --> accept inline

userspace

>                               ^                 |
>                               |-----------------|
>     - multishot:
>              add accept sqe(userspace) --> accept inline
>                                               ^     |
>                                               |--*--|
> 
>     we do accept repeatedly in * place until get EAGAIN
> 
>   2) when connections come in at a low pressure
>     similar thing like 1), we reduce a lot of userspace-kernel context
>     switch and useless vfs_poll()

Overall this looks better than v2 for sure, just some minor tweaks
needed I believe.

But we still need to consider direct accept with multishot... Should
probably be an add-on patch as I think it'd get a bit more complicated
if we need to be able to cheaply find an available free fixed fd slot.
I'll try and play with that.

-- 
Jens Axboe

Re: [PATCH 1/4] io_uring: add IORING_ACCEPT_MULTISHOT for accept

[Hao Xu]

在 2022/5/7 下午10:16, Jens Axboe 写道:
> On 5/7/22 8:06 AM, Hao Xu wrote:
>> From: Hao Xu <howeyxu@tencent.com>
>>
>> add an accept_flag IORING_ACCEPT_MULTISHOT for accept, which is to
>> support multishot.
>>
>> Signed-off-by: Hao Xu <howeyxu@tencent.com>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> 
> Heh, don't add my SOB. Guessing this came from the folding in?Nop, It is in your fastpoll-mshot branch
https://git.kernel.dk/cgit/linux-block/commit/?h=fastpoll-mshot&id=e37527e6b4ac60e1effdc8aaa1058e931930af01
> 
>> ---
>>   include/uapi/linux/io_uring.h | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
>> index 06621a278cb6..f4d9ca62a5a6 100644
>> --- a/include/uapi/linux/io_uring.h
>> +++ b/include/uapi/linux/io_uring.h
>> @@ -223,6 +223,11 @@ enum {
>>    */
>>   #define IORING_RECVSEND_POLL_FIRST	(1U << 0)
>>   
>> +/*
>> + * accept flags stored in accept_flags
>> + */
>> +#define IORING_ACCEPT_MULTISHOT	(1U << 15)
> 
> Looks like the git send-email is still acting up, this looks like
> v2?
> 

Re: [PATCH 1/4] io_uring: add IORING_ACCEPT_MULTISHOT for accept

[Jens Axboe]

On 5/7/22 9:31 AM, Hao Xu wrote:
> ? 2022/5/7 ??10:16, Jens Axboe ??:
>> On 5/7/22 8:06 AM, Hao Xu wrote:
>>> From: Hao Xu <howeyxu@tencent.com>
>>>
>>> add an accept_flag IORING_ACCEPT_MULTISHOT for accept, which is to
>>> support multishot.
>>>
>>> Signed-off-by: Hao Xu <howeyxu@tencent.com>
>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>
>> Heh, don't add my SOB. Guessing this came from the folding in?Nop, It is in your fastpoll-mshot branch
> https://git.kernel.dk/cgit/linux-block/commit/?h=fastpoll-mshot&id=e37527e6b4ac60e1effdc8aaa1058e931930af01

But that's just a stand-alone fixup patch to be folded in, the SOB
doesn't carry to other patches. So for all of them, just strip that for
v4. If/when it gets applied, my SOB will get attached at that point.

-- 
Jens Axboe

Re: [PATCH 1/4] io_uring: add IORING_ACCEPT_MULTISHOT for accept

[Hao Xu]

在 2022/5/7 下午11:38, Jens Axboe 写道:
> On 5/7/22 9:31 AM, Hao Xu wrote:
>> ? 2022/5/7 ??10:16, Jens Axboe ??:
>>> On 5/7/22 8:06 AM, Hao Xu wrote:
>>>> From: Hao Xu <howeyxu@tencent.com>
>>>>
>>>> add an accept_flag IORING_ACCEPT_MULTISHOT for accept, which is to
>>>> support multishot.
>>>>
>>>> Signed-off-by: Hao Xu <howeyxu@tencent.com>
>>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>>
>>> Heh, don't add my SOB. Guessing this came from the folding in?Nop, It is in your fastpoll-mshot branch
>> https://git.kernel.dk/cgit/linux-block/commit/?h=fastpoll-mshot&id=e37527e6b4ac60e1effdc8aaa1058e931930af01
> 
> But that's just a stand-alone fixup patch to be folded in, the SOB
> doesn't carry to other patches. So for all of them, just strip that for
> v4. If/when it gets applied, my SOB will get attached at that point.
> 
Sorry, paste a wrong link, should be this:
https://git.kernel.dk/cgit/linux-block/commit/?h=fastpoll-mshot&id=289555f559f252fbfca6bdd0886316a8b17693e2

Re: [PATCH 1/4] io_uring: add IORING_ACCEPT_MULTISHOT for accept

[Jens Axboe]

On 5/7/22 9:52 AM, Hao Xu wrote:
> ? 2022/5/7 ??11:38, Jens Axboe ??:
>> On 5/7/22 9:31 AM, Hao Xu wrote:
>>> ? 2022/5/7 ??10:16, Jens Axboe ??:
>>>> On 5/7/22 8:06 AM, Hao Xu wrote:
>>>>> From: Hao Xu <howeyxu@tencent.com>
>>>>>
>>>>> add an accept_flag IORING_ACCEPT_MULTISHOT for accept, which is to
>>>>> support multishot.
>>>>>
>>>>> Signed-off-by: Hao Xu <howeyxu@tencent.com>
>>>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>>>
>>>> Heh, don't add my SOB. Guessing this came from the folding in?Nop, It is in your fastpoll-mshot branch
>>> https://git.kernel.dk/cgit/linux-block/commit/?h=fastpoll-mshot&id=e37527e6b4ac60e1effdc8aaa1058e931930af01
>>
>> But that's just a stand-alone fixup patch to be folded in, the SOB
>> doesn't carry to other patches. So for all of them, just strip that for
>> v4. If/when it gets applied, my SOB will get attached at that point.
>>
> Sorry, paste a wrong link, should be this:
> https://git.kernel.dk/cgit/linux-block/commit/?h=fastpoll-mshot&id=289555f559f252fbfca6bdd0886316a8b17693e2

Right, but that's just me applying it to a test branch.

-- 
Jens Axboe

Re: [PATCH v3 0/4] fast poll multishot mode

[Hao Xu]

在 2022/5/7 下午11:28, Jens Axboe 写道:
> On 5/7/22 8:06 AM, Hao Xu wrote:
>> Let multishot support multishot mode, currently only add accept as its
>> first comsumer.
> 
> consumer
> 
>> theoretical analysis:
>>    1) when connections come in fast
>>      - singleshot:
>>                add accept sqe(userpsace) --> accept inline
> 
> userspace
> 
>>                                ^                 |
>>                                |-----------------|
>>      - multishot:
>>               add accept sqe(userspace) --> accept inline
>>                                                ^     |
>>                                                |--*--|
>>
>>      we do accept repeatedly in * place until get EAGAIN
>>
>>    2) when connections come in at a low pressure
>>      similar thing like 1), we reduce a lot of userspace-kernel context
>>      switch and useless vfs_poll()
> 
> Overall this looks better than v2 for sure, just some minor tweaks
> needed I believe.
> 
> But we still need to consider direct accept with multishot... Should
> probably be an add-on patch as I think it'd get a bit more complicated
> if we need to be able to cheaply find an available free fixed fd slot.
> I'll try and play with that.

I'm tending to use a new mail account to send v4 rather than the gmail
account since the git issue seems to be network related.
I'll also think about the fixed fd problem.

Thanks,
Hao

> 

Re: [PATCH 1/4] io_uring: add IORING_ACCEPT_MULTISHOT for accept

[Hao Xu]

在 2022/5/7 下午11:57, Jens Axboe 写道:
> On 5/7/22 9:52 AM, Hao Xu wrote:
>> ? 2022/5/7 ??11:38, Jens Axboe ??:
>>> On 5/7/22 9:31 AM, Hao Xu wrote:
>>>> ? 2022/5/7 ??10:16, Jens Axboe ??:
>>>>> On 5/7/22 8:06 AM, Hao Xu wrote:
>>>>>> From: Hao Xu <howeyxu@tencent.com>
>>>>>>
>>>>>> add an accept_flag IORING_ACCEPT_MULTISHOT for accept, which is to
>>>>>> support multishot.
>>>>>>
>>>>>> Signed-off-by: Hao Xu <howeyxu@tencent.com>
>>>>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>>>>
>>>>> Heh, don't add my SOB. Guessing this came from the folding in?Nop, It is in your fastpoll-mshot branch
>>>> https://git.kernel.dk/cgit/linux-block/commit/?h=fastpoll-mshot&id=e37527e6b4ac60e1effdc8aaa1058e931930af01
>>>
>>> But that's just a stand-alone fixup patch to be folded in, the SOB
>>> doesn't carry to other patches. So for all of them, just strip that for
>>> v4. If/when it gets applied, my SOB will get attached at that point.
>>>
>> Sorry, paste a wrong link, should be this:
>> https://git.kernel.dk/cgit/linux-block/commit/?h=fastpoll-mshot&id=289555f559f252fbfca6bdd0886316a8b17693e2
> 
> Right, but that's just me applying it to a test branch.
> 
I see, sorry, will remove the sign-off in v4

Re: [PATCH v3 0/4] fast poll multishot mode

[Jens Axboe]

[-- Attachment #1: Type: text/plain, Size: 917 bytes --]

On 5/7/22 10:05 AM, Hao Xu wrote:
>> But we still need to consider direct accept with multishot... Should
>> probably be an add-on patch as I think it'd get a bit more complicated
>> if we need to be able to cheaply find an available free fixed fd slot.
>> I'll try and play with that.
> 
> I'm tending to use a new mail account to send v4 rather than the gmail
> account since the git issue seems to be network related.
> I'll also think about the fixed fd problem.

Two basic attached patches that attempt do just alloc a fixed file
descriptor for this case. Not tested at all... We return the fixed file
slot in this case since we have to, to let the application know what was
picked. I kind of wish we'd done that with direct open/accept to begin
with anyway, a bit annoying that fixed vs normal open/accept behave
differently.

Anyway, something to play with, and I'm sure it can be made better.

-- 
Jens Axboe

[-- Attachment #2: 0002-io_uring-have-multishot-accept-alloc-new-fixed-file-.patch --]
[-- Type: text/x-patch, Size: 2969 bytes --]

From 66e85e379d27394398a11d02bd01abf449d7bde3 Mon Sep 17 00:00:00 2001
From: Jens Axboe <axboe@kernel.dk>
Date: Sat, 7 May 2022 10:08:31 -0600
Subject: [PATCH 2/2] io_uring: have multishot accept alloc new fixed file slot

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io_uring.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 13b99040ae90..88b95b828bed 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -258,6 +258,7 @@ struct io_rsrc_put {
 struct io_file_table {
 	struct io_fixed_file *files;
 	unsigned long *bitmap;
+	unsigned int alloc_hint;
 };
 
 struct io_rsrc_node {
@@ -5721,8 +5722,7 @@ static int io_accept_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 		return -EINVAL;
 
 	accept->file_slot = READ_ONCE(sqe->file_index);
-	if (accept->file_slot && ((accept->flags & SOCK_CLOEXEC) ||
-	   flags & IORING_ACCEPT_MULTISHOT))
+	if (accept->file_slot && (accept->flags & SOCK_CLOEXEC))
 		return -EINVAL;
 	if (accept->flags & ~(SOCK_CLOEXEC | SOCK_NONBLOCK))
 		return -EINVAL;
@@ -5750,6 +5750,18 @@ static inline void io_poll_clean(struct io_kiocb *req)
 		__io_poll_clean(req);
 }
 
+static int io_file_bitmap_get(struct io_ring_ctx *ctx)
+{
+	struct io_file_table *table = &ctx->file_table;
+	int ret;
+
+	ret = find_next_zero_bit(table->bitmap, ctx->nr_user_files,
+				 table->alloc_hint);
+	if (unlikely(ret == ctx->nr_user_files))
+		return -ENFILE;
+	return ret;
+}
+
 static int io_accept(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_ring_ctx *ctx = req->ctx;
@@ -5787,8 +5799,19 @@ static int io_accept(struct io_kiocb *req, unsigned int issue_flags)
 		fd_install(fd, file);
 		ret = fd;
 	} else {
-		ret = io_install_fixed_file(req, file, issue_flags,
-					    accept->file_slot - 1);
+		int fixed_slot = accept->file_slot - 1;
+
+		if (req->flags & REQ_F_APOLL_MULTISHOT && accept->file_slot) {
+			fixed_slot = io_file_bitmap_get(ctx);
+			if (unlikely(fixed_slot < 0)) {
+				ret = fixed_slot;
+				goto err;
+			}
+		}
+
+		ret = io_install_fixed_file(req, file, issue_flags, fixed_slot);
+		if (!ret && req->flags & REQ_F_APOLL_MULTISHOT)
+			ret = fixed_slot;
 	}
 
 	if (!(req->flags & REQ_F_APOLL_MULTISHOT)) {
@@ -5815,6 +5838,7 @@ static int io_accept(struct io_kiocb *req, unsigned int issue_flags)
 	 * since the upper layer who called io_queue_sqe() cannot get errors
 	 * happened here.
 	 */
+err:
 	io_poll_clean(req);
 	return ret;
 }
@@ -8736,11 +8760,13 @@ static void io_free_file_tables(struct io_file_table *table)
 static inline void io_file_bitmap_set(struct io_file_table *table, int bit)
 {
 	__set_bit(bit, table->bitmap);
+	table->alloc_hint = bit + 1;
 }
 
 static inline void io_file_bitmap_clear(struct io_file_table *table, int bit)
 {
 	__clear_bit(bit, table->bitmap);
+	table->alloc_hint = bit;
 }
 
 static void __io_sqe_files_unregister(struct io_ring_ctx *ctx)
-- 
2.35.1


[-- Attachment #3: 0001-io_uring-track-fixed-files-with-a-bitmap.patch --]
[-- Type: text/x-patch, Size: 3665 bytes --]

From f4801f95c01e744bd336d64ccfa5efd29a2869e2 Mon Sep 17 00:00:00 2001
From: Jens Axboe <axboe@kernel.dk>
Date: Sat, 7 May 2022 09:56:13 -0600
Subject: [PATCH 1/2] io_uring: track fixed files with a bitmap

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io_uring.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 358c195e2d99..13b99040ae90 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -257,6 +257,7 @@ struct io_rsrc_put {
 
 struct io_file_table {
 	struct io_fixed_file *files;
+	unsigned long *bitmap;
 };
 
 struct io_rsrc_node {
@@ -7645,6 +7646,7 @@ static inline struct file *io_file_get_fixed(struct io_kiocb *req, int fd,
 	/* mask in overlapping REQ_F and FFS bits */
 	req->flags |= (file_ptr << REQ_F_SUPPORT_NOWAIT_BIT);
 	io_req_set_rsrc_node(req, ctx, 0);
+	WARN_ON_ONCE(!test_bit(fd, ctx->file_table.bitmap));
 out:
 	io_ring_submit_unlock(ctx, issue_flags);
 	return file;
@@ -8711,13 +8713,34 @@ static bool io_alloc_file_tables(struct io_file_table *table, unsigned nr_files)
 {
 	table->files = kvcalloc(nr_files, sizeof(table->files[0]),
 				GFP_KERNEL_ACCOUNT);
-	return !!table->files;
+	if (unlikely(!table->files))
+		return false;
+
+	table->bitmap = bitmap_zalloc(nr_files, GFP_KERNEL_ACCOUNT);
+	if (unlikely(!table->bitmap)) {
+		kvfree(table->files);
+		return false;
+	}
+
+	return true;
 }
 
 static void io_free_file_tables(struct io_file_table *table)
 {
 	kvfree(table->files);
+	bitmap_free(table->bitmap);
 	table->files = NULL;
+	table->bitmap = NULL;
+}
+
+static inline void io_file_bitmap_set(struct io_file_table *table, int bit)
+{
+	__set_bit(bit, table->bitmap);
+}
+
+static inline void io_file_bitmap_clear(struct io_file_table *table, int bit)
+{
+	__clear_bit(bit, table->bitmap);
 }
 
 static void __io_sqe_files_unregister(struct io_ring_ctx *ctx)
@@ -8732,6 +8755,7 @@ static void __io_sqe_files_unregister(struct io_ring_ctx *ctx)
 			continue;
 		if (io_fixed_file_slot(&ctx->file_table, i)->file_ptr & FFS_SCM)
 			continue;
+		io_file_bitmap_clear(&ctx->file_table, i);
 		fput(file);
 	}
 #endif
@@ -9135,6 +9159,7 @@ static int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg,
 		}
 		file_slot = io_fixed_file_slot(&ctx->file_table, i);
 		io_fixed_file_set(file_slot, file);
+		io_file_bitmap_set(&ctx->file_table, i);
 	}
 
 	io_rsrc_node_switch(ctx, NULL);
@@ -9195,6 +9220,7 @@ static int io_install_fixed_file(struct io_kiocb *req, struct file *file,
 		if (ret)
 			goto err;
 		file_slot->file_ptr = 0;
+		io_file_bitmap_clear(&ctx->file_table, slot_index);
 		needs_switch = true;
 	}
 
@@ -9202,6 +9228,7 @@ static int io_install_fixed_file(struct io_kiocb *req, struct file *file,
 	if (!ret) {
 		*io_get_tag_slot(ctx->file_data, slot_index) = 0;
 		io_fixed_file_set(file_slot, file);
+		io_file_bitmap_set(&ctx->file_table, slot_index);
 	}
 err:
 	if (needs_switch)
@@ -9243,6 +9270,7 @@ static int io_close_fixed(struct io_kiocb *req, unsigned int issue_flags)
 		goto out;
 
 	file_slot->file_ptr = 0;
+	io_file_bitmap_clear(&ctx->file_table, offset);
 	io_rsrc_node_switch(ctx, ctx->file_data);
 	ret = 0;
 out:
@@ -9292,6 +9320,7 @@ static int __io_sqe_files_update(struct io_ring_ctx *ctx,
 			if (err)
 				break;
 			file_slot->file_ptr = 0;
+			io_file_bitmap_clear(&ctx->file_table, i);
 			needs_switch = true;
 		}
 		if (fd != -1) {
@@ -9320,6 +9349,7 @@ static int __io_sqe_files_update(struct io_ring_ctx *ctx,
 			}
 			*io_get_tag_slot(data, i) = tag;
 			io_fixed_file_set(file_slot, file);
+			io_file_bitmap_set(&ctx->file_table, i);
 		}
 	}
 
-- 
2.35.1

Re: [PATCH v3 0/4] fast poll multishot mode

[Hao Xu]

在 2022/5/8 上午12:11, Jens Axboe 写道:
> On 5/7/22 10:05 AM, Hao Xu wrote:
>>> But we still need to consider direct accept with multishot... Should
>>> probably be an add-on patch as I think it'd get a bit more complicated
>>> if we need to be able to cheaply find an available free fixed fd slot.
>>> I'll try and play with that.
>>
>> I'm tending to use a new mail account to send v4 rather than the gmail
>> account since the git issue seems to be network related.
>> I'll also think about the fixed fd problem.
> 
> Two basic attached patches that attempt do just alloc a fixed file
> descriptor for this case. Not tested at all... We return the fixed file
> slot in this case since we have to, to let the application know what was
> picked. I kind of wish we'd done that with direct open/accept to begin
> with anyway, a bit annoying that fixed vs normal open/accept behave
> differently.
> 
> Anyway, something to play with, and I'm sure it can be made better.
> 
Thanks. I tried to fix the mail account issue, still unclear what is
wrong, and too late at my timezone now, I'll try to send v4 tomorrow

Re: [PATCH v3 0/4] fast poll multishot mode

[Jens Axboe]

On 5/7/22 11:21 AM, Hao Xu wrote:
> 在 2022/5/8 上午12:11, Jens Axboe 写道:
>> On 5/7/22 10:05 AM, Hao Xu wrote:
>>>> But we still need to consider direct accept with multishot... Should
>>>> probably be an add-on patch as I think it'd get a bit more complicated
>>>> if we need to be able to cheaply find an available free fixed fd slot.
>>>> I'll try and play with that.
>>>
>>> I'm tending to use a new mail account to send v4 rather than the gmail
>>> account since the git issue seems to be network related.
>>> I'll also think about the fixed fd problem.
>>
>> Two basic attached patches that attempt do just alloc a fixed file
>> descriptor for this case. Not tested at all... We return the fixed file
>> slot in this case since we have to, to let the application know what was
>> picked. I kind of wish we'd done that with direct open/accept to begin
>> with anyway, a bit annoying that fixed vs normal open/accept behave
>> differently.
>>
>> Anyway, something to play with, and I'm sure it can be made better.
>>
> Thanks. I tried to fix the mail account issue, still unclear what is
> wrong, and too late at my timezone now, I'll try to send v4 tomorrow

No worries. IN the meantime, I played with allocated direct descriptors
yesterday and implemented them for openat/openat2/accept:

https://git.kernel.dk/cgit/linux-block/log/?h=fastpoll-mshot

It's independent of multishot accept in the sense that you can use it
without that, but multishot accept requires it with fixed files.


-- 
Jens Axboe