* Andy Lutomirski <l...@kernel.org> wrote:

> This is incomplete, but it's finally good enough that I think it's
> time to get other opinions on it.  It is a complete rewrite of the
> slow path code that handles exits to user mode.

Modulo the small comments I made about the debug checks interface plus naming 
details the structure and intention of this series gives me warm fuzzy feelings.

> The exit-to-usermode code is copied in several places and is written in a 
> nasty 
> combination of asm and C.  It's not at all clear what it's supposed to do, 
> and 
> the way it's structured makes it very hard to work with.  For example, it's 
> not 
> even clear why syscall exit hooks are called only once per syscall right now. 
>  
> (It seems to be a side effect of the way that rdi and rdx are handled in the 
> asm 
> loop, and it seems reliable, but it's still pointlessly complicated.)  The 
> existing code also makes context tracking overly complicated and hard to 
> understand.  Finally, it's nearly impossible for anyone to change what 
> happens 
> on exit to usermode, since the existing code is so fragile.

Amen.

> I tried to clean it up incrementally, but I decided it was too hard. Instead, 
> this series just replaces the code.  It seems to work.

Any known bugs beyond UML build breakage?

> Context tracking in particular works very differently now.  The low-level 
> entry 
> code checks that we're in CONTEXT_USER and switches to CONTEXT_KERNEL.  The 
> exit 
> code does the reverse.  There is no need to track what CONTEXT_XYZ state we 
> came 
> from, because we already know.  Similarly, SCHEDULE_USER is gone, since we 
> can 
> reschedule if needed by simply calling schedule() from C code.
> 
> The main things that are missing are that I haven't done the 32-bit parts 
> (anyone want to help?) and therefore I haven't deleted the old C code.  I 
> also 
> think this may break UML for trivial reasons.
> 
> Because I haven't converted the 32-bit code yet, all of the now-unnecessary 
> unnecessary calls to exception_enter are still present in traps.c.
> 
> IRQ context tracking is still duplicated.  We should probably clean it up by 
> changing the core code to supply something like 
> irq_enter_we_are_already_in_context_kernel.
> 
> Thoughts?

So assuming you fix the UML build I'm inclined to go for it, even in this 
incomplete form, to increase testing coverage.

Doing that will also decrease ongoing merge friction between your work and 
other 
entry code cleanups ...

Thanks,

        Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to