Maybe to discern between find_lock_later_rq() callers we can use dl_throttled flag in dl_se and still implement the fix in find_lock_ later_rq()? I.e., fix similar to the rt.c patch in case the task is not throttled (so caller is push_dl_task()) and not rely on pick_next_ pushable_dl_task() if the task is throttled.
Sure I can do this as well but like I mentioned above I don’t think it will be any different than this patch unless we want to handle the race for offline migration case or if you prefer this in find_lock_later_rq just to keep it more inline with the rt patch. I just found the current approach to be less risky :)
What you mean with "handle the race for offline migration case"?
By offline migration I meant dl_task_offline_migration path which calls find_lock_later_rq. So unless we think the same race that this fix is trying to address for push_dl_task can happen for dl_task_offline_migration, there is one less reason to encapsulate this in find_lock_later_rq.
And I am honestly conflicted. I think I like the encapsulation better if we can find a solution inside find_lock_later_rq(), as it also aligns better with rt.c, but you fear it's more fragile?
Yes I agree that encapsulation in find_lock_later_rq will be ideal but by keeping it limited to push_dl_task I wanted to keep the change more targeted to avoid any possible side effect on dl_task_offline_migration call path.
Let’s say if we go ahead with making the change in find_lock_later_rq itself then we will have to fallback to current checks for throttled case and for remaining we will use the task != pick_next_pushable_dl_task(rq) check. Below is the diff of how it will be:
/* Retry if something changed. */ if (double_lock_balance(rq, later_rq)) { - if (unlikely(task_rq(task) != rq || + if (unlikely(is_migration_disabled(task) || !cpumask_test_cpu(later_rq->cpu, &task->cpus_mask) || - task_on_cpu(rq, task) || - !dl_task(task) || - is_migration_disabled(task) || - !task_on_rq_queued(task))) { + (task->dl.dl_throttled && + (task_rq(task) != rq || + task_on_cpu(rq, task) || + !dl_task(task) + !task_on_rq_queued(task))) || + (!task->dl.dl_throttled && + task != pick_next_pushable_dl_task(rq)))) { double_unlock_balance(rq, later_rq); later_rq = NULL; break;
Let me know your thoughts and I can send v2 patch accordingly.
Thanks, Harshit