On Mon, Aug 27, 2018 at 6:31 PM, Rik van Riel riel@surriel.com wrote:
On Mon, 2018-08-27 at 16:04 -0700, Andy Lutomirski wrote:
+++ b/arch/x86/mm/tlb.c @@ -345,6 +345,9 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next, */ trace_tlb_flush_rcuidle(TLB_FLUSH_ON_TASK_SWITCH, TLB_FLUSH_ALL); } else {
/* Let NMI code know that CR3 may not matchexpectations. */
I don't get it. This is in the "ASID is up to date, do not need a TLB flush" path.
In what case do we have a TLB that is fully up to date, but a CR3 that does not match expectations?
Doesn't the CR3 check in nmi_uaccess_ok already catch the window of time where the CR3 has already been switched over to that of the next task?
What is special about this path wrt nmi_uaccess_ok that is not also true for the need_flush branch right above it?
What am I missing?
Nothing. My patch is buggy. ETOLITTLESLEEP.
I could drop this part of the patch entirely. Or I could drop the loaded_mm->pgd == __va(read_cr3_pa() check and instead make sure that loaded_mm is NULL at any point at which loaded_mm might not match CR3. The latter will be faster in any (hypothetical) virtualization environment where CR3 reads trap. I don't know if we have any such cases where perf works and we care about performance, though.
--Andy