On Tue, 11 Feb 2025 at 21:21, Rik van Riel <riel@xxxxxxxxxxx> wrote:
>
> On Tue, 2025-02-11 at 11:02 +0100, Brendan Jackman wrote:
> >
> > So I think here we're encoding the assumption that context_switch()
> > always calls either enter_lazy_tlb() or switch_mm_irqs_off(), which
> > is
> > a little awkward, plus the job of these functions is already kinda
> > hazy and this makes it even hazier. What about doing it in
> > arch_start_context_switch() instead?
> >
> > That would mean a bit of plumbing since we'd still wanna have the
> > tlbsync() in tlb.c, but that seems worth it to me. Plus, having it in
> > one place would give us a spot to add a comment. Now that you point
> > it
> > out it does indeed seem obvious but it didn't seem so yesterday.
> >
> While that would be a little bit cleaner to maintain,
> in theory, I'm not convinced that adding an extra
> function call to the context switch path is worthwhile
> for that small maintenance benefit.
I don't see why it has to introduce a function call, can't we just
have something like:
static inline void arch_start_context_switch(struct task_struct *prev)
{
arch_paravirt_start_context_switch(prev);
tlb_start_context_switch(prev);
}
The paravirt one would disappear when !CONFIG_PARAVIRT (as the current
arch_start_context_switch() does) and the tlb one would disappear when
!CONFIG_X86_BROADCAST_TLB_FLUSH. The whole thing should be inlinable.
