io_uring process termination/killing is not working

io_uring process termination/killing is not working

[Josef]

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

Hi,

I have a weird issue on kernel 5.8.0/5.8.1, SIGINT even SIGKILL
doesn't work to kill this process(always state D or D+), literally I
have to terminate my VM because even the kernel can't kill the process
and no issue on 5.7.12-201, however if IOSQE_IO_LINK is not set, it
works

I've attached a file to reproduce it
or here
https://gist.github.com/1Jo1/15cb3c63439d0c08e3589cfa98418b2c

---
Josef

[-- Attachment #2: io_uring_process.c --]
[-- Type: application/octet-stream, Size: 1878 bytes --]

#include <errno.h>
#include <fcntl.h>
#include <netinet/in.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <strings.h>
#include <sys/socket.h>
#include <unistd.h>
#include <poll.h>
#include "liburing.h"

#define BACKLOG 512

#define PORT 9100

struct io_uring ring;

void add_poll(struct io_uring *ring, int fd) {
    struct io_uring_sqe *sqe = io_uring_get_sqe(ring);
    io_uring_prep_poll_add(sqe, fd, POLLIN);
    sqe->flags |= IOSQE_IO_LINK;
}

void add_accept(struct io_uring *ring, int fd) {
    struct io_uring_sqe *sqe = io_uring_get_sqe(ring);
    io_uring_prep_accept(sqe, fd, 0, 0, SOCK_NONBLOCK | SOCK_CLOEXEC);
    sqe->flags |= IOSQE_IO_LINK;
}

int setup_io_uring() {
    int ret = io_uring_queue_init(16, &ring, 0);
    if (ret) {
        fprintf(stderr, "Unable to setup io_uring: %s\n", strerror(-ret));
        return 1;
    }
    return 0;
}

int main(int argc, char *argv[]) {

    struct sockaddr_in serv_addr;

    setup_io_uring();
    
    int sock_listen_fd = socket(AF_INET, SOCK_STREAM | SOCK_NONBLOCK, 0);
    const int val = 1;
    setsockopt(sock_listen_fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));

    memset(&serv_addr, 0, sizeof(serv_addr));
    serv_addr.sin_family = AF_INET;
    serv_addr.sin_port = htons(PORT);
    serv_addr.sin_addr.s_addr = INADDR_ANY;

    if (bind(sock_listen_fd, (struct sockaddr *)&serv_addr, sizeof(serv_addr)) < 0) {
         perror("Error binding socket\n");
         exit(1);
     }
    if (listen(sock_listen_fd, BACKLOG) < 0) {
         perror("Error listening on socket\n");
         exit(1);
    }

    setup_io_uring();

    add_poll(&ring, sock_listen_fd);
    add_accept(&ring, sock_listen_fd);
    io_uring_submit(&ring);

    
    while (1) {
        struct io_uring_cqe *cqe;
        io_uring_wait_cqe(&ring, &cqe);

    }

    io_uring_queue_exit(&ring);

    return 0;
}

Re: io_uring process termination/killing is not working

[Jens Axboe]

On 8/12/20 11:58 AM, Josef wrote:
> Hi,
> 
> I have a weird issue on kernel 5.8.0/5.8.1, SIGINT even SIGKILL
> doesn't work to kill this process(always state D or D+), literally I
> have to terminate my VM because even the kernel can't kill the process
> and no issue on 5.7.12-201, however if IOSQE_IO_LINK is not set, it
> works
> 
> I've attached a file to reproduce it
> or here
> https://gist.github.com/1Jo1/15cb3c63439d0c08e3589cfa98418b2c

Thanks, I'll take a look at this. It's stuck in uninterruptible
state, which is why you can't kill it.

-- 
Jens Axboe

Re: io_uring process termination/killing is not working

[Pavel Begunkov]

On 12/08/2020 21:05, Jens Axboe wrote:
> On 8/12/20 11:58 AM, Josef wrote:
>> Hi,
>>
>> I have a weird issue on kernel 5.8.0/5.8.1, SIGINT even SIGKILL
>> doesn't work to kill this process(always state D or D+), literally I
>> have to terminate my VM because even the kernel can't kill the process
>> and no issue on 5.7.12-201, however if IOSQE_IO_LINK is not set, it
>> works
>>
>> I've attached a file to reproduce it
>> or here
>> https://gist.github.com/1Jo1/15cb3c63439d0c08e3589cfa98418b2c
> 
> Thanks, I'll take a look at this. It's stuck in uninterruptible
> state, which is why you can't kill it.

It looks like one of the hangs I've been talking about a few days ago,
an accept is inflight but can't be found by cancel_files() because it's
in a link.

-- 
Pavel Begunkov

Re: io_uring process termination/killing is not working

[Pavel Begunkov]

On 12/08/2020 21:20, Pavel Begunkov wrote:
> On 12/08/2020 21:05, Jens Axboe wrote:
>> On 8/12/20 11:58 AM, Josef wrote:
>>> Hi,
>>>
>>> I have a weird issue on kernel 5.8.0/5.8.1, SIGINT even SIGKILL
>>> doesn't work to kill this process(always state D or D+), literally I
>>> have to terminate my VM because even the kernel can't kill the process
>>> and no issue on 5.7.12-201, however if IOSQE_IO_LINK is not set, it
>>> works
>>>
>>> I've attached a file to reproduce it
>>> or here
>>> https://gist.github.com/1Jo1/15cb3c63439d0c08e3589cfa98418b2c
>>
>> Thanks, I'll take a look at this. It's stuck in uninterruptible
>> state, which is why you can't kill it.
> 
> It looks like one of the hangs I've been talking about a few days ago,
> an accept is inflight but can't be found by cancel_files() because it's
> in a link.

BTW, I described it a month ago, there were more details.

-- 
Pavel Begunkov

Re: io_uring process termination/killing is not working

[Pavel Begunkov]

On 12/08/2020 21:22, Pavel Begunkov wrote:
> On 12/08/2020 21:20, Pavel Begunkov wrote:
>> On 12/08/2020 21:05, Jens Axboe wrote:
>>> On 8/12/20 11:58 AM, Josef wrote:
>>>> Hi,
>>>>
>>>> I have a weird issue on kernel 5.8.0/5.8.1, SIGINT even SIGKILL
>>>> doesn't work to kill this process(always state D or D+), literally I
>>>> have to terminate my VM because even the kernel can't kill the process
>>>> and no issue on 5.7.12-201, however if IOSQE_IO_LINK is not set, it
>>>> works
>>>>
>>>> I've attached a file to reproduce it
>>>> or here
>>>> https://gist.github.com/1Jo1/15cb3c63439d0c08e3589cfa98418b2c
>>>
>>> Thanks, I'll take a look at this. It's stuck in uninterruptible
>>> state, which is why you can't kill it.
>>
>> It looks like one of the hangs I've been talking about a few days ago,
>> an accept is inflight but can't be found by cancel_files() because it's
>> in a link.
> 
> BTW, I described it a month ago, there were more details.

https://lore.kernel.org/io-uring/34eb5e5a-8d37-0cae-be6c-c6ac4d85b5d4@gmail.com

-- 
Pavel Begunkov

Re: io_uring process termination/killing is not working

[Jens Axboe]

On 8/12/20 12:28 PM, Pavel Begunkov wrote:
> On 12/08/2020 21:22, Pavel Begunkov wrote:
>> On 12/08/2020 21:20, Pavel Begunkov wrote:
>>> On 12/08/2020 21:05, Jens Axboe wrote:
>>>> On 8/12/20 11:58 AM, Josef wrote:
>>>>> Hi,
>>>>>
>>>>> I have a weird issue on kernel 5.8.0/5.8.1, SIGINT even SIGKILL
>>>>> doesn't work to kill this process(always state D or D+), literally I
>>>>> have to terminate my VM because even the kernel can't kill the process
>>>>> and no issue on 5.7.12-201, however if IOSQE_IO_LINK is not set, it
>>>>> works
>>>>>
>>>>> I've attached a file to reproduce it
>>>>> or here
>>>>> https://gist.github.com/1Jo1/15cb3c63439d0c08e3589cfa98418b2c
>>>>
>>>> Thanks, I'll take a look at this. It's stuck in uninterruptible
>>>> state, which is why you can't kill it.
>>>
>>> It looks like one of the hangs I've been talking about a few days ago,
>>> an accept is inflight but can't be found by cancel_files() because it's
>>> in a link.
>>
>> BTW, I described it a month ago, there were more details.
> 
> https://lore.kernel.org/io-uring/34eb5e5a-8d37-0cae-be6c-c6ac4d85b5d4@gmail.com

Yeah I think you're right. How about something like the below? That'll
potentially cancel more than just the one we're looking for, but seems
kind of silly to only cancel from the file table holding request and to
the end.


diff --git a/fs/io_uring.c b/fs/io_uring.c
index 8a2afd8c33c9..0630a9622baa 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -4937,6 +5003,7 @@ static bool io_poll_remove_one(struct io_kiocb *req)
 		io_cqring_fill_event(req, -ECANCELED);
 		io_commit_cqring(req->ctx);
 		req->flags |= REQ_F_COMP_LOCKED;
+		req_set_fail_links(req);
 		io_put_req(req);
 	}
 
