* [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