On Tue, Apr 28, 2020 at 9:31 AM Peter Zijlstra <[email protected]> wrote: > > On Mon, Apr 27, 2020 at 11:43:58AM -0700, Suren Baghdasaryan wrote: > > A number of kthread-related functions indirectly take task_struct->pi_lock > > while holding worker->lock in the call chain like this: > > spin_lock(&worker->lock) > > kthread_insert_work > > wake_up_process > > try_to_wake_up > > raw_spin_lock_irqsave(&p->pi_lock, flags) > > > > This lock dependency exists whenever kthread_insert_work is called either > > directly or indirectly via __kthread_queue_delayed_work in the following > > functions: > > kthread_queue_work > > kthread_delayed_work_timer_fn > > kthread_queue_delayed_work > > kthread_flush_work > > kthread_mod_delayed_work > > > > This creates possibilities for circular dependencies like the one reported > > at: https://lkml.org/lkml/2020/4/24/954 > > Please, do not use lkml.org links.
Thanks for the review! Would https://lore.kernel.org/lkml/cajucfpg4nkhpqvzjgxz_3gm6hf1qgn_euoq8ix9cv1k9whl...@mail.gmail.com be better or should I just add the body of that report here? Or do not mention it at all? > > Also, ideally, we'd pull that kthread_queue_delayed_work() out from > under rq->lock. I understand but I don't see an easy way to do that. We need to start PSI polling whenever a monitored PSI state changes: https://elixir.bootlin.com/linux/v5.6.7/source/kernel/sched/psi.c#L783. This is happening under rq->lock because PSI accounting is done from inside enqueue_task/dequeue_task - the call chain is: enqueue_task > psi_enqueue > psi_task_change > psi_group_change > psi_schedule_poll_work > psi_task_change IIUC enqueue_task/dequeue_task are called with rq->lock taken, so moving kthread_queue_delayed_work out is not trivial. > > In fact, looking at it, WTH is the delayed branch of > kthread_queue_delayed_work() under that lock? That whole > delayed_work_list thing smells like bong-hits. I have the poll_scheduled atomic specifically to ensure that kthread_queue_delayed_work does not block as commented here: https://elixir.bootlin.com/linux/v5.7-rc3/source/kernel/sched/psi.c#L551. I understand this is not ideal. If there is a better way to schedule that kworker while ensuring it does not block I would be happy to rework this. Any suggestions? > > -- > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. >
