Re: [PATCH 09/25] kcpustat: Track running task following vtime sequences

Peter Zijlstra
2018-11-20T13:59:25Z

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().