On Wed, Nov 14, 2018 at 03:45:53AM +0100, Frederic Weisbecker wrote: > In order to make kcpustat vtime aware (ie: work on nohz_full without > freezing), we need to track the task running on the CPU in order to > fetch its vtime delta and add it to the relevant kcpustat field. > > The most efficient way to track this task is to use RCU. The task is > assigned on context switch right after we flush the vtime of the previous > task and the next task has been set on vtime. > > Things are then prepared to be ordered that way: > > WRITER (ctx switch) READER > ------------------ ----------------------- > vtime_seqcount_write_lock(prev) rcu_read_lock() > //flush prev vtime curr = > rcu_dereference(kcpustat->curr) > vtime_seqcount_write_unlock(prev) vtime_seqcount_read_start(curr) > //fetch curr vtime > vtime_seqcount_lock(next) vtime_seqcount_read_end(curr) > //Init vtime rcu_read_unlock() > vtime_seqcount_unlock(next) > > rcu_assign_pointer(kcpustat->curr, next) > > With this ordering layout, we are sure that we get a sequence with a > coherent couple (task cputime, kcpustat).
I'm confused; earlier you added a ->cpu member; but I don't see that used. Also, I'm confuddled on the purpose of rcu_assign_pointer(), what does the store_release therein ensure? Also, I'm pretty sure the below is terminally broken; task_struct is not rcu-freed, and therefore the above scenario is just broken. Nothing stops the task from going away right after rcu_dereference().
