* [PATCHSET 0/2] Fix async path resolution
@ 2020-11-13 23:51 Jens Axboe
2020-11-13 23:51 ` [PATCH 1/2] proc: don't allow async path resolution of /proc/self components Jens Axboe
2020-11-13 23:51 ` [PATCH 2/2] io_uring: handle -EOPNOTSUPP on path resolution Jens Axboe
0 siblings, 2 replies; 4+ messages in thread
From: Jens Axboe @ 2020-11-13 23:51 UTC (permalink / raw)
To: io-uring
Hi,
Looking up anything in /proc/self from the io-wq worker doesn't always
yield the right result. We can't easily add the necessary context, so
until we can, forbid the lookup and have the task do it itself.
--
Jens Axboe
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] proc: don't allow async path resolution of /proc/self components
2020-11-13 23:51 [PATCHSET 0/2] Fix async path resolution Jens Axboe
@ 2020-11-13 23:51 ` Jens Axboe
2020-11-13 23:51 ` [PATCH 2/2] io_uring: handle -EOPNOTSUPP on path resolution Jens Axboe
1 sibling, 0 replies; 4+ messages in thread
From: Jens Axboe @ 2020-11-13 23:51 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
If this is attempted by a kthread, then return -EOPNOTSUPP as we don't
currently support that. Once we can get task_pid_ptr() doing the right
thing, then this can go away again.
Signed-off-by: Jens Axboe <[email protected]>
---
fs/proc/self.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/fs/proc/self.c b/fs/proc/self.c
index 72cd69bcaf4a..cc71ce3466dc 100644
--- a/fs/proc/self.c
+++ b/fs/proc/self.c
@@ -16,6 +16,13 @@ static const char *proc_self_get_link(struct dentry *dentry,
pid_t tgid = task_tgid_nr_ns(current, ns);
char *name;
+ /*
+ * Not currently supported. Once we can inherit all of struct pid,
+ * we can allow this.
+ */
+ if (current->flags & PF_KTHREAD)
+ return ERR_PTR(-EOPNOTSUPP);
+
if (!tgid)
return ERR_PTR(-ENOENT);
/* max length of unsigned int in decimal + NULL term */
--
2.29.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] io_uring: handle -EOPNOTSUPP on path resolution
2020-11-13 23:51 [PATCHSET 0/2] Fix async path resolution Jens Axboe
2020-11-13 23:51 ` [PATCH 1/2] proc: don't allow async path resolution of /proc/self components Jens Axboe
@ 2020-11-13 23:51 ` Jens Axboe
2020-11-14 17:26 ` [PATCH v2 " Jens Axboe
1 sibling, 1 reply; 4+ messages in thread
From: Jens Axboe @ 2020-11-13 23:51 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
Any attempt to do path resolution on /proc/self from an async worker will
yield -EOPNOTSUPP. We can safely do that resolution from the task itself,
so retry it from there.
Ideally io_uring would know this upfront and not have to go through the
worker thread to find out, but that doesn't currently seem feasible.
Signed-off-by: Jens Axboe <[email protected]>
---
fs/io_uring.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index c77584de68d7..90d2f67f0ecd 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -586,6 +586,7 @@ enum {
REQ_F_NO_FILE_TABLE_BIT,
REQ_F_WORK_INITIALIZED_BIT,
REQ_F_LTIMEOUT_ACTIVE_BIT,
+ REQ_F_BLOCK_BIT,
/* not a real bit, just to check we're not overflowing the space */
__REQ_F_LAST_BIT,
@@ -631,6 +632,8 @@ enum {
REQ_F_WORK_INITIALIZED = BIT(REQ_F_WORK_INITIALIZED_BIT),
/* linked timeout is active, i.e. prepared by link's head */
REQ_F_LTIMEOUT_ACTIVE = BIT(REQ_F_LTIMEOUT_ACTIVE_BIT),
+ /* OK to do pass in force_nonblock == false for task */
+ REQ_F_BLOCK = BIT(REQ_F_BLOCK_BIT),
};
struct async_poll {
@@ -3854,6 +3857,21 @@ static int io_openat2(struct io_kiocb *req, bool force_nonblock)
if (IS_ERR(file)) {
put_unused_fd(ret);
ret = PTR_ERR(file);
+ /*
+ * A work-around to ensure that /proc/self works that way
+ * that it should - if we get -EOPNOTSUPP back, then assume
+ * that proc_self_get_link() failed us because we're in async
+ * context. We should be safe to retry this from the task
+ * itself with force_nonblock == false set, as it should not
+ * block on lookup. Would be nice to know this upfront and
+ * avoid the async dance, but doesn't seem feasible.
+ */
+ if (ret == -EOPNOTSUPP && io_wq_current_is_worker()) {
+ req->flags |= REQ_F_BLOCK;
+ refcount_inc(&req->refs);
+ io_req_task_queue(req);
+ return 0;
+ }
} else {
fsnotify_open(file);
fd_install(ret, file);
@@ -6198,6 +6216,7 @@ static void __io_queue_sqe(struct io_kiocb *req, struct io_comp_state *cs)
{
struct io_kiocb *linked_timeout;
const struct cred *old_creds = NULL;
+ bool force_nonblock = true;
int ret;
again:
@@ -6214,7 +6233,10 @@ static void __io_queue_sqe(struct io_kiocb *req, struct io_comp_state *cs)
old_creds = override_creds(req->work.identity->creds);
}
- ret = io_issue_sqe(req, true, cs);
+ if (req->flags & REQ_F_BLOCK)
+ force_nonblock = false;
+
+ ret = io_issue_sqe(req, force_nonblock, cs);
/*
* We async punt it if the file wasn't marked NOWAIT, or if the file
--
2.29.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] io_uring: handle -EOPNOTSUPP on path resolution
2020-11-13 23:51 ` [PATCH 2/2] io_uring: handle -EOPNOTSUPP on path resolution Jens Axboe
@ 2020-11-14 17:26 ` Jens Axboe
0 siblings, 0 replies; 4+ messages in thread
From: Jens Axboe @ 2020-11-14 17:26 UTC (permalink / raw)
To: io-uring
Any attempt to do path resolution on /proc/self from an async worker will
yield -EOPNOTSUPP. We can safely do that resolution from the task itself,
and without blocking, so retry it from there.
Ideally io_uring would know this upfront and not have to go through the
worker thread to find out, but that doesn't currently seem feasible.
Signed-off-by: Jens Axboe <[email protected]>
---
v2:
- Handle this internally to the open command, so we don't
need to touch the fast/common path.
diff --git a/fs/io_uring.c b/fs/io_uring.c
index c77584de68d7..f05978a74ce1 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -478,6 +478,7 @@ struct io_sr_msg {
struct io_open {
struct file *file;
int dfd;
+ bool ignore_nonblock;
struct filename *filename;
struct open_how how;
unsigned long nofile;
@@ -3796,6 +3797,7 @@ static int __io_openat_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe
return ret;
}
req->open.nofile = rlimit(RLIMIT_NOFILE);
+ req->open.ignore_nonblock = false;
req->flags |= REQ_F_NEED_CLEANUP;
return 0;
}
@@ -3839,7 +3841,7 @@ static int io_openat2(struct io_kiocb *req, bool force_nonblock)
struct file *file;
int ret;
- if (force_nonblock)
+ if (force_nonblock && !req->open.ignore_nonblock)
return -EAGAIN;
ret = build_open_flags(&req->open.how, &op);
@@ -3854,6 +3856,21 @@ static int io_openat2(struct io_kiocb *req, bool force_nonblock)
if (IS_ERR(file)) {
put_unused_fd(ret);
ret = PTR_ERR(file);
+ /*
+ * A work-around to ensure that /proc/self works that way
+ * that it should - if we get -EOPNOTSUPP back, then assume
+ * that proc_self_get_link() failed us because we're in async
+ * context. We should be safe to retry this from the task
+ * itself with force_nonblock == false set, as it should not
+ * block on lookup. Would be nice to know this upfront and
+ * avoid the async dance, but doesn't seem feasible.
+ */
+ if (ret == -EOPNOTSUPP && io_wq_current_is_worker()) {
+ req->open.ignore_nonblock = true;
+ refcount_inc(&req->refs);
+ io_req_task_queue(req);
+ return 0;
+ }
} else {
fsnotify_open(file);
fd_install(ret, file);
--
Jens Axboe
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-11-14 17:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-13 23:51 [PATCHSET 0/2] Fix async path resolution Jens Axboe
2020-11-13 23:51 ` [PATCH 1/2] proc: don't allow async path resolution of /proc/self components Jens Axboe
2020-11-13 23:51 ` [PATCH 2/2] io_uring: handle -EOPNOTSUPP on path resolution Jens Axboe
2020-11-14 17:26 ` [PATCH v2 " Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox