Re: [RFC][PATCH v2 11/11] context_tracking,x86: Fix text_poke_sync() vs NOHZ_FULL

On Tue, Oct 26, 2021 at 03:19:11PM -0300, Marcelo Tosatti wrote:

On Thu, Oct 21, 2021 at 10:18:59PM +0200, Peter Zijlstra wrote:

quoted

On Thu, Oct 21, 2021 at 04:57:09PM -0300, Marcelo Tosatti wrote:

quoted

quoted

Pretty much everything in noinstr is magical, we just have to think
harder there (and possibly start writing more comments there).

mds_user_clear_cpu_buffers happens after sync_core, in your patchset, 
if i am not mistaken.

Of course it does, mds_user_clear_cpu_buffers() is on exit, the
sync_core() is on entry.

                                                                  static_key enable/disable

__exit_to_user_mode ->                                            context_tracking_set_cpu_work(cpu, work)
   user_enter_irqoff ->                                                  preempt_disable();
   __context_tracking_enter(CONTEXT_USER);                               seq = atomic_read(&ct->seq);
      ct_seq_user_enter(raw_cpu_ptr(&context_tracking));                 if (__context_tracking_seq_in_user(seq)) {
      {                                                                          /* ctrl-dep */
        arch_atomic_set(&ct->work, 0);                                           atomic_or(work, &ct->work);
        return arch_atomic_add_return(CT_SEQ_USER, &ct->seq);                    ret = atomic_try_cmpxchg(&ct->seq, &seq, seq|CT_SEQ_WORK);
                                                                         }
      }                                                                  preempt_enable();
   arch_exit_to_user_mode()
   mds_user_clear_cpu_buffers();  <--- sync_core work queued,
                                       but not executed.
                                       i-cache potentially stale?

ct_seq_user_enter should happen _after_ all possible static_key users?

Right, so this one is actually okay, because that branch is *never*
changed after boot.

I'm not quite sure why it isn't an alternative(). At some point I
proposed static_call_lock() [1] and the corrolary is static_branch_lock(),
which I suppose could be employed here. But I'm not sure that actually
helps much with auditing all that.


[1] https://lkml.kernel.org/r/20210904105529.GA5106@worktop.programming.kicks-ass.net