@@ -7935,6 +8002,47 @@ static bool io_wq_files_match(struct io_wq_work *work, void *data)
 	return work->files == files;
 }
 
+static bool __io_poll_remove_link(struct io_kiocb *preq, struct io_kiocb *req)
+{
+	struct io_kiocb *link;
+
+	if (!(preq->flags & REQ_F_LINK_HEAD))
+		return false;
+
+	list_for_each_entry(link, &preq->link_list, link_list) {
+		if (link != req)
+			break;
+		io_poll_remove_one(preq);
+		return true;
+	}
+
+	return false;
+}
+
+/*
+ * We're looking to cancel 'req' because it's holding on to our files, but
+ * 'req' could be a link to another request. See if it is, and cancel that
+ * parent request if so.
+ */
+static void io_poll_remove_link(struct io_ring_ctx *ctx, struct io_kiocb *req)
+{
+	struct hlist_node *tmp;
+	struct io_kiocb *preq;
+	int i;
+
+	spin_lock_irq(&ctx->completion_lock);
+	for (i = 0; i < (1U << ctx->cancel_hash_bits); i++) {
+		struct hlist_head *list;
+
+		list = &ctx->cancel_hash[i];
+		hlist_for_each_entry_safe(preq, tmp, list, hash_node) {
+			if (__io_poll_remove_link(preq, req))
+				break;
+		}
+	}
+	spin_unlock_irq(&ctx->completion_lock);
+}
+
 static void io_uring_cancel_files(struct io_ring_ctx *ctx,
 				  struct files_struct *files)
 {
@@ -7989,6 +8097,8 @@ static void io_uring_cancel_files(struct io_ring_ctx *ctx,
 			}
 		} else {
 			io_wq_cancel_work(ctx->io_wq, &cancel_req->work);
+			/* could be a link, check and remove if it is */
+			io_poll_remove_link(ctx, cancel_req);
 			io_put_req(cancel_req);
 		}
 

-- 
Jens Axboe

Re: io_uring process termination/killing is not working

[Josef]

On Thu, 13 Aug 2020 at 01:32, Jens Axboe <axboe@kernel.dk> wrote:
> Yeah I think you're right. How about something like the below? That'll
> potentially cancel more than just the one we're looking for, but seems
> kind of silly to only cancel from the file table holding request and to
> the end.
>
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 8a2afd8c33c9..0630a9622baa 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -4937,6 +5003,7 @@ static bool io_poll_remove_one(struct io_kiocb *req)
>                 io_cqring_fill_event(req, -ECANCELED);
>                 io_commit_cqring(req->ctx);
>                 req->flags |= REQ_F_COMP_LOCKED;
> +               req_set_fail_links(req);
>                 io_put_req(req);
>         }
>
> @@ -7935,6 +8002,47 @@ static bool io_wq_files_match(struct io_wq_work *work, void *data)
>         return work->files == files;
>  }
>
> +static bool __io_poll_remove_link(struct io_kiocb *preq, struct io_kiocb *req)
> +{
> +       struct io_kiocb *link;
> +
> +       if (!(preq->flags & REQ_F_LINK_HEAD))
> +               return false;
> +
> +       list_for_each_entry(link, &preq->link_list, link_list) {
> +               if (link != req)
> +                       break;
> +               io_poll_remove_one(preq);
> +               return true;
> +       }
> +
> +       return false;
> +}
> +
> +/*
> + * We're looking to cancel 'req' because it's holding on to our files, but
> + * 'req' could be a link to another request. See if it is, and cancel that
> + * parent request if so.
> + */
> +static void io_poll_remove_link(struct io_ring_ctx *ctx, struct io_kiocb *req)
> +{
> +       struct hlist_node *tmp;
> +       struct io_kiocb *preq;
> +       int i;
> +
> +       spin_lock_irq(&ctx->completion_lock);
> +       for (i = 0; i < (1U << ctx->cancel_hash_bits); i++) {
> +               struct hlist_head *list;
> +
> +               list = &ctx->cancel_hash[i];
> +               hlist_for_each_entry_safe(preq, tmp, list, hash_node) {
> +                       if (__io_poll_remove_link(preq, req))
> +                               break;
> +               }
> +       }
> +       spin_unlock_irq(&ctx->completion_lock);
> +}
> +
>  static void io_uring_cancel_files(struct io_ring_ctx *ctx,
>                                   struct files_struct *files)
>  {
> @@ -7989,6 +8097,8 @@ static void io_uring_cancel_files(struct io_ring_ctx *ctx,
>                         }
>                 } else {
>                         io_wq_cancel_work(ctx->io_wq, &cancel_req->work);
> +                       /* could be a link, check and remove if it is */
> +                       io_poll_remove_link(ctx, cancel_req);
>                         io_put_req(cancel_req);
>                 }
>
>

btw it works for me thanks

--
Josef


On Thu, 13 Aug 2020 at 01:32, Jens Axboe <axboe@kernel.dk> wrote:
>
> On 8/12/20 12:28 PM, Pavel Begunkov wrote:
> > On 12/08/2020 21:22, Pavel Begunkov wrote:
> >> On 12/08/2020 21:20, Pavel Begunkov wrote:
> >>> On 12/08/2020 21:05, Jens Axboe wrote:
> >>>> On 8/12/20 11:58 AM, Josef wrote:
> >>>>> Hi,
> >>>>>
> >>>>> I have a weird issue on kernel 5.8.0/5.8.1, SIGINT even SIGKILL
> >>>>> doesn't work to kill this process(always state D or D+), literally I
> >>>>> have to terminate my VM because even the kernel can't kill the process
> >>>>> and no issue on 5.7.12-201, however if IOSQE_IO_LINK is not set, it
> >>>>> works
> >>>>>
> >>>>> I've attached a file to reproduce it
> >>>>> or here
> >>>>> https://gist.github.com/1Jo1/15cb3c63439d0c08e3589cfa98418b2c
> >>>>
> >>>> Thanks, I'll take a look at this. It's stuck in uninterruptible
> >>>> state, which is why you can't kill it.
> >>>
> >>> It looks like one of the hangs I've been talking about a few days ago,
> >>> an accept is inflight but can't be found by cancel_files() because it's
> >>> in a link.
> >>
> >> BTW, I described it a month ago, there were more details.
> >
> > https://lore.kernel.org/io-uring/34eb5e5a-8d37-0cae-be6c-c6ac4d85b5d4@gmail.com
>
> Yeah I think you're right. How about something like the below? That'll
> potentially cancel more than just the one we're looking for, but seems
> kind of silly to only cancel from the file table holding request and to
> the end.
>
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 8a2afd8c33c9..0630a9622baa 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -4937,6 +5003,7 @@ static bool io_poll_remove_one(struct io_kiocb *req)
>                 io_cqring_fill_event(req, -ECANCELED);
>                 io_commit_cqring(req->ctx);
>                 req->flags |= REQ_F_COMP_LOCKED;
> +               req_set_fail_links(req);
>                 io_put_req(req);
>         }
>
> @@ -7935,6 +8002,47 @@ static bool io_wq_files_match(struct io_wq_work *work, void *data)
>         return work->files == files;
>  }
>
> +static bool __io_poll_remove_link(struct io_kiocb *preq, struct io_kiocb *req)
> +{
> +       struct io_kiocb *link;
> +
> +       if (!(preq->flags & REQ_F_LINK_HEAD))
> +               return false;
> +
> +       list_for_each_entry(link, &preq->link_list, link_list) {
> +               if (link != req)
> +                       break;
> +               io_poll_remove_one(preq);
> +               return true;
> +       }
> +
> +       return false;
> +}
> +
> +/*
> + * We're looking to cancel 'req' because it's holding on to our files, but
> + * 'req' could be a link to another request. See if it is, and cancel that
> + * parent request if so.
> + */
> +static void io_poll_remove_link(struct io_ring_ctx *ctx, struct io_kiocb *req)
> +{
> +       struct hlist_node *tmp;
> +       struct io_kiocb *preq;
> +       int i;
> +
> +       spin_lock_irq(&ctx->completion_lock);
> +       for (i = 0; i < (1U << ctx->cancel_hash_bits); i++) {
> +               struct hlist_head *list;
> +
> +               list = &ctx->cancel_hash[i];
> +               hlist_for_each_entry_safe(preq, tmp, list, hash_node) {
> +                       if (__io_poll_remove_link(preq, req))
> +                               break;
> +               }
> +       }
> +       spin_unlock_irq(&ctx->completion_lock);
> +}
> +
>  static void io_uring_cancel_files(struct io_ring_ctx *ctx,
>                                   struct files_struct *files)
>  {
> @@ -7989,6 +8097,8 @@ static void io_uring_cancel_files(struct io_ring_ctx *ctx,
>                         }
>                 } else {
>                         io_wq_cancel_work(ctx->io_wq, &cancel_req->work);
> +                       /* could be a link, check and remove if it is */
> +                       io_poll_remove_link(ctx, cancel_req);
>                         io_put_req(cancel_req);
>                 }
>
>
> --
> Jens Axboe
>

Re: io_uring process termination/killing is not working

[Jens Axboe]

