On Wed, Oct 28, 2015 at 6:58 PM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote: > On Wed, Oct 28, 2015 at 05:57:10PM -0700, Olav Haugan wrote: >> On 15-10-25 11:09:24, Peter Zijlstra wrote: >> > On Sat, Oct 24, 2015 at 11:01:02AM -0700, Olav Haugan wrote: >> > > Task->on_rq has three states: >> > > 0 - Task is not on runqueue (rq) >> > > 1 (TASK_ON_RQ_QUEUED) - Task is on rq >> > > 2 (TASK_ON_RQ_MIGRATING) - Task is on rq but in the process of being >> > > migrated to another rq >> > > >> > > When a task is moving between rqs task->on_rq state should be >> > > TASK_ON_RQ_MIGRATING >> > >> > Only when not holding both rq locks.. >> >> IMHO I think we should keep the state of p->on_rq updated with the correct state >> all the time unless I am incorrect in what p->on_rq represent. The task >> is moving between rq's and is on the rq so the state should be >> TASK_ON_RQ_MIGRATING right? I do realize that the code is currently not >> broken. However, in the future someone might come along and change >> set_task_cpu() and the code change might rely on an accurate p->on_rq value. It >> would be good design to keep this value correct. > > At the same time; we should also provide lean and fast code. Is it > better to add assertions about required state than to add superfluous > code for just in case scenarios. The state is only worth publishing if it's exceptional. I think Peter's new documentation helps to make this more clear. The intent of this change may be better captured by pointing out in a comment somewhere that detach_task() is *also* updating the task_cpu pointer which then lets us lean on holding that lock to make the state non-interesting.` > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html