Re: [PATCH v5 1/7] mm: pass task and mm to do_madvise
From: Jann Horn <jannh@google.com>
Date: 2020-02-14 17:26:03
Also in:
io-uring, linux-mm, lkml
+Jens and io-uring list On Fri, Feb 14, 2020 at 6:06 PM Minchan Kim [off-list ref] wrote:
In upcoming patches, do_madvise will be called from external process context so we shouldn't asssume "current" is always hinted process's task_struct.
[...]
[1] http://lore.kernel.org/r/CAG48ez27=pwm5m_N_988xT1huO7g7h6arTQL44zev6TD-h-7Tg@mail.gmail.com (local)
[...]
quoted hunk ↗ jump to hunk
diff --git a/fs/io_uring.c b/fs/io_uring.c
[...]
quoted hunk ↗ jump to hunk
@@ -2736,7 +2736,7 @@ static int io_madvise(struct io_kiocb *req, struct io_kiocb **nxt, if (force_nonblock) return -EAGAIN; - ret = do_madvise(ma->addr, ma->len, ma->advice); + ret = do_madvise(current, current->mm, ma->addr, ma->len, ma->advice); if (ret < 0) req_set_fail_links(req); io_cqring_add_event(req, ret);
Jens, can you have a look at this change and the following patch <https://lore.kernel.org/linux-mm/20200214170520.160271-4-minchan@kernel.org/ (local)> ("[PATCH v5 3/7] mm: check fatal signal pending of target process")? Basically Minchan's patch tries to plumb through the identity of the target task so that if that task gets killed in the middle of the operation, the (potentially long-running and costly) madvise operation can be cancelled. Just passing in "current" instead (which in this case is the uring worker thread AFAIK) doesn't really break anything, other than making the optimization not work, but I wonder whether this couldn't be done more cleanly - maybe by passing in NULL to mean "we don't know who the target task is", since I think we don't know that here?