On 8/13/20 10:07 AM, Josef wrote:
> On Thu, 13 Aug 2020 at 01:32, Jens Axboe <axboe@kernel.dk> wrote:
>> Yeah I think you're right. How about something like the below? That'll
>> potentially cancel more than just the one we're looking for, but seems
>> kind of silly to only cancel from the file table holding request and to
>> the end.
>>
>>
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index 8a2afd8c33c9..0630a9622baa 100644
>> --- a/fs/io_uring.c
>> +++ b/fs/io_uring.c
>> @@ -4937,6 +5003,7 @@ static bool io_poll_remove_one(struct io_kiocb *req)
>>                 io_cqring_fill_event(req, -ECANCELED);
>>                 io_commit_cqring(req->ctx);
>>                 req->flags |= REQ_F_COMP_LOCKED;
>> +               req_set_fail_links(req);
>>                 io_put_req(req);
>>         }
>>
>> @@ -7935,6 +8002,47 @@ static bool io_wq_files_match(struct io_wq_work *work, void *data)
>>         return work->files == files;
>>  }
>>
>> +static bool __io_poll_remove_link(struct io_kiocb *preq, struct io_kiocb *req)
>> +{
>> +       struct io_kiocb *link;
>> +
>> +       if (!(preq->flags & REQ_F_LINK_HEAD))
>> +               return false;
>> +
>> +       list_for_each_entry(link, &preq->link_list, link_list) {
>> +               if (link != req)
>> +                       break;
>> +               io_poll_remove_one(preq);
>> +               return true;
>> +       }
>> +
>> +       return false;
>> +}
>> +
>> +/*
>> + * We're looking to cancel 'req' because it's holding on to our files, but
>> + * 'req' could be a link to another request. See if it is, and cancel that
>> + * parent request if so.
>> + */
>> +static void io_poll_remove_link(struct io_ring_ctx *ctx, struct io_kiocb *req)
>> +{
>> +       struct hlist_node *tmp;
>> +       struct io_kiocb *preq;
>> +       int i;
>> +
>> +       spin_lock_irq(&ctx->completion_lock);
>> +       for (i = 0; i < (1U << ctx->cancel_hash_bits); i++) {
>> +               struct hlist_head *list;
>> +
>> +               list = &ctx->cancel_hash[i];
>> +               hlist_for_each_entry_safe(preq, tmp, list, hash_node) {
>> +                       if (__io_poll_remove_link(preq, req))
>> +                               break;
>> +               }
>> +       }
>> +       spin_unlock_irq(&ctx->completion_lock);
>> +}
>> +
>>  static void io_uring_cancel_files(struct io_ring_ctx *ctx,
>>                                   struct files_struct *files)
>>  {
>> @@ -7989,6 +8097,8 @@ static void io_uring_cancel_files(struct io_ring_ctx *ctx,
>>                         }
>>                 } else {
>>                         io_wq_cancel_work(ctx->io_wq, &cancel_req->work);
>> +                       /* could be a link, check and remove if it is */
>> +                       io_poll_remove_link(ctx, cancel_req);
>>                         io_put_req(cancel_req);
>>                 }
>>
>>
> 
> btw it works for me thanks

Thanks for testing, I've committed an updated version that also ensures
we find timeout based links:

https://git.kernel.dk/cgit/linux-block/commit/?h=io_uring-5.9&id=f254ac04c8744cf7bfed012717eac34eacc65dfb

-- 
Jens Axboe

Re: io_uring process termination/killing is not working

[Pavel Begunkov]

On 13/08/2020 02:32, Jens Axboe wrote:
> On 8/12/20 12:28 PM, Pavel Begunkov wrote:
>> On 12/08/2020 21:22, Pavel Begunkov wrote:
>>> On 12/08/2020 21:20, Pavel Begunkov wrote:
>>>> On 12/08/2020 21:05, Jens Axboe wrote:
>>>>> On 8/12/20 11:58 AM, Josef wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I have a weird issue on kernel 5.8.0/5.8.1, SIGINT even SIGKILL
>>>>>> doesn't work to kill this process(always state D or D+), literally I
>>>>>> have to terminate my VM because even the kernel can't kill the process
>>>>>> and no issue on 5.7.12-201, however if IOSQE_IO_LINK is not set, it
>>>>>> works
>>>>>>
>>>>>> I've attached a file to reproduce it
>>>>>> or here
>>>>>> https://gist.github.com/1Jo1/15cb3c63439d0c08e3589cfa98418b2c
>>>>>
>>>>> Thanks, I'll take a look at this. It's stuck in uninterruptible
>>>>> state, which is why you can't kill it.
>>>>
>>>> It looks like one of the hangs I've been talking about a few days ago,
>>>> an accept is inflight but can't be found by cancel_files() because it's
>>>> in a link.
>>>
>>> BTW, I described it a month ago, there were more details.
>>
>> https://lore.kernel.org/io-uring/34eb5e5a-8d37-0cae-be6c-c6ac4d85b5d4@gmail.com
> 
> Yeah I think you're right. How about something like the below? That'll
> potentially cancel more than just the one we're looking for, but seems
> kind of silly to only cancel from the file table holding request and to
> the end.

The bug is not poll/t-out related, IIRC my test reproduces it with
read(pipe)->open(). See the previously sent link.

As mentioned, I'm going to patch that up, if you won't beat me on that.

