On Wed, Feb 13, 2019 at 10:51:24AM -0800, Andy Lutomirski wrote: > > On Feb 13, 2019, at 7:45 AM, Peter Zijlstra <[email protected]> wrote:
> > Which I suppose means that GCC generates the PUSHF/POPF to preserve the > > EFLAGS, since we mark those explicitly clobbered. > > > > Not quite. A flags clobber doesn’t save the control bits like AC > except on certain rather buggy llvm compilers. The change you’re > looking for is: > > http://git.kernel.org/tip/2c7577a7583747c9b71f26dced7f696b739da745 Indeed, failed to find that. > > For a little bit of context; it turns out that user_access_begin() / > > user_access_end() sets EFLAGS.AC and scheduling in between there wrecks > > that because we're apparently not saving that anymore. > > But only explicit scheduling — preemption and sleepy page faults are > fine because the interrupt frame saves flags. No, like pointed out elsewhere in this thread, anything that does preempt_disable() is utterly broken with this. Because at that point the IRQ return path doesn't reschedule but preempt_enable() will, and that doesn't preserve EFLAGS again. > > Now, I'm tempted to add the PUSHF / POPF right back because of this, but > > first I suppose we need to figure out if that change was on purpose and > > why that went missing from the Changelog. > > That’s certainly the easy solution. Or we could teach the might_sleep > checks about this, but that could be a mess. That's not enough, we'd have to teach preempt_disable(), but worse, preempt_disable_notrace(). Anything that lands in ftrace, which _will_ use preempt_disable_notrace(), will screw this thing up.
