On 11/11/20 7:04 AM, Xiaoguang Wang wrote:
> hi,
>
>> Hello Xiaoguang Wang,
>>
>> The patch e0c06f5ab2c5: "io_uring: refactor io_sq_thread() handling"
>> from Nov 3, 2020, leads to the following static checker warning:
>>
>> fs/io_uring.c:6939 io_sq_thread()
>> error: uninitialized symbol 'timeout'.
>>
>> fs/io_uring.c
>> 6883 static int io_sq_thread(void *data)
>> 6884 {
>> 6885 struct cgroup_subsys_state *cur_css = NULL;
>> 6886 struct files_struct *old_files = current->files;
>> 6887 struct nsproxy *old_nsproxy = current->nsproxy;
>> 6888 struct pid *old_thread_pid = current->thread_pid;
>> 6889 const struct cred *old_cred = NULL;
>> 6890 struct io_sq_data *sqd = data;
>> 6891 struct io_ring_ctx *ctx;
>> 6892 unsigned long timeout;
>> ^^^^^^^^^^^^^^^^^^^^^^
>>
>> 6893 DEFINE_WAIT(wait);
>> 6894
>> 6895 task_lock(current);
>> 6896 current->files = NULL;
>> 6897 current->nsproxy = NULL;
>> 6898 current->thread_pid = NULL;
>> 6899 task_unlock(current);
>> 6900
>> 6901 while (!kthread_should_stop()) {
>> 6902 int ret;
>> 6903 bool cap_entries, sqt_spin, needs_sched;
>> 6904
>> 6905 /*
>> 6906 * Any changes to the sqd lists are synchronized through the
>> 6907 * kthread parking. This synchronizes the thread vs users,
>> 6908 * the users are synchronized on the sqd->ctx_lock.
>> 6909 */
>> 6910 if (kthread_should_park())
>> 6911 kthread_parkme();
>> 6912
>> 6913 if (unlikely(!list_empty(&sqd->ctx_new_list))) {
>> 6914 io_sqd_init_new(sqd);
>> 6915 timeout = jiffies + sqd->sq_thread_idle;
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> timeout not set on else path.
> Thanks for the report, but indeed I think it's not a bug. When io_sq_thread
> is created initially, it's not waken up to run, and once it's waken up to run,
> it will see that sqd->ctx_new_list is not empty, then timeout always can be
> initialized.
We should still clean it up and avoid both the checker tripping on on this,
and humans. It's not easy/possible to verify that it is sane.
--
Jens Axboe