> 
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 8a2afd8c33c9..0630a9622baa 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -4937,6 +5003,7 @@ static bool io_poll_remove_one(struct io_kiocb *req)
>  		io_cqring_fill_event(req, -ECANCELED);
>  		io_commit_cqring(req->ctx);
>  		req->flags |= REQ_F_COMP_LOCKED;
> +		req_set_fail_links(req);
>  		io_put_req(req);
>  	}
>  
> @@ -7935,6 +8002,47 @@ static bool io_wq_files_match(struct io_wq_work *work, void *data)
>  	return work->files == files;
>  }
>  
> +static bool __io_poll_remove_link(struct io_kiocb *preq, struct io_kiocb *req)
> +{
> +	struct io_kiocb *link;
> +
> +	if (!(preq->flags & REQ_F_LINK_HEAD))
> +		return false;
> +
> +	list_for_each_entry(link, &preq->link_list, link_list) {
> +		if (link != req)
> +			break;
> +		io_poll_remove_one(preq);
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +/*
> + * We're looking to cancel 'req' because it's holding on to our files, but
> + * 'req' could be a link to another request. See if it is, and cancel that
> + * parent request if so.
> + */
> +static void io_poll_remove_link(struct io_ring_ctx *ctx, struct io_kiocb *req)
> +{
> +	struct hlist_node *tmp;
> +	struct io_kiocb *preq;
> +	int i;
> +
> +	spin_lock_irq(&ctx->completion_lock);
> +	for (i = 0; i < (1U << ctx->cancel_hash_bits); i++) {
> +		struct hlist_head *list;
> +
> +		list = &ctx->cancel_hash[i];
> +		hlist_for_each_entry_safe(preq, tmp, list, hash_node) {
> +			if (__io_poll_remove_link(preq, req))
> +				break;
> +		}
> +	}
> +	spin_unlock_irq(&ctx->completion_lock);
> +}
> +
>  static void io_uring_cancel_files(struct io_ring_ctx *ctx,
>  				  struct files_struct *files)
>  {
> @@ -7989,6 +8097,8 @@ static void io_uring_cancel_files(struct io_ring_ctx *ctx,
>  			}
>  		} else {
>  			io_wq_cancel_work(ctx->io_wq, &cancel_req->work);
> +			/* could be a link, check and remove if it is */
> +			io_poll_remove_link(ctx, cancel_req);
>  			io_put_req(cancel_req);
>  		}
>  
> 

-- 
Pavel Begunkov

Re: io_uring process termination/killing is not working

[Jens Axboe]

On 8/15/20 12:45 AM, Pavel Begunkov wrote:
> On 13/08/2020 02:32, Jens Axboe wrote:
>> On 8/12/20 12:28 PM, Pavel Begunkov wrote:
>>> On 12/08/2020 21:22, Pavel Begunkov wrote:
>>>> On 12/08/2020 21:20, Pavel Begunkov wrote:
>>>>> On 12/08/2020 21:05, Jens Axboe wrote:
>>>>>> On 8/12/20 11:58 AM, Josef wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> I have a weird issue on kernel 5.8.0/5.8.1, SIGINT even SIGKILL
>>>>>>> doesn't work to kill this process(always state D or D+), literally I
>>>>>>> have to terminate my VM because even the kernel can't kill the process
>>>>>>> and no issue on 5.7.12-201, however if IOSQE_IO_LINK is not set, it
>>>>>>> works
>>>>>>>
>>>>>>> I've attached a file to reproduce it
>>>>>>> or here
>>>>>>> https://gist.github.com/1Jo1/15cb3c63439d0c08e3589cfa98418b2c
>>>>>>
>>>>>> Thanks, I'll take a look at this. It's stuck in uninterruptible
>>>>>> state, which is why you can't kill it.
>>>>>
>>>>> It looks like one of the hangs I've been talking about a few days ago,
>>>>> an accept is inflight but can't be found by cancel_files() because it's
>>>>> in a link.
>>>>
>>>> BTW, I described it a month ago, there were more details.
>>>
>>> https://lore.kernel.org/io-uring/34eb5e5a-8d37-0cae-be6c-c6ac4d85b5d4@gmail.com
>>
>> Yeah I think you're right. How about something like the below? That'll
>> potentially cancel more than just the one we're looking for, but seems
>> kind of silly to only cancel from the file table holding request and to
>> the end.
> 
> The bug is not poll/t-out related, IIRC my test reproduces it with
> read(pipe)->open(). See the previously sent link.

Right, but in this context for poll, I just mean any request that has a
poll handler armed. Not necessarily only a pure poll. The patch should
fix your case, too.

> As mentioned, I'm going to patch that up, if you won't beat me on that.

Please test and send a fix if you find something! I'm going to ship what
I have this weekend, but we can always add a fix on top if we need
anything.

-- 
Jens Axboe

Re: io_uring process termination/killing is not working

[Pavel Begunkov]

On 15/08/2020 18:12, Jens Axboe wrote:
> On 8/15/20 12:45 AM, Pavel Begunkov wrote:
>> On 13/08/2020 02:32, Jens Axboe wrote:
>>> On 8/12/20 12:28 PM, Pavel Begunkov wrote:
>>>> On 12/08/2020 21:22, Pavel Begunkov wrote:
>>>>> On 12/08/2020 21:20, Pavel Begunkov wrote:
>>>>>> On 12/08/2020 21:05, Jens Axboe wrote:
>>>>>>> On 8/12/20 11:58 AM, Josef wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> I have a weird issue on kernel 5.8.0/5.8.1, SIGINT even SIGKILL
>>>>>>>> doesn't work to kill this process(always state D or D+), literally I
>>>>>>>> have to terminate my VM because even the kernel can't kill the process
>>>>>>>> and no issue on 5.7.12-201, however if IOSQE_IO_LINK is not set, it
>>>>>>>> works
>>>>>>>>
>>>>>>>> I've attached a file to reproduce it
>>>>>>>> or here
>>>>>>>> https://gist.github.com/1Jo1/15cb3c63439d0c08e3589cfa98418b2c
>>>>>>>
>>>>>>> Thanks, I'll take a look at this. It's stuck in uninterruptible
>>>>>>> state, which is why you can't kill it.
>>>>>>
>>>>>> It looks like one of the hangs I've been talking about a few days ago,
>>>>>> an accept is inflight but can't be found by cancel_files() because it's
>>>>>> in a link.
>>>>>
>>>>> BTW, I described it a month ago, there were more details.
>>>>
>>>> https://lore.kernel.org/io-uring/34eb5e5a-8d37-0cae-be6c-c6ac4d85b5d4@gmail.com
>>>
>>> Yeah I think you're right. How about something like the below? That'll
>>> potentially cancel more than just the one we're looking for, but seems
>>> kind of silly to only cancel from the file table holding request and to
>>> the end.
>>
>> The bug is not poll/t-out related, IIRC my test reproduces it with
>> read(pipe)->open(). See the previously sent link.
> 
> Right, but in this context for poll, I just mean any request that has a
> poll handler armed. Not necessarily only a pure poll. The patch should
> fix your case, too.

Ok. I was thinking about sleeping in io_read(), etc. from io-wq context.
That should have the same effect.

> 
>> As mentioned, I'm going to patch that up, if you won't beat me on that.
> 
> Please test and send a fix if you find something! I'm going to ship what
> I have this weekend, but we can always add a fix on top if we need
> anything.

Sure

-- 
Pavel Begunkov

Re: io_uring process termination/killing is not working

[Josef]

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

it seems to be that read event doesn't work properly, but I'm not sure
if it is related to what Pavel mentioned
poll<link>accept works but not poll<link>read -> cqe still receives
poll event but no read event, however I received a read event after
the third request via telnet

I just tested https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=io_uring-5.9&id=d4e7cd36a90e38e0276d6ce0c20f5ccef17ec38c
and
https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=io_uring-5.9&id=227c0c9673d86732995474d277f84e08ee763e46
(but it works on Linux 5.7)


On Sat, 15 Aug 2020 at 18:50, Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> On 15/08/2020 18:12, Jens Axboe wrote:
> > On 8/15/20 12:45 AM, Pavel Begunkov wrote:
> >> On 13/08/2020 02:32, Jens Axboe wrote:
> >>> On 8/12/20 12:28 PM, Pavel Begunkov wrote:
> >>>> On 12/08/2020 21:22, Pavel Begunkov wrote:
> >>>>> On 12/08/2020 21:20, Pavel Begunkov wrote:
> >>>>>> On 12/08/2020 21:05, Jens Axboe wrote:
> >>>>>>> On 8/12/20 11:58 AM, Josef wrote:
> >>>>>>>> Hi,
> >>>>>>>>
> >>>>>>>> I have a weird issue on kernel 5.8.0/5.8.1, SIGINT even SIGKILL
> >>>>>>>> doesn't work to kill this process(always state D or D+), literally I
> >>>>>>>> have to terminate my VM because even the kernel can't kill the process
> >>>>>>>> and no issue on 5.7.12-201, however if IOSQE_IO_LINK is not set, it
> >>>>>>>> works
> >>>>>>>>
> >>>>>>>> I've attached a file to reproduce it
> >>>>>>>> or here
> >>>>>>>> https://gist.github.com/1Jo1/15cb3c63439d0c08e3589cfa98418b2c
> >>>>>>>
> >>>>>>> Thanks, I'll take a look at this. It's stuck in uninterruptible
> >>>>>>> state, which is why you can't kill it.
> >>>>>>
> >>>>>> It looks like one of the hangs I've been talking about a few days ago,
> >>>>>> an accept is inflight but can't be found by cancel_files() because it's
> >>>>>> in a link.
> >>>>>
> >>>>> BTW, I described it a month ago, there were more details.
> >>>>
> >>>> https://lore.kernel.org/io-uring/34eb5e5a-8d37-0cae-be6c-c6ac4d85b5d4@gmail.com
> >>>
> >>> Yeah I think you're right. How about something like the below? That'll
> >>> potentially cancel more than just the one we're looking for, but seems
> >>> kind of silly to only cancel from the file table holding request and to
> >>> the end.
> >>
> >> The bug is not poll/t-out related, IIRC my test reproduces it with
> >> read(pipe)->open(). See the previously sent link.
> >
> > Right, but in this context for poll, I just mean any request that has a
> > poll handler armed. Not necessarily only a pure poll. The patch should
> > fix your case, too.
>
> Ok. I was thinking about sleeping in io_read(), etc. from io-wq context.
> That should have the same effect.
>
> >
> >> As mentioned, I'm going to patch that up, if you won't beat me on that.
> >
> > Please test and send a fix if you find something! I'm going to ship what
> > I have this weekend, but we can always add a fix on top if we need
> > anything.
>
> Sure
>
> --
> Pavel Begunkov

--
Josef Grieb

[-- Attachment #2: io_uring_read_issue.c --]
[-- Type: application/octet-stream, Size: 2793 bytes --]

#include <errno.h>
#include <fcntl.h>
#include <netinet/in.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <strings.h>
#include <sys/socket.h>
#include <unistd.h>
#include <poll.h>
#include "liburing.h"

#define BACKLOG 512

#define PORT 9300

struct io_uring ring;

char buf[100];

void add_poll(int fd) {
    struct io_uring_sqe *sqe = io_uring_get_sqe(&ring);
    io_uring_prep_poll_add(sqe, fd, POLLIN);
    sqe->user_data = 1;
    sqe->flags |= IOSQE_IO_LINK;
}

void add_accept(int fd) {
    struct io_uring_sqe *sqe = io_uring_get_sqe(&ring);
    io_uring_prep_accept(sqe, fd, 0, 0, SOCK_NONBLOCK | SOCK_CLOEXEC);
    sqe->user_data = 2;
    sqe->flags |= IOSQE_IO_LINK;
}

void add_read(int fd) {
    struct io_uring_sqe *sqe = io_uring_get_sqe(&ring);
    io_uring_prep_read(sqe, fd, &buf, 100, 0);
    sqe->user_data = 3;
    sqe->flags |= IOSQE_IO_LINK;
}

int setup_io_uring() {
    int ret = io_uring_queue_init(16, &ring, 0);
    if (ret) {
        fprintf(stderr, "Unable to setup io_uring: %s\n", strerror(-ret));
        return 1;
    }
    return 0;
}

int main(int argc, char *argv[]) {

    struct sockaddr_in serv_addr;

    setup_io_uring();
    
    int sock_listen_fd = socket(AF_INET, SOCK_STREAM | SOCK_NONBLOCK, 0);
    const int val = 1;
    setsockopt(sock_listen_fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));

    memset(&serv_addr, 0, sizeof(serv_addr));
    serv_addr.sin_family = AF_INET;
    serv_addr.sin_port = htons(PORT);
    serv_addr.sin_addr.s_addr = INADDR_ANY;

    if (bind(sock_listen_fd, (struct sockaddr *)&serv_addr, sizeof(serv_addr)) < 0) {
         perror("Error binding socket\n");
         exit(1);
     }
    if (listen(sock_listen_fd, BACKLOG) < 0) {
         perror("Error listening on socket\n");
         exit(1);
    }

    setup_io_uring();

    add_poll(sock_listen_fd);
    add_accept(sock_listen_fd);
    io_uring_submit(&ring);

    while (1) {
        struct io_uring_cqe *cqe;
        io_uring_wait_cqe(&ring, &cqe);

        printf("Res: res: %d\n", cqe->res);
        
        if (cqe->user_data == 1) {
            printf("Poll Event\n");
        }
        
        if (cqe->user_data == 2 && cqe->res > 0) {
            printf("Accept Event\n");
                    
            add_poll(sock_listen_fd);
            add_accept(sock_listen_fd);

            //avoid link between add_accept and add_poll
            struct io_uring_sqe *sqe = io_uring_get_sqe(&ring);
            io_uring_prep_nop(sqe); 

            add_poll(cqe->res);
            add_read(cqe->res);
        }

        if (cqe->user_data == 3) {
            printf("Read Buf: %s \n", buf);
        }
        io_uring_submit(&ring);

        io_uring_cqe_seen(&ring, cqe);
    }

    io_uring_queue_exit(&ring);

    return 0;
}

Re: io_uring process termination/killing is not working

[Jens Axboe]

On 8/15/20 2:43 PM, Josef wrote:
> it seems to be that read event doesn't work properly, but I'm not sure
> if it is related to what Pavel mentioned
> poll<link>accept works but not poll<link>read -> cqe still receives
> poll event but no read event, however I received a read event after
> the third request via telnet
> 
> I just tested https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=io_uring-5.9&id=d4e7cd36a90e38e0276d6ce0c20f5ccef17ec38c
> and
> https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=io_uring-5.9&id=227c0c9673d86732995474d277f84e08ee763e46
> (but it works on Linux 5.7)

I'll take a look. BTW, you seem to be using links in a funny way. You set the
IOSQE_IO_LINK on the start of a link chain, and then the chain stops when
you _don't_ have that flag set. You just set it on everything, then
work-around it with a NOP?

For this example, only the poll should have IOSQE_IO_LINK set, accept
and read should not.

This isn't causing your issue, just wanted to clarify how links are
used.

-- 
Jens Axboe

Re: io_uring process termination/killing is not working

[Josef]

> I'll take a look. BTW, you seem to be using links in a funny way. You set the
> IOSQE_IO_LINK on the start of a link chain, and then the chain stops when
> you _don't_ have that flag set. You just set it on everything, then
> work-around it with a NOP?
>
> For this example, only the poll should have IOSQE_IO_LINK set, accept
> and read should not.

haha yeah...thanks that makes more sense, I should read the man page
more carefully!!

--
Josef Grieb

Re: io_uring process termination/killing is not working

[Jens Axboe]

On 8/15/20 3:35 PM, Jens Axboe wrote:
> On 8/15/20 2:43 PM, Josef wrote:
>> it seems to be that read event doesn't work properly, but I'm not sure
>> if it is related to what Pavel mentioned
>> poll<link>accept works but not poll<link>read -> cqe still receives
>> poll event but no read event, however I received a read event after
>> the third request via telnet
>>
>> I just tested https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=io_uring-5.9&id=d4e7cd36a90e38e0276d6ce0c20f5ccef17ec38c
>> and
>> https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=io_uring-5.9&id=227c0c9673d86732995474d277f84e08ee763e46
>> (but it works on Linux 5.7)
> 
> I'll take a look. BTW, you seem to be using links in a funny way. You set the
> IOSQE_IO_LINK on the start of a link chain, and then the chain stops when
> you _don't_ have that flag set. You just set it on everything, then
> work-around it with a NOP?
> 
> For this example, only the poll should have IOSQE_IO_LINK set, accept
> and read should not.
> 
> This isn't causing your issue, just wanted to clarify how links are
> used.

Please try:

https://git.kernel.dk/cgit/linux-block/commit/?h=io_uring-5.9&id=41d3344604e80db0e466f9deca5262b0914e4827

There was a bug with the -EAGAIN doing repeated retries on sockets that
are marked non-blocking.

-- 
Jens Axboe

Re: io_uring process termination/killing is not working

[Josef]

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

> Please try:
>
> https://git.kernel.dk/cgit/linux-block/commit/?h=io_uring-5.9&id=41d3344604e80db0e466f9deca5262b0914e4827
>
> There was a bug with the -EAGAIN doing repeated retries on sockets that
> are marked non-blocking.
>

no it's not working, however I received the read event after
the second request (instead of the third request before) via Telnet

--
Josef Grieb

[-- Attachment #2: io_uring_read_issue.c --]
[-- Type: text/x-c-code, Size: 2619 bytes --]

#include <errno.h>
#include <fcntl.h>
#include <netinet/in.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <strings.h>
#include <sys/socket.h>
#include <unistd.h>
#include <poll.h>
#include "liburing.h"

#define BACKLOG 512

#define PORT 9700

struct io_uring ring;

char buf[100];

void add_poll(int fd, unsigned int poll_mask) {
    struct io_uring_sqe *sqe = io_uring_get_sqe(&ring);
    io_uring_prep_poll_add(sqe, fd, poll_mask);
    sqe->user_data = 1;
    sqe->flags |= IOSQE_IO_LINK;
}

void add_accept(int fd) {
    struct io_uring_sqe *sqe = io_uring_get_sqe(&ring);
    io_uring_prep_accept(sqe, fd, 0, 0, SOCK_NONBLOCK | SOCK_CLOEXEC);
    sqe->user_data = 2;
}

void add_read(int fd) {
    struct io_uring_sqe *sqe = io_uring_get_sqe(&ring);
    io_uring_prep_read(sqe, fd, &buf, 100, 0);
    sqe->user_data = 3;
}

int setup_io_uring() {
    int ret = io_uring_queue_init(16, &ring, 0);
    if (ret) {
        fprintf(stderr, "Unable to setup io_uring: %s\n", strerror(-ret));
        return 1;
    }
    return 0;
}

int main(int argc, char *argv[]) {

    struct sockaddr_in serv_addr;

    setup_io_uring();
    
    int sock_listen_fd = socket(AF_INET, SOCK_STREAM | SOCK_NONBLOCK, 0);
    const int val = 1;
    setsockopt(sock_listen_fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));

    memset(&serv_addr, 0, sizeof(serv_addr));
    serv_addr.sin_family = AF_INET;
    serv_addr.sin_port = htons(PORT);
    serv_addr.sin_addr.s_addr = INADDR_ANY;

    if (bind(sock_listen_fd, (struct sockaddr *)&serv_addr, sizeof(serv_addr)) < 0) {
         perror("Error binding socket\n");
         exit(1);
     }
    if (listen(sock_listen_fd, BACKLOG) < 0) {
         perror("Error listening on socket\n");
         exit(1);
    }

    setup_io_uring();

    add_poll(sock_listen_fd, POLLIN);
    add_accept(sock_listen_fd);
    io_uring_submit(&ring);

    while (1) {
        struct io_uring_cqe *cqe;
        io_uring_wait_cqe(&ring, &cqe);

        printf("Res: res: %d\n", cqe->res);
        
        if (cqe->user_data == 1) {
            printf("Poll Event\n");
        }
        
        if (cqe->user_data == 2 && cqe->res > 0) {
            printf("Accept Event\n");
                    
            add_poll(sock_listen_fd, POLLIN);
            add_accept(sock_listen_fd);

            add_poll(cqe->res, POLLIN);
            add_read(cqe->res);
        }

        if (cqe->user_data == 3) {
            printf("Read Buf: %s \n", buf);
        }
        io_uring_submit(&ring);

        io_uring_cqe_seen(&ring, cqe);
    }

    io_uring_queue_exit(&ring);

    return 0;
}

Re: io_uring process termination/killing is not working

[Jens Axboe]

On 8/15/20 5:36 PM, Josef wrote:
>> Please try:
>>
>> https://git.kernel.dk/cgit/linux-block/commit/?h=io_uring-5.9&id=41d3344604e80db0e466f9deca5262b0914e4827
>>
>> There was a bug with the -EAGAIN doing repeated retries on sockets that
>> are marked non-blocking.
>>
> 
> no it's not working, however I received the read event after
> the second request (instead of the third request before) via Telnet

Are you sure your code is correct? I haven't looked too closely, but it
doesn't look very solid. There's no error checking, and you seem to be
setting up two rings (one overwriting the other). FWIW, I get the same
behavior on 5.7-stable and the above branch, except that the 5.7 hangs
on exit due to the other bug you found and that is fixed in the 5.9
branch.

I'll take a closer look later or tomorrow, but just want to make sure
I'm not spending time debugging your program :)

Hence it'd be helpful if you explain what your expectations are of
the program, and how that differs from how it behaves.

-- 
Jens Axboe

Re: io_uring process termination/killing is not working

[Jens Axboe]

On 8/15/20 5:41 PM, Jens Axboe wrote:
> On 8/15/20 5:36 PM, Josef wrote:
>>> Please try:
>>>
>>> https://git.kernel.dk/cgit/linux-block/commit/?h=io_uring-5.9&id=41d3344604e80db0e466f9deca5262b0914e4827
>>>
>>> There was a bug with the -EAGAIN doing repeated retries on sockets that
>>> are marked non-blocking.
>>>
>>
>> no it's not working, however I received the read event after
>> the second request (instead of the third request before) via Telnet
> 
> Are you sure your code is correct? I haven't looked too closely, but it
> doesn't look very solid. There's no error checking, and you seem to be
> setting up two rings (one overwriting the other). FWIW, I get the same
> behavior on 5.7-stable and the above branch, except that the 5.7 hangs
> on exit due to the other bug you found and that is fixed in the 5.9
> branch.
> 
> I'll take a closer look later or tomorrow, but just want to make sure
> I'm not spending time debugging your program :)
> 
> Hence it'd be helpful if you explain what your expectations are of
> the program, and how that differs from how it behaves.

Took a closer look, and made a few tweaks. Got rid of the extra links
and the nop, and I added a poll+read resubmit when a read completes.
Not sure how your program could work without that, if you expect it
to continue to echo out what is written on the connection? Also killed
that extra ring init.

After that, I made the following tweak to return short reads when
the the file is non-blocking. Then it seems to work as expected
for me.


diff --git a/fs/io_uring.c b/fs/io_uring.c
index cd956526c74c..dc506b75659c 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -3197,7 +3197,8 @@ static int io_read(struct io_kiocb *req, bool force_nonblock,
 	}
 
 	/* read it all, or we did blocking attempt. no retry. */
-	if (!iov_iter_count(iter) || !force_nonblock)
+	if (!iov_iter_count(iter) || !force_nonblock ||
+	    (req->file->f_flags & O_NONBLOCK))
 		goto done;
 
 	io_size -= ret;

-- 
Jens Axboe

Re: io_uring process termination/killing is not working

[Josef]

> > Hence it'd be helpful if you explain what your expectations are of
> > the program, and how that differs from how it behaves

yeah that's true, I'm sorry about that.

> > Are you sure your code is correct? I haven't looked too closely, but it
> > doesn't look very solid. There's no error checking, and you seem to be
> > setting up two rings (one overwriting the other). FWIW, I get the same
> > behavior on 5.7-stable and the above branch, except that the 5.7 hangs
> > on exit due to the other bug you found and that is fixed in the 5.9
> > branch.
> >
>
> Took a closer look, and made a few tweaks. Got rid of the extra links
> and the nop, and I added a poll+read resubmit when a read completes.
> Not sure how your program could work without that, if you expect it
> to continue to echo out what is written on the connection? Also killed
> that extra ring init.
>

sorry my bad.. I will ensure that the code is more self-explanatory
and better error checking next time. It was supposed to reproduce the
read event problem in C since I had the same issue in netty, basically
the idea was just to read the event once to keep it more simple

> After that, I made the following tweak to return short reads when
> the the file is non-blocking. Then it seems to work as expected
> for me

yeah I tested and it works in netty & my bad C example, thank you for
the super fast fix :)


--
Josef Grieb

Re: io_uring process termination/killing is not working

[Jens Axboe]

On 8/15/20 8:14 PM, Josef wrote:
>>> Hence it'd be helpful if you explain what your expectations are of
>>> the program, and how that differs from how it behaves
> 
> yeah that's true, I'm sorry about that.
> 
>>> Are you sure your code is correct? I haven't looked too closely, but it
>>> doesn't look very solid. There's no error checking, and you seem to be
>>> setting up two rings (one overwriting the other). FWIW, I get the same
>>> behavior on 5.7-stable and the above branch, except that the 5.7 hangs
>>> on exit due to the other bug you found and that is fixed in the 5.9
>>> branch.
>>>
>>
>> Took a closer look, and made a few tweaks. Got rid of the extra links
>> and the nop, and I added a poll+read resubmit when a read completes.
>> Not sure how your program could work without that, if you expect it
>> to continue to echo out what is written on the connection? Also killed
>> that extra ring init.
>>
> 
> sorry my bad.. I will ensure that the code is more self-explanatory
> and better error checking next time. It was supposed to reproduce the
> read event problem in C since I had the same issue in netty, basically
> the idea was just to read the event once to keep it more simple

No worries, it's still better to get something than nothing! And I don't
care about the error handling, but I think providing an explanation of
expectations and reality from the point of view of the reporter is a key
element in a bug report. It just makes sure that the problem description
is as clear as it can be.

>> After that, I made the following tweak to return short reads when
>> the the file is non-blocking. Then it seems to work as expected
>> for me
> 
> yeah I tested and it works in netty & my bad C example, thank you for
> the super fast fix :)

