* sparse fixes for io_uring
@ 2022-05-18 8:39 Christoph Hellwig
2022-05-18 8:40 ` [PATCH 1/6] io_uring: use a rwf_t for io_rw.flags Christoph Hellwig
` (5 more replies)
0 siblings, 6 replies; 8+ messages in thread
From: Christoph Hellwig @ 2022-05-18 8:39 UTC (permalink / raw)
To: axboe; +Cc: asml.silence, io-uring
Try to make sparse a little more happy. The poll code is still a major
trainwreck, but that will need more intensive care.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/6] io_uring: use a rwf_t for io_rw.flags
2022-05-18 8:39 sparse fixes for io_uring Christoph Hellwig
@ 2022-05-18 8:40 ` Christoph Hellwig
2022-05-18 12:27 ` Jens Axboe
2022-05-18 8:40 ` [PATCH 2/6] io_uring: don't use ERR_PTR for user pointers Christoph Hellwig
` (4 subsequent siblings)
5 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2022-05-18 8:40 UTC (permalink / raw)
To: axboe; +Cc: asml.silence, io-uring
Remove the bogus __force casts and just use the proper type instead.
Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/io_uring.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 0b05d5aa5891a..99862cbc1041c 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -644,7 +644,7 @@ struct io_rw {
struct kiocb kiocb;
u64 addr;
u32 len;
- u32 flags;
+ rwf_t flags;
};
struct io_connect {
@@ -3636,7 +3636,7 @@ static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe)
req->imu = NULL;
req->rw.addr = READ_ONCE(sqe->addr);
req->rw.len = READ_ONCE(sqe->len);
- req->rw.flags = (__force u32)READ_ONCE(sqe->rw_flags);
+ req->rw.flags = READ_ONCE(sqe->rw_flags);
/* used for fixed read/write too - just read unconditionally */
req->buf_index = READ_ONCE(sqe->buf_index);
return 0;
@@ -4274,7 +4274,7 @@ static int io_rw_init_file(struct io_kiocb *req, fmode_t mode)
req->flags |= io_file_get_flags(file) << REQ_F_SUPPORT_NOWAIT_BIT;
kiocb->ki_flags = iocb_flags(file);
- ret = kiocb_set_rw_flags(kiocb, (__force rwf_t)req->rw.flags);
+ ret = kiocb_set_rw_flags(kiocb, req->rw.flags);
if (unlikely(ret))
return ret;
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/6] io_uring: don't use ERR_PTR for user pointers
2022-05-18 8:39 sparse fixes for io_uring Christoph Hellwig
2022-05-18 8:40 ` [PATCH 1/6] io_uring: use a rwf_t for io_rw.flags Christoph Hellwig
@ 2022-05-18 8:40 ` Christoph Hellwig
2022-05-18 8:40 ` [PATCH 3/6] io_uring: drop a spurious inline on a forward declaration Christoph Hellwig
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2022-05-18 8:40 UTC (permalink / raw)
To: axboe; +Cc: asml.silence, io-uring
ERR_PTR abuses the high bits of a pointer to transport error information.
This is only safe for kernel pointers and not user pointers. Fix
io_buffer_select and its helpers to just return NULL for failure and get
rid of this abuse.
Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/io_uring.c | 83 +++++++++++++++++++++++----------------------------
1 file changed, 37 insertions(+), 46 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 99862cbc1041c..abb7108258f96 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -3797,11 +3797,8 @@ static int io_buffer_add_list(struct io_ring_ctx *ctx,
}
static void __user *io_provided_buffer_select(struct io_kiocb *req, size_t *len,
- struct io_buffer_list *bl,
- unsigned int issue_flags)
+ struct io_buffer_list *bl)
{
- void __user *ret = ERR_PTR(-ENOBUFS);
-
if (!list_empty(&bl->buf_list)) {
struct io_buffer *kbuf;
@@ -3812,11 +3809,9 @@ static void __user *io_provided_buffer_select(struct io_kiocb *req, size_t *len,
req->flags |= REQ_F_BUFFER_SELECTED;
req->kbuf = kbuf;
req->buf_index = kbuf->bid;
- ret = u64_to_user_ptr(kbuf->addr);
+ return u64_to_user_ptr(kbuf->addr);
}
-
- io_ring_submit_unlock(req->ctx, issue_flags);
- return ret;
+ return NULL;
}
static void __user *io_ring_buffer_select(struct io_kiocb *req, size_t *len,
@@ -3829,7 +3824,7 @@ static void __user *io_ring_buffer_select(struct io_kiocb *req, size_t *len,
if (unlikely(smp_load_acquire(&br->tail) == head)) {
io_ring_submit_unlock(req->ctx, issue_flags);
- return ERR_PTR(-ENOBUFS);
+ return NULL;
}
head &= bl->mask;
@@ -3847,22 +3842,19 @@ static void __user *io_ring_buffer_select(struct io_kiocb *req, size_t *len,
req->buf_list = bl;
req->buf_index = buf->bid;
- if (!(issue_flags & IO_URING_F_UNLOCKED))
- return u64_to_user_ptr(buf->addr);
-
- /*
- * If we came in unlocked, we have no choice but to
- * consume the buffer here. This does mean it'll be
- * pinned until the IO completes. But coming in
- * unlocked means we're in io-wq context, hence there
- * should be no further retry. For the locked case, the
- * caller must ensure to call the commit when the
- * transfer completes (or if we get -EAGAIN and must
- * poll or retry).
- */
- req->buf_list = NULL;
- bl->head++;
- io_ring_submit_unlock(req->ctx, issue_flags);
+ if (issue_flags & IO_URING_F_UNLOCKED) {
+ /*
+ * If we came in unlocked, we have no choice but to consume the
+ * buffer here. This does mean it'll be pinned until the IO
+ * completes. But coming in unlocked means we're in io-wq
+ * context, hence there should be no further retry. For the
+ * locked case, the caller must ensure to call the commit when
+ * the transfer completes (or if we get -EAGAIN and must poll
+ * or retry).
+ */
+ req->buf_list = NULL;
+ bl->head++;
+ }
return u64_to_user_ptr(buf->addr);
}
@@ -3871,20 +3863,19 @@ static void __user *io_buffer_select(struct io_kiocb *req, size_t *len,
{
struct io_ring_ctx *ctx = req->ctx;
struct io_buffer_list *bl;
+ void __user *ret = NULL;
io_ring_submit_lock(req->ctx, issue_flags);
bl = io_buffer_get_list(ctx, req->buf_index);
- if (unlikely(!bl)) {
- io_ring_submit_unlock(req->ctx, issue_flags);
- return ERR_PTR(-ENOBUFS);
+ if (likely(bl)) {
+ if (bl->buf_nr_pages)
+ ret = io_ring_buffer_select(req, len, bl, issue_flags);
+ else
+ ret = io_provided_buffer_select(req, len, bl);
}
-
- /* selection helpers drop the submit lock again, if needed */
- if (bl->buf_nr_pages)
- return io_ring_buffer_select(req, len, bl, issue_flags);
-
- return io_provided_buffer_select(req, len, bl, issue_flags);
+ io_ring_submit_unlock(req->ctx, issue_flags);
+ return ret;
}
#ifdef CONFIG_COMPAT
@@ -3906,8 +3897,8 @@ static ssize_t io_compat_import(struct io_kiocb *req, struct iovec *iov,
len = clen;
buf = io_buffer_select(req, &len, issue_flags);
- if (IS_ERR(buf))
- return PTR_ERR(buf);
+ if (!buf)
+ return -ENOBUFS;
req->rw.addr = (unsigned long) buf;
iov[0].iov_base = buf;
req->rw.len = iov[0].iov_len = (compat_size_t) len;
@@ -3929,8 +3920,8 @@ static ssize_t __io_iov_buffer_select(struct io_kiocb *req, struct iovec *iov,
if (len < 0)
return -EINVAL;
buf = io_buffer_select(req, &len, issue_flags);
- if (IS_ERR(buf))
- return PTR_ERR(buf);
+ if (!buf)
+ return -ENOBUFS;
req->rw.addr = (unsigned long) buf;
iov[0].iov_base = buf;
req->rw.len = iov[0].iov_len = len;
@@ -3987,8 +3978,8 @@ static struct iovec *__io_import_iovec(int rw, struct io_kiocb *req,
if (opcode == IORING_OP_READ || opcode == IORING_OP_WRITE) {
if (io_do_buffer_select(req)) {
buf = io_buffer_select(req, &sqe_len, issue_flags);
- if (IS_ERR(buf))
- return ERR_CAST(buf);
+ if (!buf)
+ return ERR_PTR(-ENOBUFS);
req->rw.addr = (unsigned long) buf;
req->rw.len = sqe_len;
}
@@ -5259,8 +5250,8 @@ static int io_nop(struct io_kiocb *req, unsigned int issue_flags)
size_t len = 1;
buf = io_buffer_select(req, &len, issue_flags);
- if (IS_ERR(buf))
- return PTR_ERR(buf);
+ if (!buf)
+ return -ENOBUFS;
}
if (!(req->ctx->flags & IORING_SETUP_CQE32))
@@ -6394,8 +6385,8 @@ static int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags)
void __user *buf;
buf = io_buffer_select(req, &sr->len, issue_flags);
- if (IS_ERR(buf))
- return PTR_ERR(buf);
+ if (!buf)
+ return -ENOBUFS;
kmsg->fast_iov[0].iov_base = buf;
kmsg->fast_iov[0].iov_len = sr->len;
iov_iter_init(&kmsg->msg.msg_iter, READ, kmsg->fast_iov, 1,
@@ -6464,8 +6455,8 @@ static int io_recv(struct io_kiocb *req, unsigned int issue_flags)
void __user *buf;
buf = io_buffer_select(req, &sr->len, issue_flags);
- if (IS_ERR(buf))
- return PTR_ERR(buf);
+ if (!buf)
+ return -ENOBUFS;
sr->buf = buf;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/6] io_uring: drop a spurious inline on a forward declaration
2022-05-18 8:39 sparse fixes for io_uring Christoph Hellwig
2022-05-18 8:40 ` [PATCH 1/6] io_uring: use a rwf_t for io_rw.flags Christoph Hellwig
2022-05-18 8:40 ` [PATCH 2/6] io_uring: don't use ERR_PTR for user pointers Christoph Hellwig
@ 2022-05-18 8:40 ` Christoph Hellwig
2022-05-18 8:40 ` [PATCH 4/6] io_uring: make apoll_events a __poll_t Christoph Hellwig
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2022-05-18 8:40 UTC (permalink / raw)
To: axboe; +Cc: asml.silence, io-uring
io_file_get_normal isn't marked inline, so don't claim it as such in the
forward declaration.
Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/io_uring.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index abb7108258f96..fc435f95ef340 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1354,7 +1354,7 @@ static int __io_register_rsrc_update(struct io_ring_ctx *ctx, unsigned type,
static void io_clean_op(struct io_kiocb *req);
static inline struct file *io_file_get_fixed(struct io_kiocb *req, int fd,
unsigned issue_flags);
-static inline struct file *io_file_get_normal(struct io_kiocb *req, int fd);
+static struct file *io_file_get_normal(struct io_kiocb *req, int fd);
static void io_drop_inflight_file(struct io_kiocb *req);
static bool io_assign_file(struct io_kiocb *req, unsigned int issue_flags);
static void io_queue_sqe(struct io_kiocb *req);
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/6] io_uring: make apoll_events a __poll_t
2022-05-18 8:39 sparse fixes for io_uring Christoph Hellwig
` (2 preceding siblings ...)
2022-05-18 8:40 ` [PATCH 3/6] io_uring: drop a spurious inline on a forward declaration Christoph Hellwig
@ 2022-05-18 8:40 ` Christoph Hellwig
2022-05-18 8:40 ` [PATCH 5/6] io_uring: consistently use the EPOLL* defines Christoph Hellwig
2022-05-18 8:40 ` [PATCH 6/6] io_uring: use rcu_dereference in io_close Christoph Hellwig
5 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2022-05-18 8:40 UTC (permalink / raw)
To: axboe; +Cc: asml.silence, io-uring
apoll_events is fed to vfs_poll and the poll tables, so it should be
a __poll_t.
Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/io_uring.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index fc435f95ef340..1b46c3e9df33a 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1031,7 +1031,7 @@ struct io_kiocb {
/* used by request caches, completion batching and iopoll */
struct io_wq_work_node comp_list;
/* cache ->apoll->events */
- int apoll_events;
+ __poll_t apoll_events;
};
atomic_t refs;
atomic_t poll_refs;
@@ -6977,7 +6977,7 @@ static void io_apoll_task_func(struct io_kiocb *req, bool *locked)
io_req_complete_failed(req, ret);
}
-static void __io_poll_execute(struct io_kiocb *req, int mask, int events)
+static void __io_poll_execute(struct io_kiocb *req, int mask, __poll_t events)
{
req->cqe.res = mask;
/*
@@ -6996,7 +6996,8 @@ static void __io_poll_execute(struct io_kiocb *req, int mask, int events)
io_req_task_work_add(req, false);
}
-static inline void io_poll_execute(struct io_kiocb *req, int res, int events)
+static inline void io_poll_execute(struct io_kiocb *req, int res,
+ __poll_t events)
{
if (io_poll_get_ownership(req))
__io_poll_execute(req, res, events);
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 5/6] io_uring: consistently use the EPOLL* defines
2022-05-18 8:39 sparse fixes for io_uring Christoph Hellwig
` (3 preceding siblings ...)
2022-05-18 8:40 ` [PATCH 4/6] io_uring: make apoll_events a __poll_t Christoph Hellwig
@ 2022-05-18 8:40 ` Christoph Hellwig
2022-05-18 8:40 ` [PATCH 6/6] io_uring: use rcu_dereference in io_close Christoph Hellwig
5 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2022-05-18 8:40 UTC (permalink / raw)
To: axboe; +Cc: asml.silence, io-uring
POLL* are unannotated values for the userspace ABI, while everything
in-kernel should use EPOLL* and the __poll_t type.
Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/io_uring.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 1b46c3e9df33a..c9596d551bd67 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -7012,7 +7012,7 @@ static void io_poll_cancel_req(struct io_kiocb *req)
#define wqe_to_req(wait) ((void *)((unsigned long) (wait)->private & ~1))
#define wqe_is_double(wait) ((unsigned long) (wait)->private & 1)
-#define IO_ASYNC_POLL_COMMON (EPOLLONESHOT | POLLPRI)
+#define IO_ASYNC_POLL_COMMON (EPOLLONESHOT | EPOLLPRI)
static int io_poll_wake(struct wait_queue_entry *wait, unsigned mode, int sync,
void *key)
@@ -7217,14 +7217,14 @@ static int io_arm_poll_handler(struct io_kiocb *req, unsigned issue_flags)
mask |= EPOLLONESHOT;
if (def->pollin) {
- mask |= POLLIN | POLLRDNORM;
+ mask |= EPOLLIN | EPOLLRDNORM;
/* If reading from MSG_ERRQUEUE using recvmsg, ignore POLLIN */
if ((req->opcode == IORING_OP_RECVMSG) &&
(req->sr_msg.msg_flags & MSG_ERRQUEUE))
- mask &= ~POLLIN;
+ mask &= ~EPOLLIN;
} else {
- mask |= POLLOUT | POLLWRNORM;
+ mask |= EPOLLOUT | EPOLLWRNORM;
}
if (def->poll_exclusive)
mask |= EPOLLEXCLUSIVE;
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 6/6] io_uring: use rcu_dereference in io_close
2022-05-18 8:39 sparse fixes for io_uring Christoph Hellwig
` (4 preceding siblings ...)
2022-05-18 8:40 ` [PATCH 5/6] io_uring: consistently use the EPOLL* defines Christoph Hellwig
@ 2022-05-18 8:40 ` Christoph Hellwig
5 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2022-05-18 8:40 UTC (permalink / raw)
To: axboe; +Cc: asml.silence, io-uring
Accessing the file table needs a rcu_dereference_protected().
Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/io_uring.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index c9596d551bd67..2b848a8dfc46c 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5984,7 +5984,8 @@ static int io_close(struct io_kiocb *req, unsigned int issue_flags)
spin_unlock(&files->file_lock);
goto err;
}
- file = fdt->fd[close->fd];
+ file = rcu_dereference_protected(fdt->fd[close->fd],
+ lockdep_is_held(&files->file_lock));
if (!file || file->f_op == &io_uring_fops) {
spin_unlock(&files->file_lock);
file = NULL;
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/6] io_uring: use a rwf_t for io_rw.flags
2022-05-18 8:40 ` [PATCH 1/6] io_uring: use a rwf_t for io_rw.flags Christoph Hellwig
@ 2022-05-18 12:27 ` Jens Axboe
0 siblings, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2022-05-18 12:27 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: io-uring, asml.silence
On Wed, 18 May 2022 10:40:00 +0200, Christoph Hellwig wrote:
> Remove the bogus __force casts and just use the proper type instead.
>
>
Applied, thanks!
[1/6] io_uring: use a rwf_t for io_rw.flags
commit: 20cbd21d899b72765e38481a926c7b2008c64350
[2/6] io_uring: don't use ERR_PTR for user pointers
commit: 984824db844a9bd6e9e15ee469241982526a6ccd
[3/6] io_uring: drop a spurious inline on a forward declaration
commit: ee67ba3b20f7dcd001b7743eb8e46880cb27fdc6
[4/6] io_uring: make apoll_events a __poll_t
commit: 58f5c8d39e0ea07fdaaea6a85c49000da83dc0cc
[5/6] io_uring: consistently use the EPOLL* defines
commit: a294bef57c55a45aef51d31e71d6892e8eba1483
[6/6] io_uring: use rcu_dereference in io_close
commit: 0bf1dbee9baf3e78bff297245178f8c9a8ef8670
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-05-18 12:27 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-18 8:39 sparse fixes for io_uring Christoph Hellwig
2022-05-18 8:40 ` [PATCH 1/6] io_uring: use a rwf_t for io_rw.flags Christoph Hellwig
2022-05-18 12:27 ` Jens Axboe
2022-05-18 8:40 ` [PATCH 2/6] io_uring: don't use ERR_PTR for user pointers Christoph Hellwig
2022-05-18 8:40 ` [PATCH 3/6] io_uring: drop a spurious inline on a forward declaration Christoph Hellwig
2022-05-18 8:40 ` [PATCH 4/6] io_uring: make apoll_events a __poll_t Christoph Hellwig
2022-05-18 8:40 ` [PATCH 5/6] io_uring: consistently use the EPOLL* defines Christoph Hellwig
2022-05-18 8:40 ` [PATCH 6/6] io_uring: use rcu_dereference in io_close Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox