Skip to content

Commit 805b13a

Browse files
committed
io_uring: ensure RCU callback ordering with rcu_barrier()
After more careful studying, Paul informs me that we cannot rely on ordering of RCU callbacks in the way that the the tagged commit did. The current construct looks like this: void C(struct rcu_head *rhp) { do_something(rhp); call_rcu(&p->rh, B); } call_rcu(&p->rh, A); call_rcu(&p->rh, C); and we're relying on ordering between A and B, which isn't guaranteed. Make this explicit instead, and have a work item issue the rcu_barrier() to ensure that A has run before we manually execute B. While thorough testing never showed this issue, it's dependent on the per-cpu load in terms of RCU callbacks. The updated method simplifies the code as well, and eliminates the need to maintain an rcu_head in the fileset data. Fixes: c1e2148 ("io_uring: free fixed_file_data after RCU grace period") Reported-by: Paul E. McKenney <[email protected]> Signed-off-by: Jens Axboe <[email protected]>
1 parent f0e20b8 commit 805b13a

File tree

1 file changed

+13
-16
lines changed

1 file changed

+13
-16
lines changed

fs/io_uring.c

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,6 @@ struct fixed_file_data {
191191
struct llist_head put_llist;
192192
struct work_struct ref_work;
193193
struct completion done;
194-
struct rcu_head rcu;
195194
};
196195

197196
struct io_ring_ctx {
@@ -5331,24 +5330,21 @@ static void io_file_ref_kill(struct percpu_ref *ref)
53315330
complete(&data->done);
53325331
}
53335332

5334-
static void __io_file_ref_exit_and_free(struct rcu_head *rcu)
5333+
static void io_file_ref_exit_and_free(struct work_struct *work)
53355334
{
5336-
struct fixed_file_data *data = container_of(rcu, struct fixed_file_data,
5337-
rcu);
5338-
percpu_ref_exit(&data->refs);
5339-
kfree(data);
5340-
}
5335+
struct fixed_file_data *data;
5336+
5337+
data = container_of(work, struct fixed_file_data, ref_work);
53415338

5342-
static void io_file_ref_exit_and_free(struct rcu_head *rcu)
5343-
{
53445339
/*
5345-
* We need to order our exit+free call against the potentially
5346-
* existing call_rcu() for switching to atomic. One way to do that
5347-
* is to have this rcu callback queue the final put and free, as we
5348-
* could otherwise have a pre-existing atomic switch complete _after_
5349-
* the free callback we queued.
5340+
* Ensure any percpu-ref atomic switch callback has run, it could have
5341+
* been in progress when the files were being unregistered. Once
5342+
* that's done, we can safely exit and free the ref and containing
5343+
* data structure.
53505344
*/
5351-
call_rcu(rcu, __io_file_ref_exit_and_free);
5345+
rcu_barrier();
5346+
percpu_ref_exit(&data->refs);
5347+
kfree(data);
53525348
}
53535349

53545350
static int io_sqe_files_unregister(struct io_ring_ctx *ctx)
@@ -5369,7 +5365,8 @@ static int io_sqe_files_unregister(struct io_ring_ctx *ctx)
53695365
for (i = 0; i < nr_tables; i++)
53705366
kfree(data->table[i].files);
53715367
kfree(data->table);
5372-
call_rcu(&data->rcu, io_file_ref_exit_and_free);
5368+
INIT_WORK(&data->ref_work, io_file_ref_exit_and_free);
5369+
queue_work(system_wq, &data->ref_work);
53735370
ctx->file_data = NULL;
53745371
ctx->nr_user_files = 0;
53755372
return 0;

0 commit comments

Comments
 (0)