Great, thanks!

-- 
Jens Axboe

Re: io_uring process termination/killing is not working

[Jens Axboe]

On 8/15/20 9:48 AM, Pavel Begunkov wrote:
> On 15/08/2020 18:12, Jens Axboe wrote:
>> On 8/15/20 12:45 AM, Pavel Begunkov wrote:
>>> On 13/08/2020 02:32, Jens Axboe wrote:
>>>> On 8/12/20 12:28 PM, Pavel Begunkov wrote:
>>>>> On 12/08/2020 21:22, Pavel Begunkov wrote:
>>>>>> On 12/08/2020 21:20, Pavel Begunkov wrote:
>>>>>>> On 12/08/2020 21:05, Jens Axboe wrote:
>>>>>>>> On 8/12/20 11:58 AM, Josef wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> I have a weird issue on kernel 5.8.0/5.8.1, SIGINT even SIGKILL
>>>>>>>>> doesn't work to kill this process(always state D or D+), literally I
>>>>>>>>> have to terminate my VM because even the kernel can't kill the process
>>>>>>>>> and no issue on 5.7.12-201, however if IOSQE_IO_LINK is not set, it
>>>>>>>>> works
>>>>>>>>>
>>>>>>>>> I've attached a file to reproduce it
>>>>>>>>> or here
>>>>>>>>> https://gist.github.com/1Jo1/15cb3c63439d0c08e3589cfa98418b2c
>>>>>>>>
>>>>>>>> Thanks, I'll take a look at this. It's stuck in uninterruptible
>>>>>>>> state, which is why you can't kill it.
>>>>>>>
>>>>>>> It looks like one of the hangs I've been talking about a few days ago,
>>>>>>> an accept is inflight but can't be found by cancel_files() because it's
>>>>>>> in a link.
>>>>>>
>>>>>> BTW, I described it a month ago, there were more details.
>>>>>
>>>>> https://lore.kernel.org/io-uring/34eb5e5a-8d37-0cae-be6c-c6ac4d85b5d4@gmail.com
>>>>
>>>> Yeah I think you're right. How about something like the below? That'll
>>>> potentially cancel more than just the one we're looking for, but seems
>>>> kind of silly to only cancel from the file table holding request and to
>>>> the end.
>>>
>>> The bug is not poll/t-out related, IIRC my test reproduces it with
>>> read(pipe)->open(). See the previously sent link.
>>
>> Right, but in this context for poll, I just mean any request that has a
>> poll handler armed. Not necessarily only a pure poll. The patch should
>> fix your case, too.
> 
> Ok. I was thinking about sleeping in io_read(), etc. from io-wq context.
> That should have the same effect.

