[PATCH 0/8] for-next cleanups

[PATCH 0/8] for-next cleanups

[Pavel Begunkov]

A random set of cleanups

Patches 1 and 2 address kernel test robot warnings.
Patches 3 and 4 add some extra lockdep checks
The rest are doing minor rsrc cleanups.

Pavel Begunkov (8):
  io_uring: shut io_prep_async_work warning
  io_uring/kbuf: remove extra ->buf_ring null check
  io_uring: add irq lockdep checks
  io_uring/rsrc: add lockdep checks
  io_uring/rsrc: consolidate node caching
  io_uring/rsrc: zero node's rsrc data on alloc
  io_uring/rsrc: refactor io_rsrc_node_switch
  io_uring/rsrc: extract SCM file put helper

 include/linux/io_uring_types.h |  1 -
 io_uring/alloc_cache.h         |  5 +++
 io_uring/io_uring.c            |  9 ++---
 io_uring/io_uring.h            |  2 +
 io_uring/kbuf.c                | 14 +++----
 io_uring/rsrc.c                | 73 +++++++++++++++-------------------
 io_uring/rsrc.h                |  7 ++++
 7 files changed, 56 insertions(+), 55 deletions(-)

-- 
2.40.0

[PATCH 1/8] io_uring: shut io_prep_async_work warning

[Pavel Begunkov]

io_uring/io_uring.c:432 io_prep_async_work() error: we previously
assumed 'req->file' could be null (see line 425).

Even though it's a false positive as there will not be REQ_F_ISREG set
without a file, let's add a simple check to make the kernel test robot
happy. We don't care about performance here, but assumingly it'll be
optimised out by the compiler.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/io_uring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 9bbf58297a0e..b171c26d331d 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -425,7 +425,7 @@ static void io_prep_async_work(struct io_kiocb *req)
 	if (req->file && !io_req_ffs_set(req))
 		req->flags |= io_file_get_flags(req->file) << REQ_F_SUPPORT_NOWAIT_BIT;
 
