* Re: [PATCHSET 0/2] io_rsrc_node cleanups
2024-11-03 17:47 [PATCHSET 0/2] io_rsrc_node cleanups Jens Axboe
@ 2024-11-03 18:03 ` Jens Axboe
0 siblings, 0 replies; 2+ messages in thread
From: Jens Axboe @ 2024-11-03 18:03 UTC (permalink / raw)
To: io-uring
[-- Attachment #1: Type: text/plain, Size: 1029 bytes --]
On 11/3/24 10:47 AM, Jens Axboe wrote:
> Hi,
>
> Nothing major here - just a patch reclaiming a bit of space in struct
> io_rsrc_node. Nothing that yields anything yet, but may as well free
> up the 'type' to have more future room.
>
> 2nd patch reclaims 8b from struct io_kiocb, by taking advantage of the
> fact that provided and registered buffers cannot currently be used
> together. This may change in the future, but it's true for now.
>
> include/linux/io_uring_types.h | 7 ++++++-
> io_uring/io_uring.c | 6 +++---
> io_uring/net.c | 3 ++-
> io_uring/nop.c | 3 ++-
> io_uring/notif.c | 4 ++--
> io_uring/rsrc.c | 11 +++++------
> io_uring/rsrc.h | 31 +++++++++++++++++++++++--------
> io_uring/rw.c | 3 ++-
> io_uring/uring_cmd.c | 4 ++--
> 9 files changed, 47 insertions(+), 25 deletions(-)
Not sure what happened with git send-email here, but here are the two
patches.
--
Jens Axboe
[-- Attachment #2: 0002-io_uring-rsrc-split-io_kiocb-node-type-assignments.patch --]
[-- Type: text/x-patch, Size: 7340 bytes --]
From 0c81a9455e0f5b440bb29efcef35225841328a91 Mon Sep 17 00:00:00 2001
From: Jens Axboe <[email protected]>
Date: Sun, 3 Nov 2024 08:46:07 -0700
Subject: [PATCH 2/2] io_uring/rsrc: split io_kiocb node type assignments
Currently the io_rsrc_node assignment in io_kiocb is an array of two
pointers, as two nodes may be assigned to a request - one file node,
and one buffer node. However, the buffer node can co-exist with the
provided buffers, as currently it's not supported to use both provided
and registered buffers at the same time.
This crucially brings struct io_kiocb down to 4 cache lines again, as
before it spilled into the 5th cacheline.
Signed-off-by: Jens Axboe <[email protected]>
---
include/linux/io_uring_types.h | 7 ++++++-
io_uring/io_uring.c | 6 +++---
io_uring/net.c | 3 ++-
io_uring/nop.c | 3 ++-
io_uring/notif.c | 4 ++--
io_uring/rsrc.h | 16 ++++++++++------
io_uring/rw.c | 3 ++-
io_uring/uring_cmd.c | 4 ++--
8 files changed, 29 insertions(+), 17 deletions(-)
diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
index 8ea01ff009b1..a87927a392f2 100644
--- a/include/linux/io_uring_types.h
+++ b/include/linux/io_uring_types.h
@@ -479,6 +479,7 @@ enum {
REQ_F_BL_NO_RECYCLE_BIT,
REQ_F_BUFFERS_COMMIT_BIT,
REQ_F_GROUP_LEADER_BIT,
+ REQ_F_BUF_NODE_BIT,
/* not a real bit, just to check we're not overflowing the space */
__REQ_F_LAST_BIT,
@@ -561,6 +562,8 @@ enum {
REQ_F_BUFFERS_COMMIT = IO_REQ_FLAG(REQ_F_BUFFERS_COMMIT_BIT),
/* sqe group lead */
REQ_F_GROUP_LEADER = IO_REQ_FLAG(REQ_F_GROUP_LEADER_BIT),
+ /* buf node is valid */
+ REQ_F_BUF_NODE = IO_REQ_FLAG(REQ_F_BUF_NODE_BIT),
};
typedef void (*io_req_tw_func_t)(struct io_kiocb *req, struct io_tw_state *ts);
@@ -641,6 +644,8 @@ struct io_kiocb {
* REQ_F_BUFFER_RING is set.
*/
struct io_buffer_list *buf_list;
+
+ struct io_rsrc_node *buf_node;
};
union {
@@ -650,7 +655,7 @@ struct io_kiocb {
__poll_t apoll_events;
};
- struct io_rsrc_node *rsrc_nodes[2];
+ struct io_rsrc_node *file_node;
atomic_t refs;
bool cancel_seq_set;
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 921d79dfb955..5b421e67c031 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1081,8 +1081,8 @@ void io_req_defer_failed(struct io_kiocb *req, s32 res)
static void io_preinit_req(struct io_kiocb *req, struct io_ring_ctx *ctx)
{
req->ctx = ctx;
- req->rsrc_nodes[IORING_RSRC_FILE] = NULL;
- req->rsrc_nodes[IORING_RSRC_BUFFER] = NULL;
+ req->buf_node = NULL;
+ req->file_node = NULL;
req->link = NULL;
req->async_data = NULL;
/* not necessary, but safer to zero */
@@ -2044,7 +2044,7 @@ inline struct file *io_file_get_fixed(struct io_kiocb *req, int fd,
io_ring_submit_lock(ctx, issue_flags);
node = io_rsrc_node_lookup(&ctx->file_table.data, fd);
if (node) {
- io_req_assign_rsrc_node(req, node);
+ io_req_assign_rsrc_node(&req->file_node, node);
req->flags |= io_slot_flags(node);
file = io_slot_file(node);
}
diff --git a/io_uring/net.c b/io_uring/net.c
index 2f7b334ed708..2ccc2b409431 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -1348,7 +1348,8 @@ static int io_send_zc_import(struct io_kiocb *req, unsigned int issue_flags)
io_ring_submit_lock(ctx, issue_flags);
node = io_rsrc_node_lookup(&ctx->buf_table, sr->buf_index);
if (node) {
- io_req_assign_rsrc_node(sr->notif, node);
+ io_req_assign_rsrc_node(&sr->notif->buf_node, node);
+ sr->notif->flags |= REQ_F_BUF_NODE;
ret = 0;
}
io_ring_submit_unlock(ctx, issue_flags);
diff --git a/io_uring/nop.c b/io_uring/nop.c
index 149dbdc53607..bc22bcc739f3 100644
--- a/io_uring/nop.c
+++ b/io_uring/nop.c
@@ -67,7 +67,8 @@ int io_nop(struct io_kiocb *req, unsigned int issue_flags)
io_ring_submit_lock(ctx, issue_flags);
node = io_rsrc_node_lookup(&ctx->buf_table, nop->buffer);
if (node) {
- io_req_assign_rsrc_node(req, node);
+ io_req_assign_rsrc_node(&req->buf_node, node);
+ req->flags |= REQ_F_BUF_NODE;
ret = 0;
}
io_ring_submit_unlock(ctx, issue_flags);
diff --git a/io_uring/notif.c b/io_uring/notif.c
index 4f02e969cf08..8dfbb0bd8e4d 100644
--- a/io_uring/notif.c
+++ b/io_uring/notif.c
@@ -117,8 +117,8 @@ struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx)
notif->file = NULL;
notif->task = current;
io_get_task_refs(1);
- notif->rsrc_nodes[IORING_RSRC_FILE] = NULL;
- notif->rsrc_nodes[IORING_RSRC_BUFFER] = NULL;
+ notif->file_node = NULL;
+ notif->buf_node = NULL;
nd = io_notif_to_data(notif);
nd->zc_report = false;
diff --git a/io_uring/rsrc.h b/io_uring/rsrc.h
index 9a8fac31fa49..bc3a863b14bb 100644
--- a/io_uring/rsrc.h
+++ b/io_uring/rsrc.h
@@ -95,10 +95,14 @@ static inline bool io_reset_rsrc_node(struct io_rsrc_data *data, int index)
static inline void io_req_put_rsrc_nodes(struct io_kiocb *req)
{
- io_put_rsrc_node(req->rsrc_nodes[IORING_RSRC_FILE]);
- io_put_rsrc_node(req->rsrc_nodes[IORING_RSRC_BUFFER]);
- req->rsrc_nodes[IORING_RSRC_FILE] = NULL;
- req->rsrc_nodes[IORING_RSRC_BUFFER] = NULL;
+ if (req->file_node) {
+ io_put_rsrc_node(req->file_node);
+ req->file_node = NULL;
+ }
+ if (req->flags & REQ_F_BUF_NODE) {
+ io_put_rsrc_node(req->buf_node);
+ req->buf_node = NULL;
+ }
}
static inline struct io_ring_ctx *io_rsrc_node_ctx(struct io_rsrc_node *node)
@@ -111,11 +115,11 @@ static inline int io_rsrc_node_type(struct io_rsrc_node *node)
return node->ctx_ptr & IORING_RSRC_TYPE_MASK;
}
-static inline void io_req_assign_rsrc_node(struct io_kiocb *req,
+static inline void io_req_assign_rsrc_node(struct io_rsrc_node **dst_node,
struct io_rsrc_node *node)
{
node->refs++;
- req->rsrc_nodes[io_rsrc_node_type(node)] = node;
+ *dst_node = node;
}
int io_files_update(struct io_kiocb *req, unsigned int issue_flags);
diff --git a/io_uring/rw.c b/io_uring/rw.c
index 1ea6be2ccc90..144730344c0f 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -341,7 +341,8 @@ static int io_prep_rw_fixed(struct io_kiocb *req, const struct io_uring_sqe *sqe
node = io_rsrc_node_lookup(&ctx->buf_table, req->buf_index);
if (!node)
return -EFAULT;
- io_req_assign_rsrc_node(req, node);
+ io_req_assign_rsrc_node(&req->buf_node, node);
+ req->flags |= REQ_F_BUF_NODE;
io = req->async_data;
ret = io_import_fixed(ddir, &io->iter, node->buf, rw->addr, rw->len);
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index 535909a38e76..88a73d21fc0b 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -219,7 +219,7 @@ int io_uring_cmd_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
* being called. This prevents destruction of the mapped buffer
* we'll need at actual import time.
*/
- io_req_assign_rsrc_node(req, node);
+ io_req_assign_rsrc_node(&req->buf_node, node);
}
ioucmd->cmd_op = READ_ONCE(sqe->cmd_op);
@@ -275,7 +275,7 @@ int io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
struct iov_iter *iter, void *ioucmd)
{
struct io_kiocb *req = cmd_to_io_kiocb(ioucmd);
- struct io_rsrc_node *node = req->rsrc_nodes[IORING_RSRC_BUFFER];
+ struct io_rsrc_node *node = req->buf_node;
/* Must have had rsrc_node assigned at prep time */
if (node)
--
2.45.2
[-- Attachment #3: 0001-io_uring-rsrc-encode-node-type-and-ctx-together.patch --]
[-- Type: text/x-patch, Size: 2834 bytes --]
From 020853e1546816a308067e6690d39b1a9c31a69d Mon Sep 17 00:00:00 2001
From: Jens Axboe <[email protected]>
Date: Sun, 3 Nov 2024 08:17:28 -0700
Subject: [PATCH 1/2] io_uring/rsrc: encode node type and ctx together
Rather than keep the type field separate rom ctx, use the fact that we
can encode up to 4 types of nodes in the LSB of the ctx pointer. Doesn't
reclaim any space right now on 64-bit archs, but it leaves a full int
for future use.
Signed-off-by: Jens Axboe <[email protected]>
---
io_uring/rsrc.c | 11 +++++------
io_uring/rsrc.h | 17 ++++++++++++++---
2 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
index 60fa857985cb..2fb1791d7255 100644
--- a/io_uring/rsrc.c
+++ b/io_uring/rsrc.c
@@ -124,9 +124,8 @@ struct io_rsrc_node *io_rsrc_node_alloc(struct io_ring_ctx *ctx, int type)
node = kzalloc(sizeof(*node), GFP_KERNEL);
if (node) {
- node->ctx = ctx;
+ node->ctx_ptr = (unsigned long) ctx | type;
node->refs = 1;
- node->type = type;
}
return node;
}
@@ -445,21 +444,21 @@ int io_files_update(struct io_kiocb *req, unsigned int issue_flags)
void io_free_rsrc_node(struct io_rsrc_node *node)
{
- struct io_ring_ctx *ctx = node->ctx;
+ struct io_ring_ctx *ctx = io_rsrc_node_ctx(node);
lockdep_assert_held(&ctx->uring_lock);
if (node->tag)
- io_post_aux_cqe(node->ctx, node->tag, 0, 0);
+ io_post_aux_cqe(ctx, node->tag, 0, 0);
- switch (node->type) {
+ switch (io_rsrc_node_type(node)) {
case IORING_RSRC_FILE:
if (io_slot_file(node))
fput(io_slot_file(node));
break;
case IORING_RSRC_BUFFER:
if (node->buf)
- io_buffer_unmap(node->ctx, node);
+ io_buffer_unmap(ctx, node);
break;
default:
WARN_ON_ONCE(1);
diff --git a/io_uring/rsrc.h b/io_uring/rsrc.h
index a40fad783a69..9a8fac31fa49 100644
--- a/io_uring/rsrc.h
+++ b/io_uring/rsrc.h
@@ -11,12 +11,13 @@
enum {
IORING_RSRC_FILE = 0,
IORING_RSRC_BUFFER = 1,
+
+ IORING_RSRC_TYPE_MASK = 0x3UL,
};
struct io_rsrc_node {
- struct io_ring_ctx *ctx;
+ unsigned long ctx_ptr;
int refs;
- u16 type;
u64 tag;
union {
@@ -100,11 +101,21 @@ static inline void io_req_put_rsrc_nodes(struct io_kiocb *req)
req->rsrc_nodes[IORING_RSRC_BUFFER] = NULL;
}
+static inline struct io_ring_ctx *io_rsrc_node_ctx(struct io_rsrc_node *node)
+{
+ return (struct io_ring_ctx *) (node->ctx_ptr & ~IORING_RSRC_TYPE_MASK);
+}
+
+static inline int io_rsrc_node_type(struct io_rsrc_node *node)
+{
+ return node->ctx_ptr & IORING_RSRC_TYPE_MASK;
+}
+
static inline void io_req_assign_rsrc_node(struct io_kiocb *req,
struct io_rsrc_node *node)
{
node->refs++;
- req->rsrc_nodes[node->type] = node;
+ req->rsrc_nodes[io_rsrc_node_type(node)] = node;
}
int io_files_update(struct io_kiocb *req, unsigned int issue_flags);
--
2.45.2
^ permalink raw reply related [flat|nested] 2+ messages in thread