We already cancel any blocking work for the exiting task - but we do
that _after_ trying to cancel files, so we should probably just swap
those around in io_uring_flush(). That'll remove any need to find and
cancel those explicitly in io_uring_cancel_files().

-- 
Jens Axboe

Re: io_uring process termination/killing is not working

[Jens Axboe]

On 8/16/20 6:45 AM, Jens Axboe wrote:
> On 8/15/20 9:48 AM, Pavel Begunkov wrote:
>> On 15/08/2020 18:12, Jens Axboe wrote:
>>> On 8/15/20 12:45 AM, Pavel Begunkov wrote:
>>>> On 13/08/2020 02:32, Jens Axboe wrote:
>>>>> On 8/12/20 12:28 PM, Pavel Begunkov wrote:
>>>>>> On 12/08/2020 21:22, Pavel Begunkov wrote:
>>>>>>> On 12/08/2020 21:20, Pavel Begunkov wrote:
>>>>>>>> On 12/08/2020 21:05, Jens Axboe wrote:
>>>>>>>>> On 8/12/20 11:58 AM, Josef wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> I have a weird issue on kernel 5.8.0/5.8.1, SIGINT even SIGKILL
>>>>>>>>>> doesn't work to kill this process(always state D or D+), literally I
>>>>>>>>>> have to terminate my VM because even the kernel can't kill the process
>>>>>>>>>> and no issue on 5.7.12-201, however if IOSQE_IO_LINK is not set, it
>>>>>>>>>> works
>>>>>>>>>>
>>>>>>>>>> I've attached a file to reproduce it
>>>>>>>>>> or here
>>>>>>>>>> https://gist.github.com/1Jo1/15cb3c63439d0c08e3589cfa98418b2c
>>>>>>>>>
>>>>>>>>> Thanks, I'll take a look at this. It's stuck in uninterruptible
>>>>>>>>> state, which is why you can't kill it.
>>>>>>>>
>>>>>>>> It looks like one of the hangs I've been talking about a few days ago,
>>>>>>>> an accept is inflight but can't be found by cancel_files() because it's
>>>>>>>> in a link.
>>>>>>>
>>>>>>> BTW, I described it a month ago, there were more details.
>>>>>>
>>>>>> https://lore.kernel.org/io-uring/34eb5e5a-8d37-0cae-be6c-c6ac4d85b5d4@gmail.com
>>>>>
>>>>> Yeah I think you're right. How about something like the below? That'll
>>>>> potentially cancel more than just the one we're looking for, but seems
>>>>> kind of silly to only cancel from the file table holding request and to
>>>>> the end.
>>>>
>>>> The bug is not poll/t-out related, IIRC my test reproduces it with
>>>> read(pipe)->open(). See the previously sent link.
>>>
>>> Right, but in this context for poll, I just mean any request that has a
>>> poll handler armed. Not necessarily only a pure poll. The patch should
>>> fix your case, too.
>>
>> Ok. I was thinking about sleeping in io_read(), etc. from io-wq context.
>> That should have the same effect.
> 
> We already cancel any blocking work for the exiting task - but we do
> that _after_ trying to cancel files, so we should probably just swap
> those around in io_uring_flush(). That'll remove any need to find and
> cancel those explicitly in io_uring_cancel_files().

