Messages in this thread | | | Date | Mon, 27 Sep 2010 20:26:10 +0400 | From | Ivan Kokshaysky <> | Subject | Re: alpha: potential race around hae_cache in RESTORE_ALL |
| |
On Mon, Sep 27, 2010 at 01:46:24PM +0100, Al Viro wrote: > AFAICS, we have 3 variants: > 1) alpha_mv.hae_register == &alpha_mv.hae_cache; all that code > becomes a no-op. > 2) UP boxen with hae_register pointing someplace real; we save > HAE in SAVE_ALL, restore it in RESTORE_ALL and disable interrupts around > the updates of hae_cache/*hae_register to keep them in sync. readl() > et.al. set HAE, then do memory access and rely on not giving CPU up between > these moments. Since alpha doesn't do PREEMPT, we are OK (otherwise we'd > needed to disable preempt in those places; also not a big deal) > 3) SMP t2 boxen; we protect the entire sequence from setting HAE to > memory access with spinlock and with disabling interrupts. We don't rely on > interrupts not modifying the damn thing, but we *do* rely on other CPU not > messing with HAE on syscall paths outside of spinlock-protected area. And > we have RESTORE_ALL hit us on all exits to userland, interrupt, trap and > syscall alike. > > Looks like (3) has always been broken...
Ah, agreed with all of the above.
Looks like we need to drop HAE bits from SAVE_ALL/RESTORE_ALL, which benefits (1) and automatically fixes (3), and do the entire IO sequences in (2) with disabled interrupts (if HAE is involved). The latter includes apecs, lca and jensen.
Ivan.
| |