Re: "Cannot allocate memory" on ring creation (not RLIMIT_MEMLOCK)

From: Pavel Begunkov <asml.silence@gmail.com>
To: Jens Axboe <axboe@kernel.dk>, Josef <josef.grieb@gmail.com>
Cc: Norman Maurer <norman.maurer@googlemail.com>,
	Dmitry Kadashev <dkadashev@gmail.com>,
	io-uring <io-uring@vger.kernel.org>
Subject: Re: "Cannot allocate memory" on ring creation (not RLIMIT_MEMLOCK)
Date: Sat, 19 Dec 2020 23:42:16 +0000	[thread overview]
Message-ID: <986c85af-bb77-60d4-8739-49b662554157@gmail.com> (raw)
In-Reply-To: <614f8422-3e0e-25b9-4cc2-4f1c07705ab0@kernel.dk>

On 19/12/2020 23:13, Jens Axboe wrote:
> On 12/19/20 2:54 PM, Jens Axboe wrote:
>> On 12/19/20 1:51 PM, Josef wrote:
>>>> And even more so, it's IOSQE_ASYNC on the IORING_OP_READ on an eventfd
>>>> file descriptor. You probably don't want/mean to do that as it's
>>>> pollable, I guess it's done because you just set it on all reads for the
>>>> test?
>>>
>>> yes exactly, eventfd fd is blocking, so it actually makes no sense to
>>> use IOSQE_ASYNC
>>
>> Right, and it's pollable too.
>>
>>> I just tested eventfd without the IOSQE_ASYNC flag, it seems to work
>>> in my tests, thanks a lot :)
>>>
>>>> In any case, it should of course work. This is the leftover trace when
>>>> we should be exiting, but an io-wq worker is still trying to get data
>>>> from the eventfd:
>>>
>>> interesting, btw what kind of tool do you use for kernel debugging?
>>
>> Just poking at it and thinking about it, no hidden magic I'm afraid...
> 
> Josef, can you try with this added? Looks bigger than it is, most of it
> is just moving one function below another.

Hmm, which kernel revision are you poking? Seems it doesn't match
io_uring-5.10, and for 5.11 io_uring_cancel_files() is never called with
NULL files.

if (!files)
	__io_uring_cancel_task_requests(ctx, task);
else
	io_uring_cancel_files(ctx, task, files);

> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index f3690dfdd564..96f6445ab827 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -8735,10 +8735,43 @@ static void io_cancel_defer_files(struct io_ring_ctx *ctx,
[...]
>  static void io_uring_cancel_files(struct io_ring_ctx *ctx,
>  				  struct task_struct *task,
>  				  struct files_struct *files)
>  {
> +	/* files == NULL, task is exiting. Cancel all that match task */
> +	if (!files)
> +		__io_uring_cancel_task_requests(ctx, task);
> +

For 5.11 I believe it should look like

diff --git a/fs/io_uring.c b/fs/io_uring.c
index f3690dfdd564..38fb351cc1dd 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -8822,9 +8822,8 @@ static void io_uring_cancel_task_requests(struct io_ring_ctx *ctx,
 	io_cqring_overflow_flush(ctx, true, task, files);
 	io_ring_submit_unlock(ctx, (ctx->flags & IORING_SETUP_IOPOLL));
 
-	if (!files)
-		__io_uring_cancel_task_requests(ctx, task);
-	else
+	__io_uring_cancel_task_requests(ctx, task);
+	if (files)
 		io_uring_cancel_files(ctx, task, files);
 
 	if ((ctx->flags & IORING_SETUP_SQPOLL) && ctx->sq_data) {


-- 
Pavel Begunkov
