Skip to content

Commit a0d45c3

Browse files
committed
io_uring/fdinfo: remove need for sqpoll lock for thread/pid retrieval
A previous commit added a trylock for getting the SQPOLL thread info via fdinfo, but this introduced a regression where we often fail to get it if the thread is busy. For that case, we end up not printing the current CPU and PID info. Rather than rely on this lock, just print the pid we already stored in the io_sq_data struct, and ensure we update the current CPU every time we've slept or potentially rescheduled. The latter won't potentially be 100% accurate, but that wasn't the case before either as the task can get migrated at any time unless it has been pinned at creation time. We retain keeping the io_sq_data dereference inside the ctx->uring_lock, as it has always been, as destruction of the thread and data happen below that. We could make this RCU safe, but there's little point in doing that. With this, we always print the last valid information we had, rather than have spurious outputs with missing information. Fixes: 7644b1a ("io_uring/fdinfo: lock SQ thread while retrieving thread cpu/pid") Signed-off-by: Jens Axboe <[email protected]>
1 parent b85ea95 commit a0d45c3

File tree

2 files changed

+12
-9
lines changed

2 files changed

+12
-9
lines changed

io_uring/fdinfo.c

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -145,13 +145,8 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *f)
145145
if (has_lock && (ctx->flags & IORING_SETUP_SQPOLL)) {
146146
struct io_sq_data *sq = ctx->sq_data;
147147

148-
if (mutex_trylock(&sq->lock)) {
149-
if (sq->thread) {
150-
sq_pid = task_pid_nr(sq->thread);
151-
sq_cpu = task_cpu(sq->thread);
152-
}
153-
mutex_unlock(&sq->lock);
154-
}
148+
sq_pid = sq->task_pid;
149+
sq_cpu = sq->sq_cpu;
155150
}
156151

157152
seq_printf(m, "SqThread:\t%d\n", sq_pid);

io_uring/sqpoll.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ static bool io_sqd_handle_event(struct io_sq_data *sqd)
214214
did_sig = get_signal(&ksig);
215215
cond_resched();
216216
mutex_lock(&sqd->lock);
217+
sqd->sq_cpu = raw_smp_processor_id();
217218
}
218219
return did_sig || test_bit(IO_SQ_THREAD_SHOULD_STOP, &sqd->state);
219220
}
@@ -229,10 +230,15 @@ static int io_sq_thread(void *data)
229230
snprintf(buf, sizeof(buf), "iou-sqp-%d", sqd->task_pid);
230231
set_task_comm(current, buf);
231232

232-
if (sqd->sq_cpu != -1)
233+
/* reset to our pid after we've set task_comm, for fdinfo */
234+
sqd->task_pid = current->pid;
235+
236+
if (sqd->sq_cpu != -1) {
233237
set_cpus_allowed_ptr(current, cpumask_of(sqd->sq_cpu));
234-
else
238+
} else {
235239
set_cpus_allowed_ptr(current, cpu_online_mask);
240+
sqd->sq_cpu = raw_smp_processor_id();
241+
}
236242

237243
mutex_lock(&sqd->lock);
238244
while (1) {
@@ -261,6 +267,7 @@ static int io_sq_thread(void *data)
261267
mutex_unlock(&sqd->lock);
262268
cond_resched();
263269
mutex_lock(&sqd->lock);
270+
sqd->sq_cpu = raw_smp_processor_id();
264271
}
265272
continue;
266273
}
@@ -294,6 +301,7 @@ static int io_sq_thread(void *data)
294301
mutex_unlock(&sqd->lock);
295302
schedule();
296303
mutex_lock(&sqd->lock);
304+
sqd->sq_cpu = raw_smp_processor_id();
297305
}
298306
list_for_each_entry(ctx, &sqd->ctx_list, sqd_list)
299307
atomic_andnot(IORING_SQ_NEED_WAKEUP,

0 commit comments

Comments
 (0)