Messages in this thread Patch in this message | | | Date | Wed, 16 Jun 2021 13:49:12 -0400 (EDT) | From | Mathieu Desnoyers <> | Subject | Re: [PATCH 2/8] x86/mm: Handle unlazying membarrier core sync in the arch code |
| |
----- On Jun 15, 2021, at 11:21 PM, Andy Lutomirski luto@kernel.org wrote: [...] > @@ -473,16 +474,24 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct > mm_struct *next,
[...]
> @@ -510,16 +520,35 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct > mm_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
| |