-	if (req->flags & REQ_F_ISREG) {
+	if (req->file && (req->flags & REQ_F_ISREG)) {
 		bool should_hash = def->hash_reg_file;
 
 		/* don't serialize this request if the fs doesn't need it */
-- 
2.40.0

[PATCH 2/8] io_uring/kbuf: remove extra ->buf_ring null check

[Pavel Begunkov]

The kernel test robot complains about __io_remove_buffers().

io_uring/kbuf.c:221 __io_remove_buffers() warn: variable dereferenced
before check 'bl->buf_ring' (see line 219)

That check is not needed as ->buf_ring will always be set, so we can
remove it and so silence the warning.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/kbuf.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
index 79c25459e8de..0905c1761fba 100644
--- a/io_uring/kbuf.c
+++ b/io_uring/kbuf.c
@@ -218,14 +218,12 @@ static int __io_remove_buffers(struct io_ring_ctx *ctx,
 	if (bl->is_mapped) {
 		i = bl->buf_ring->tail - bl->head;
 		if (bl->is_mmap) {
-			if (bl->buf_ring) {
-				struct page *page;
-
-				page = virt_to_head_page(bl->buf_ring);
-				if (put_page_testzero(page))
-					free_compound_page(page);
-				bl->buf_ring = NULL;
-			}
+			struct page *page;
+
+			page = virt_to_head_page(bl->buf_ring);
+			if (put_page_testzero(page))
+				free_compound_page(page);
+			bl->buf_ring = NULL;
 			bl->is_mmap = 0;
 		} else if (bl->buf_nr_pages) {
 			int j;
-- 
2.40.0

[PATCH 3/8] io_uring: add irq lockdep checks

[Pavel Begunkov]

We don't post CQEs from the IRQ context, add a check catching that.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/io_uring.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
index ef449e43d493..25515d69d205 100644
--- a/io_uring/io_uring.h
+++ b/io_uring/io_uring.h
@@ -94,6 +94,8 @@ bool io_match_task_safe(struct io_kiocb *head, struct task_struct *task,
 
 #define io_lockdep_assert_cq_locked(ctx)				\
 	do {								\
+		lockdep_assert(in_task());				\
+									\
 		if (ctx->flags & IORING_SETUP_IOPOLL) {			\
 			lockdep_assert_held(&ctx->uring_lock);		\
 		} else if (!ctx->task_complete) {			\
-- 
2.40.0

[PATCH 4/8] io_uring/rsrc: add lockdep checks

[Pavel Begunkov]

Add a lockdep chek to make sure that file and buffer updates hold
->uring_lock.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/rsrc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
index 603a783a0383..24e4e2109549 100644
--- a/io_uring/rsrc.c
+++ b/io_uring/rsrc.c
@@ -534,6 +534,8 @@ static int __io_register_rsrc_update(struct io_ring_ctx *ctx, unsigned type,
 	__u32 tmp;
 	int err;
 
+	lockdep_assert_held(&ctx->uring_lock);
+
 	if (check_add_overflow(up->offset, nr_args, &tmp))
 		return -EOVERFLOW;
 	err = io_rsrc_node_switch_start(ctx);
-- 
2.40.0

[PATCH 5/8] io_uring/rsrc: consolidate node caching

[Pavel Begunkov]

We store one pre-allocated rsrc node in ->rsrc_backup_node, merge it
with ->rsrc_node_cache.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 include/linux/io_uring_types.h |  1 -
 io_uring/alloc_cache.h         |  5 +++++
 io_uring/io_uring.c            |  2 --
 io_uring/rsrc.c                | 20 +++++++++++---------
 4 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
index fa621a508a01..40cab420b1bd 100644
--- a/include/linux/io_uring_types.h
+++ b/include/linux/io_uring_types.h
@@ -326,7 +326,6 @@ struct io_ring_ctx {
 	struct io_restriction		restrictions;
 
 	/* slow path rsrc auxilary data, used by update/register */
-	struct io_rsrc_node		*rsrc_backup_node;
 	struct io_mapped_ubuf		*dummy_ubuf;
 	struct io_rsrc_data		*file_data;
 	struct io_rsrc_data		*buf_data;
diff --git a/io_uring/alloc_cache.h b/io_uring/alloc_cache.h
index 851a527afb5e..241245cb54a6 100644
--- a/io_uring/alloc_cache.h
+++ b/io_uring/alloc_cache.h
@@ -23,6 +23,11 @@ static inline bool io_alloc_cache_put(struct io_alloc_cache *cache,
 	return false;
 }
 
+static inline bool io_alloc_cache_empty(struct io_alloc_cache *cache)
+{
+	return !cache->list.next;
+}
+
 static inline struct io_cache_entry *io_alloc_cache_get(struct io_alloc_cache *cache)
 {
 	if (cache->list.next) {
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index b171c26d331d..075bae8a2bb1 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -2852,8 +2852,6 @@ static __cold void io_ring_ctx_free(struct io_ring_ctx *ctx)
 	/* there are no registered resources left, nobody uses it */
 	if (ctx->rsrc_node)
 		io_rsrc_node_destroy(ctx, ctx->rsrc_node);
-	if (ctx->rsrc_backup_node)
-		io_rsrc_node_destroy(ctx, ctx->rsrc_backup_node);
 
 	WARN_ON_ONCE(!list_empty(&ctx->rsrc_ref_list));
 
diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
index 24e4e2109549..73f9e10d9bf0 100644
--- a/io_uring/rsrc.c
+++ b/io_uring/rsrc.c
@@ -230,7 +230,7 @@ void io_rsrc_node_switch(struct io_ring_ctx *ctx,
 			 struct io_rsrc_data *data_to_kill)
 	__must_hold(&ctx->uring_lock)
 {
-	WARN_ON_ONCE(!ctx->rsrc_backup_node);
+	WARN_ON_ONCE(io_alloc_cache_empty(&ctx->rsrc_node_cache));
 	WARN_ON_ONCE(data_to_kill && !ctx->rsrc_node);
 
 	if (data_to_kill) {
@@ -245,18 +245,20 @@ void io_rsrc_node_switch(struct io_ring_ctx *ctx,
 		ctx->rsrc_node = NULL;
 	}
 
-	if (!ctx->rsrc_node) {
-		ctx->rsrc_node = ctx->rsrc_backup_node;
-		ctx->rsrc_backup_node = NULL;
-	}
+	if (!ctx->rsrc_node)
+		ctx->rsrc_node = io_rsrc_node_alloc(ctx);
 }
 
 int io_rsrc_node_switch_start(struct io_ring_ctx *ctx)
 {
-	if (ctx->rsrc_backup_node)
-		return 0;
-	ctx->rsrc_backup_node = io_rsrc_node_alloc(ctx);
-	return ctx->rsrc_backup_node ? 0 : -ENOMEM;
+	if (io_alloc_cache_empty(&ctx->rsrc_node_cache)) {
+		struct io_rsrc_node *node = kzalloc(sizeof(*node), GFP_KERNEL);
+
+		if (!node)
+			return -ENOMEM;
+		io_alloc_cache_put(&ctx->rsrc_node_cache, &node->cache);
+	}
+	return 0;
 }
 
 __cold static int io_rsrc_ref_quiesce(struct io_rsrc_data *data,
-- 
2.40.0

[PATCH 6/8] io_uring/rsrc: zero node's rsrc data on alloc

[Pavel Begunkov]

struct io_rsrc_node::rsrc_data field is initialised on rsrc removal and
shouldn't be used before that, still let's play safe and zero the field
on alloc.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/rsrc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
index 73f9e10d9bf0..329cc3851dfd 100644
--- a/io_uring/rsrc.c
+++ b/io_uring/rsrc.c
@@ -218,6 +218,7 @@ static struct io_rsrc_node *io_rsrc_node_alloc(struct io_ring_ctx *ctx)
 			return NULL;
 	}
 
+	ref_node->rsrc_data = NULL;
 	ref_node->refs = 1;
 	INIT_LIST_HEAD(&ref_node->node);
 	INIT_LIST_HEAD(&ref_node->item_list);
-- 
2.40.0

[PATCH 7/8] io_uring/rsrc: refactor io_rsrc_node_switch

[Pavel Begunkov]

We use io_rsrc_node_switch() coupled with io_rsrc_node_switch_start()
for a bunch of cases including initialising ctx->rsrc_node, i.e. by
passing NULL instead of rsrc_data. Leave it to only deal with actual
node changing.

For that, first remove it from io_uring_create() and add a function
allocating the first node. Then also remove all calls to
io_rsrc_node_switch() from files/buffers register as we already have a
node installed and it does essentially nothing.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/io_uring.c |  5 ++---
 io_uring/rsrc.c     | 36 +++++++++++-------------------------
 io_uring/rsrc.h     |  7 +++++++
 3 files changed, 20 insertions(+), 28 deletions(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 075bae8a2bb1..9083a8466ebf 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -3881,11 +3881,10 @@ static __cold int io_uring_create(unsigned entries, struct io_uring_params *p,
 	ret = io_sq_offload_create(ctx, p);
 	if (ret)
 		goto err;
-	/* always set a rsrc node */
-	ret = io_rsrc_node_switch_start(ctx);
+
+	ret = io_rsrc_init(ctx);
 	if (ret)
 		goto err;
-	io_rsrc_node_switch(ctx, NULL);
 
 	memset(&p->sq_off, 0, sizeof(p->sq_off));
 	p->sq_off.head = offsetof(struct io_rings, sq.head);
diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
index 329cc3851dfd..f2c660ffea74 100644
--- a/io_uring/rsrc.c
+++ b/io_uring/rsrc.c
@@ -204,7 +204,7 @@ void io_rsrc_node_ref_zero(struct io_rsrc_node *node)
 	}
 }
 
-static struct io_rsrc_node *io_rsrc_node_alloc(struct io_ring_ctx *ctx)
+struct io_rsrc_node *io_rsrc_node_alloc(struct io_ring_ctx *ctx)
 {
 	struct io_rsrc_node *ref_node;
 	struct io_cache_entry *entry;
@@ -231,23 +231,18 @@ void io_rsrc_node_switch(struct io_ring_ctx *ctx,
 			 struct io_rsrc_data *data_to_kill)
 	__must_hold(&ctx->uring_lock)
 {
-	WARN_ON_ONCE(io_alloc_cache_empty(&ctx->rsrc_node_cache));
-	WARN_ON_ONCE(data_to_kill && !ctx->rsrc_node);
+	struct io_rsrc_node *node = ctx->rsrc_node;
+	struct io_rsrc_node *backup = io_rsrc_node_alloc(ctx);
 
-	if (data_to_kill) {
-		struct io_rsrc_node *rsrc_node = ctx->rsrc_node;
-
-		rsrc_node->rsrc_data = data_to_kill;
-		list_add_tail(&rsrc_node->node, &ctx->rsrc_ref_list);
-
-		data_to_kill->refs++;
-		/* put master ref */
-		io_put_rsrc_node(ctx, rsrc_node);
-		ctx->rsrc_node = NULL;
-	}
+	if (WARN_ON_ONCE(!backup))
+		return;
 
-	if (!ctx->rsrc_node)
-		ctx->rsrc_node = io_rsrc_node_alloc(ctx);
+	data_to_kill->refs++;
+	node->rsrc_data = data_to_kill;
+	list_add_tail(&node->node, &ctx->rsrc_ref_list);
+	/* put master ref */
+	io_put_rsrc_node(ctx, node);
+	ctx->rsrc_node = backup;
 }
 
 int io_rsrc_node_switch_start(struct io_ring_ctx *ctx)
@@ -921,9 +916,6 @@ int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg,
 		return -EMFILE;
 	if (nr_args > rlimit(RLIMIT_NOFILE))
 		return -EMFILE;
-	ret = io_rsrc_node_switch_start(ctx);
-	if (ret)
-		return ret;
 	ret = io_rsrc_data_alloc(ctx, io_rsrc_file_put, tags, nr_args,
 				 &ctx->file_data);
 	if (ret)
@@ -978,7 +970,6 @@ int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg,
 
 	/* default it to the whole table */
 	io_file_table_set_alloc_range(ctx, 0, ctx->nr_user_files);
-	io_rsrc_node_switch(ctx, NULL);
 	return 0;
 fail:
 	__io_sqe_files_unregister(ctx);
@@ -1260,9 +1251,6 @@ int io_sqe_buffers_register(struct io_ring_ctx *ctx, void __user *arg,
 		return -EBUSY;
 	if (!nr_args || nr_args > IORING_MAX_REG_BUFFERS)
 		return -EINVAL;
-	ret = io_rsrc_node_switch_start(ctx);
-	if (ret)
-		return ret;
 	ret = io_rsrc_data_alloc(ctx, io_rsrc_buf_put, tags, nr_args, &data);
 	if (ret)
 		return ret;
@@ -1300,8 +1288,6 @@ int io_sqe_buffers_register(struct io_ring_ctx *ctx, void __user *arg,
 	ctx->buf_data = data;
 	if (ret)
 		__io_sqe_buffers_unregister(ctx);
-	else
-		io_rsrc_node_switch(ctx, NULL);
 	return ret;
 }
 
diff --git a/io_uring/rsrc.h b/io_uring/rsrc.h
index 8729f2fee256..17dfe180208f 100644
--- a/io_uring/rsrc.h
+++ b/io_uring/rsrc.h
@@ -74,6 +74,7 @@ void io_rsrc_put_work(struct work_struct *work);
 void io_wait_rsrc_data(struct io_rsrc_data *data);
 void io_rsrc_node_destroy(struct io_ring_ctx *ctx, struct io_rsrc_node *ref_node);
 int io_rsrc_node_switch_start(struct io_ring_ctx *ctx);
+struct io_rsrc_node *io_rsrc_node_alloc(struct io_ring_ctx *ctx);
 int io_queue_rsrc_removal(struct io_rsrc_data *data, unsigned idx,
 			  struct io_rsrc_node *node, void *rsrc);
 void io_rsrc_node_switch(struct io_ring_ctx *ctx,
@@ -164,6 +165,12 @@ static inline u64 *io_get_tag_slot(struct io_rsrc_data *data, unsigned int idx)
 	return &data->tags[table_idx][off];
 }
 
+static inline int io_rsrc_init(struct io_ring_ctx *ctx)
+{
+	ctx->rsrc_node = io_rsrc_node_alloc(ctx);
+	return ctx->rsrc_node ? 0 : -ENOMEM;
+}
+
 int io_files_update(struct io_kiocb *req, unsigned int issue_flags);
 int io_files_update_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
 
-- 
2.40.0

[PATCH 8/8] io_uring/rsrc: extract SCM file put helper

[Pavel Begunkov]

SCM file accounting is a slow path and is only used for UNIX files.
Extract a helper out of io_rsrc_file_put() that does the SCM
unaccounting.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/rsrc.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
index f2c660ffea74..11058e20bdcc 100644
--- a/io_uring/rsrc.c
+++ b/io_uring/rsrc.c
@@ -832,20 +832,14 @@ int __io_scm_file_account(struct io_ring_ctx *ctx, struct file *file)
 	return 0;
 }
 
-static void io_rsrc_file_put(struct io_ring_ctx *ctx, struct io_rsrc_put *prsrc)
+static __cold void io_rsrc_file_scm_put(struct io_ring_ctx *ctx, struct file *file)
 {
-	struct file *file = prsrc->file;
 #if defined(CONFIG_UNIX)
 	struct sock *sock = ctx->ring_sock->sk;
 	struct sk_buff_head list, *head = &sock->sk_receive_queue;
 	struct sk_buff *skb;
 	int i;
 
-	if (!io_file_need_scm(file)) {
-		fput(file);
-		return;
-	}
-
 	__skb_queue_head_init(&list);
 
 	/*
@@ -895,11 +889,19 @@ static void io_rsrc_file_put(struct io_ring_ctx *ctx, struct io_rsrc_put *prsrc)
 			__skb_queue_tail(head, skb);
 		spin_unlock_irq(&head->lock);
 	}
-#else
-	fput(file);
 #endif
 }
 
+static void io_rsrc_file_put(struct io_ring_ctx *ctx, struct io_rsrc_put *prsrc)
+{
+	struct file *file = prsrc->file;
+
+	if (likely(!io_file_need_scm(file)))
+		fput(file);
+	else
+		io_rsrc_file_scm_put(ctx, file);
+}
+
 int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg,
 			  unsigned nr_args, u64 __user *tags)
 {
-- 
2.40.0

Re: [PATCH 0/8] for-next cleanups

[Jens Axboe]

On Tue, 11 Apr 2023 12:06:00 +0100, Pavel Begunkov wrote:
> A random set of cleanups
> 
> Patches 1 and 2 address kernel test robot warnings.
> Patches 3 and 4 add some extra lockdep checks
> The rest are doing minor rsrc cleanups.
> 
> Pavel Begunkov (8):
>   io_uring: shut io_prep_async_work warning
>   io_uring/kbuf: remove extra ->buf_ring null check
>   io_uring: add irq lockdep checks
>   io_uring/rsrc: add lockdep checks
>   io_uring/rsrc: consolidate node caching
>   io_uring/rsrc: zero node's rsrc data on alloc
>   io_uring/rsrc: refactor io_rsrc_node_switch
>   io_uring/rsrc: extract SCM file put helper
> 
> [...]

Applied, thanks!

[1/8] io_uring: shut io_prep_async_work warning
      commit: 8b1df11f97333d6d8647f1c6c0554eb2d9774396
[2/8] io_uring/kbuf: remove extra ->buf_ring null check
      commit: ceac766a5581e4e671ec8e5236b8fdaed8e4c8c9
[3/8] io_uring: add irq lockdep checks
      commit: 8ce4269eeedc5b31f5817f610b42cba8be8fa9de
[4/8] io_uring/rsrc: add lockdep checks
      commit: 786788a8cfe03056e9c7b1c6e418c1db92a0ce80
[5/8] io_uring/rsrc: consolidate node caching
      commit: 528407b1e0ea51260fff2cc8b669c632a65d7a09
[6/8] io_uring/rsrc: zero node's rsrc data on alloc
      commit: 13c223962eac16f161cf9b6355209774c609af28
[7/8] io_uring/rsrc: refactor io_rsrc_node_switch
      commit: 2933ae6eaa05e8db6ad33a3ca12af18d2a25358c
[8/8] io_uring/rsrc: extract SCM file put helper
      commit: d581076b6a85c6f8308a4ba2bdcd82651f5183df

Best regards,
-- 
Jens Axboe