I guess there's still the case of the task just closing the fd, not
necessarily exiting. So I do agree with you that the io-wq case is still
unhandled. I'll take a look...

-- 
Jens Axboe

Re: io_uring process termination/killing is not working

[Jens Axboe]

On 8/16/20 7:53 AM, Jens Axboe wrote:
> On 8/16/20 6:45 AM, Jens Axboe wrote:
>> On 8/15/20 9:48 AM, Pavel Begunkov wrote:
>>> On 15/08/2020 18:12, Jens Axboe wrote:
>>>> On 8/15/20 12:45 AM, Pavel Begunkov wrote:
>>>>> On 13/08/2020 02:32, Jens Axboe wrote:
>>>>>> On 8/12/20 12:28 PM, Pavel Begunkov wrote:
>>>>>>> On 12/08/2020 21:22, Pavel Begunkov wrote:
>>>>>>>> On 12/08/2020 21:20, Pavel Begunkov wrote:
>>>>>>>>> On 12/08/2020 21:05, Jens Axboe wrote:
>>>>>>>>>> On 8/12/20 11:58 AM, Josef wrote:
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> I have a weird issue on kernel 5.8.0/5.8.1, SIGINT even SIGKILL
>>>>>>>>>>> doesn't work to kill this process(always state D or D+), literally I
>>>>>>>>>>> have to terminate my VM because even the kernel can't kill the process
>>>>>>>>>>> and no issue on 5.7.12-201, however if IOSQE_IO_LINK is not set, it
>>>>>>>>>>> works
>>>>>>>>>>>
>>>>>>>>>>> I've attached a file to reproduce it
>>>>>>>>>>> or here
>>>>>>>>>>> https://gist.github.com/1Jo1/15cb3c63439d0c08e3589cfa98418b2c
>>>>>>>>>>
>>>>>>>>>> Thanks, I'll take a look at this. It's stuck in uninterruptible
>>>>>>>>>> state, which is why you can't kill it.
>>>>>>>>>
>>>>>>>>> It looks like one of the hangs I've been talking about a few days ago,
>>>>>>>>> an accept is inflight but can't be found by cancel_files() because it's
>>>>>>>>> in a link.
>>>>>>>>
>>>>>>>> BTW, I described it a month ago, there were more details.
>>>>>>>
>>>>>>> https://lore.kernel.org/io-uring/34eb5e5a-8d37-0cae-be6c-c6ac4d85b5d4@gmail.com
>>>>>>
>>>>>> Yeah I think you're right. How about something like the below? That'll
>>>>>> potentially cancel more than just the one we're looking for, but seems
>>>>>> kind of silly to only cancel from the file table holding request and to
>>>>>> the end.
>>>>>
>>>>> The bug is not poll/t-out related, IIRC my test reproduces it with
>>>>> read(pipe)->open(). See the previously sent link.
>>>>
>>>> Right, but in this context for poll, I just mean any request that has a
>>>> poll handler armed. Not necessarily only a pure poll. The patch should
>>>> fix your case, too.
>>>
>>> Ok. I was thinking about sleeping in io_read(), etc. from io-wq context.
>>> That should have the same effect.
>>
>> We already cancel any blocking work for the exiting task - but we do
>> that _after_ trying to cancel files, so we should probably just swap
>> those around in io_uring_flush(). That'll remove any need to find and
>> cancel those explicitly in io_uring_cancel_files().
> 
> I guess there's still the case of the task just closing the fd, not
> necessarily exiting. So I do agree with you that the io-wq case is still
> unhandled. I'll take a look...

The below should do it.


diff --git a/fs/io_uring.c b/fs/io_uring.c
index dc506b75659c..346a3eb84785 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -8063,6 +8063,33 @@ static bool io_timeout_remove_link(struct io_ring_ctx *ctx,
 	return found;
 }
 
+static bool io_cancel_link_cb(struct io_wq_work *work, void *data)
+{
+	return io_match_link(container_of(work, struct io_kiocb, work), data);
+}
+
+static void io_attempt_cancel(struct io_ring_ctx *ctx, struct io_kiocb *req)
+{
+	enum io_wq_cancel cret;
+
+	/* cancel this particular work, if it's running */
+	cret = io_wq_cancel_work(ctx->io_wq, &req->work);
+	if (cret != IO_WQ_CANCEL_NOTFOUND)
+		return;
+
+	/* find links that hold this pending, cancel those */
+	cret = io_wq_cancel_cb(ctx->io_wq, io_cancel_link_cb, req, true);
+	if (cret != IO_WQ_CANCEL_NOTFOUND)
+		return;
+
+	/* if we have a poll link holding this pending, cancel that */
+	if (io_poll_remove_link(ctx, req))
+		return;
+
+	/* final option, timeout link is holding this req pending */
+	io_timeout_remove_link(ctx, req);
+}
+
 static void io_uring_cancel_files(struct io_ring_ctx *ctx,
 				  struct files_struct *files)
 {
@@ -8116,10 +8143,8 @@ static void io_uring_cancel_files(struct io_ring_ctx *ctx,
 				continue;
 			}
 		} else {
-			io_wq_cancel_work(ctx->io_wq, &cancel_req->work);
-			/* could be a link, check and remove if it is */
-			if (!io_poll_remove_link(ctx, cancel_req))
-				io_timeout_remove_link(ctx, cancel_req);
+			/* cancel this request, or head link requests */
+			io_attempt_cancel(ctx, cancel_req);
 			io_put_req(cancel_req);
 		}
 
-- 
Jens Axboe

Re: io_uring process termination/killing is not working

[Jens Axboe]

On 8/15/20 8:20 PM, Jens Axboe wrote:
> On 8/15/20 8:14 PM, Josef wrote:
>>>> Hence it'd be helpful if you explain what your expectations are of
>>>> the program, and how that differs from how it behaves
>>
>> yeah that's true, I'm sorry about that.
>>
>>>> Are you sure your code is correct? I haven't looked too closely, but it
>>>> doesn't look very solid. There's no error checking, and you seem to be
>>>> setting up two rings (one overwriting the other). FWIW, I get the same
>>>> behavior on 5.7-stable and the above branch, except that the 5.7 hangs
>>>> on exit due to the other bug you found and that is fixed in the 5.9
>>>> branch.
>>>>
>>>
>>> Took a closer look, and made a few tweaks. Got rid of the extra links
>>> and the nop, and I added a poll+read resubmit when a read completes.
>>> Not sure how your program could work without that, if you expect it
>>> to continue to echo out what is written on the connection? Also killed
>>> that extra ring init.

BTW, something I think you're aware of, but wanted to bring up
explicitly - if IORING_FEAT_FAST_POLL is available in the ring features,
then you generally don't want/need to link potentially blocking requests
on pollable files with a poll in front. io_uring will do this
internally, and save you an sqe and completion event for each of these
types of requests.

Your test case is a perfect example of that, neither the accept nor the
socket read would need a poll linked in front of them, as they are both
pollable file types and will not trigger use of an async thread to wait
for the event. Instead an internal poll is armed which will trigger the
issue of that request, when the socket is ready.

-- 
Jens Axboe

Re: io_uring process termination/killing is not working

[Josef]

> BTW, something I think you're aware of, but wanted to bring up
> explicitly - if IORING_FEAT_FAST_POLL is available in the ring features,
> then you generally don't want/need to link potentially blocking requests
> on pollable files with a poll in front. io_uring will do this
> internally, and save you an sqe and completion event for each of these
> types of requests.
>
> Your test case is a perfect example of that, neither the accept nor the
> socket read would need a poll linked in front of them, as they are both
> pollable file types and will not trigger use of an async thread to wait
> for the event. Instead an internal poll is armed which will trigger the
> issue of that request, when the socket is ready.

I agree as well I don't need a poll linked in font of read event, but
not for the non blocking accept event because of this fix in Linux
5.8:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.8&id=e697deed834de15d2322d0619d51893022c90ea2
I receive an immediate response to tell if there's ever any data
there, same behaviour like accept non blocking socket, that’s why I
use poll link to avoid that

--
Josef Grieb

Re: io_uring process termination/killing is not working

[Jens Axboe]

