Re: [PATCH 2/8] x86/mm: Handle unlazying membarrier core sync in the arch code
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date: 2021-06-16 17:49:18
Also in:
lkml
----- On Jun 15, 2021, at 11:21 PM, Andy Lutomirski luto@kernel.org wrote: [...]
quoted hunk ↗ jump to hunk
@@ -473,16 +474,24 @@ void switch_mm_irqs_off(struct mm_struct *prev, structmm_struct *next,
[...]
quoted hunk ↗ jump to hunk
@@ -510,16 +520,35 @@ void switch_mm_irqs_off(struct mm_struct *prev, structmm_struct *next, * If the TLB is up to date, just use it. * The barrier synchronizes with the tlb_gen increment in * the TLB shootdown code. + * + * As a future optimization opportunity, it's plausible + * that the x86 memory model is strong enough that this + * smp_mb() isn't needed. */ smp_mb(); next_tlb_gen = atomic64_read(&next->context.tlb_gen); if (this_cpu_read(cpu_tlbstate.ctxs[prev_asid].tlb_gen) == - next_tlb_gen) + next_tlb_gen) { +#ifdef CONFIG_MEMBARRIER + /* + * We switched logical mm but we're not going to + * write to CR3. We already did smp_mb() above, + * but membarrier() might require a sync_core() + * as well. + */ + if (unlikely(atomic_read(&next->membarrier_state) & + MEMBARRIER_STATE_PRIVATE_EXPEDITED_SYNC_CORE)) + sync_core_before_usermode(); +#endif + return; + }
[...] I find that mixing up preprocessor #ifdef and code logic hurts readability. Can you lift this into a static function within the same compile unit, and provides an empty implementation for the #else case ? Thanks, Mathieu prev->sched_class->task_dead(prev); -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com