[PATCH v2 5/7] ARM: switch_to: clean up Thumb2 code path

[Nicolas Pitre]

On Mon, 18 Oct 2021, Ard Biesheuvel wrote:

> On Mon, 18 Oct 2021 at 17:37, Nicolas Pitre <nico at fluxnic.net> wrote:
> >
> > On Mon, 18 Oct 2021, Ard Biesheuvel wrote:
> >
> > > The load-multiple instruction that essentially performs the switch_to
> > > operation in ARM mode, by loading all callee save registers as well the
> > > stack pointer and the program counter, is split into 3 separate loads
> > > for Thumb-2, with the IP register used as a temporary to capture the
> > > value of R4 before it gets overwritten.
> > >
> > > We can clean this up a bit, by sticking with a single LDMIA instruction,
> > > but one that pops SP and PC into IP and LR, respectively, and by using
> > > ordinary move register and branch instructions to get those values into
> > > SP and PC. This also allows us to move the set_current call closer to
> > > the assignment of SP, reducing the window where those are mutually out
> > > of sync. This is especially relevant for CONFIG_VMAP_STACK, which is
> > > being introduced in a subsequent patch, where we need to issue a load
> > > that might fault from the new stack while running from the old one, to
> > > ensure that stale PMD entries in the VMALLOC space are synced up.
> >
> > What could happen if they are of sync? The window is small but still.
> >
> 
> It would mean 'current' as well as 'current_thread_info' would point
> to the new task while running on the old stack, whereas without
> THREAD_INFO_IN_TASK, those are guaranteed to be in sync.
> 
> I think it probably doesn't make lots of difference, although after
> the next patch, we may take a fault in this code path on a stale PMD
> entry, and I suppose the accounting etc would be affected.
> 
> In any case, this is something Russell brought up in the review of the
> THREAD_INFO_IN_TASK series, and I agree that it's best to keep these
> in sync as much as we can.

If this happens only once in a blue moon and the consequence is benign 
(like wrong accounting for one perf sample or the like) then this is 
probably OK. However it would be a good idea to document it in the code 
and not only in the commit log.


Nicolas