On 8/16/20 2:09 PM, Josef wrote:
>> BTW, something I think you're aware of, but wanted to bring up
>> explicitly - if IORING_FEAT_FAST_POLL is available in the ring features,
>> then you generally don't want/need to link potentially blocking requests
>> on pollable files with a poll in front. io_uring will do this
>> internally, and save you an sqe and completion event for each of these
>> types of requests.
>>
>> Your test case is a perfect example of that, neither the accept nor the
>> socket read would need a poll linked in front of them, as they are both
>> pollable file types and will not trigger use of an async thread to wait
>> for the event. Instead an internal poll is armed which will trigger the
>> issue of that request, when the socket is ready.
> 
> I agree as well I don't need a poll linked in font of read event, but
> not for the non blocking accept event because of this fix in Linux
> 5.8:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.8&id=e697deed834de15d2322d0619d51893022c90ea2
> I receive an immediate response to tell if there's ever any data
> there, same behaviour like accept non blocking socket, that’s why I
> use poll link to avoid that

Gotcha, yes for that case it can be useful.

-- 
Jens Axboe

Re: io_uring process termination/killing is not working

[Josef]

> BTW, something I think you're aware of, but wanted to bring up
> explicitly - if IORING_FEAT_FAST_POLL is available in the ring features,
> then you generally don't want/need to link potentially blocking requests
> on pollable files with a poll in front. io_uring will do this
> internally, and save you an sqe and completion event for each of these
> types of requests.
>
> Your test case is a perfect example of that, neither the accept nor the
> socket read would need a poll linked in front of them, as they are both
> pollable file types and will not trigger use of an async thread to wait
> for the event. Instead an internal poll is armed which will trigger the
> issue of that request, when the socket is ready

I am guessing if the socket(non blocking) is not writable as you said
it's a pollable file type, io_uring will first use an internal poll to
check if the socket is writable, right? so I don't explicitly need a
poll(POLLOUT)

https://git.kernel.dk/cgit/linux-block/commit/?h=for-5.7/io_uring&id=d7718a9d25a61442da8ee8aeeff6a0097f0ccfd6
you didn't mentioned writeable non blocking sockets, that's why I'm
asking

--
Josef Grieb

Re: io_uring process termination/killing is not working

[Pavel Begunkov]

On 17/08/2020 11:58, Josef wrote:
>> BTW, something I think you're aware of, but wanted to bring up
>> explicitly - if IORING_FEAT_FAST_POLL is available in the ring features,
>> then you generally don't want/need to link potentially blocking requests
>> on pollable files with a poll in front. io_uring will do this
>> internally, and save you an sqe and completion event for each of these
>> types of requests.
>>
>> Your test case is a perfect example of that, neither the accept nor the
>> socket read would need a poll linked in front of them, as they are both
>> pollable file types and will not trigger use of an async thread to wait
>> for the event. Instead an internal poll is armed which will trigger the
>> issue of that request, when the socket is ready
> 
> I am guessing if the socket(non blocking) is not writable as you said
> it's a pollable file type, io_uring will first use an internal poll to
> check if the socket is writable, right? so I don't explicitly need a
> poll(POLLOUT)

IIRC, it won't.

E.g. one possible scenario:
1. io_issue_sqe(force_nonblock=true), aka inlined execution
2. io_send() sets REQ_F_NOWAIT
3. __io_queue_sqe() fails such request.


BTW, Jens, it's not really clear to me, what semantics is expected
from io_uring when *NOWAIT is set (e.g. MSG_DONTWAIT)? If that's
fail-fast, then it doesn't look consistent that io_wq_submit_work()
keeps resubmitting it in a loop.

-- 
Pavel Begunkov

Re: io_uring process termination/killing is not working

[Pavel Begunkov]

On 16/08/2020 18:22, Jens Axboe wrote:
> On 8/16/20 7:53 AM, Jens Axboe wrote:
>> On 8/16/20 6:45 AM, Jens Axboe wrote:
>>> On 8/15/20 9:48 AM, Pavel Begunkov wrote:
>>>> On 15/08/2020 18:12, Jens Axboe wrote:
>>>>> On 8/15/20 12:45 AM, Pavel Begunkov wrote:
>>>>>> On 13/08/2020 02:32, Jens Axboe wrote:
>>>>>>> On 8/12/20 12:28 PM, Pavel Begunkov wrote:
>>>>>>>> On 12/08/2020 21:22, Pavel Begunkov wrote:
>>>>>>>>> On 12/08/2020 21:20, Pavel Begunkov wrote:
>>>>>>>>>> On 12/08/2020 21:05, Jens Axboe wrote:
>>>>>>>>>>> On 8/12/20 11:58 AM, Josef wrote:
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> I have a weird issue on kernel 5.8.0/5.8.1, SIGINT even SIGKILL
>>>>>>>>>>>> doesn't work to kill this process(always state D or D+), literally I
>>>>>>>>>>>> have to terminate my VM because even the kernel can't kill the process
>>>>>>>>>>>> and no issue on 5.7.12-201, however if IOSQE_IO_LINK is not set, it
>>>>>>>>>>>> works
>>>>>>>>>>>>
>>>>>>>>>>>> I've attached a file to reproduce it
>>>>>>>>>>>> or here
>>>>>>>>>>>> https://gist.github.com/1Jo1/15cb3c63439d0c08e3589cfa98418b2c
>>>>>>>>>>>
>>>>>>>>>>> Thanks, I'll take a look at this. It's stuck in uninterruptible
>>>>>>>>>>> state, which is why you can't kill it.
>>>>>>>>>>
>>>>>>>>>> It looks like one of the hangs I've been talking about a few days ago,
>>>>>>>>>> an accept is inflight but can't be found by cancel_files() because it's
>>>>>>>>>> in a link.
>>>>>>>>>
>>>>>>>>> BTW, I described it a month ago, there were more details.
>>>>>>>>
>>>>>>>> https://lore.kernel.org/io-uring/34eb5e5a-8d37-0cae-be6c-c6ac4d85b5d4@gmail.com
>>>>>>>
>>>>>>> Yeah I think you're right. How about something like the below? That'll
>>>>>>> potentially cancel more than just the one we're looking for, but seems
>>>>>>> kind of silly to only cancel from the file table holding request and to
>>>>>>> the end.
>>>>>>
>>>>>> The bug is not poll/t-out related, IIRC my test reproduces it with
>>>>>> read(pipe)->open(). See the previously sent link.
>>>>>
>>>>> Right, but in this context for poll, I just mean any request that has a
>>>>> poll handler armed. Not necessarily only a pure poll. The patch should
>>>>> fix your case, too.
>>>>
>>>> Ok. I was thinking about sleeping in io_read(), etc. from io-wq context.
>>>> That should have the same effect.
>>>
>>> We already cancel any blocking work for the exiting task - but we do
>>> that _after_ trying to cancel files, so we should probably just swap
>>> those around in io_uring_flush(). That'll remove any need to find and
>>> cancel those explicitly in io_uring_cancel_files().
>>
>> I guess there's still the case of the task just closing the fd, not
>> necessarily exiting. So I do agree with you that the io-wq case is still
>> unhandled. I'll take a look...

Right. It should be in the cancel code itself.
Thanks for dealing with this. It seems, you had a busy week with all
these syzkaller reports.


> 
> The below should do it.
> 
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index dc506b75659c..346a3eb84785 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -8063,6 +8063,33 @@ static bool io_timeout_remove_link(struct io_ring_ctx *ctx,
>  	return found;
>  }
>  
> +static bool io_cancel_link_cb(struct io_wq_work *work, void *data)
> +{
> +	return io_match_link(container_of(work, struct io_kiocb, work), data);
> +}
> +
> +static void io_attempt_cancel(struct io_ring_ctx *ctx, struct io_kiocb *req)
> +{
> +	enum io_wq_cancel cret;
> +
> +	/* cancel this particular work, if it's running */
> +	cret = io_wq_cancel_work(ctx->io_wq, &req->work);
> +	if (cret != IO_WQ_CANCEL_NOTFOUND)
> +		return;
> +
> +	/* find links that hold this pending, cancel those */
> +	cret = io_wq_cancel_cb(ctx->io_wq, io_cancel_link_cb, req, true);
> +	if (cret != IO_WQ_CANCEL_NOTFOUND)
> +		return;
> +
> +	/* if we have a poll link holding this pending, cancel that */
> +	if (io_poll_remove_link(ctx, req))
> +		return;
> +
> +	/* final option, timeout link is holding this req pending */
> +	io_timeout_remove_link(ctx, req);
> +}
> +
>  static void io_uring_cancel_files(struct io_ring_ctx *ctx,
>  				  struct files_struct *files)
>  {
> @@ -8116,10 +8143,8 @@ static void io_uring_cancel_files(struct io_ring_ctx *ctx,
>  				continue;
>  			}
>  		} else {
> -			io_wq_cancel_work(ctx->io_wq, &cancel_req->work);
> -			/* could be a link, check and remove if it is */
> -			if (!io_poll_remove_link(ctx, cancel_req))
> -				io_timeout_remove_link(ctx, cancel_req);
> +			/* cancel this request, or head link requests */
> +			io_attempt_cancel(ctx, cancel_req);
>  			io_put_req(cancel_req);
>  		}
>  
> 

-- 
Pavel Begunkov