From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk0-f200.google.com (mail-qk0-f200.google.com [209.85.220.200]) by kanga.kvack.org (Postfix) with ESMTP id 572526B02C3 for ; Fri, 26 May 2017 09:25:33 -0400 (EDT) Received: by mail-qk0-f200.google.com with SMTP id g39so3046607qkh.3 for ; Fri, 26 May 2017 06:25:33 -0700 (PDT) Received: from mx1.redhat.com (mx1.redhat.com. [209.132.183.28]) by mx.google.com with ESMTPS id j123si726132qkc.268.2017.05.26.06.25.31 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 26 May 2017 06:25:32 -0700 (PDT) Message-ID: <1495805128.29205.64.camel@redhat.com> Subject: Re: [PATCH v3 2/8] x86/mm: Change the leave_mm() condition for local TLB flushes From: Rik van Riel Date: Fri, 26 May 2017 09:25:28 -0400 In-Reply-To: References: <61de238db6d9c9018db020c41047ce32dac64488.1495759610.git.luto@kernel.org> <1495762747.29205.63.camel@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: owner-linux-mm@kvack.org List-ID: To: Andy Lutomirski Cc: X86 ML , "linux-kernel@vger.kernel.org" , Borislav Petkov , Linus Torvalds , Andrew Morton , Mel Gorman , "linux-mm@kvack.org" , Nadav Amit , Dave Hansen , Nadav Amit , Michal Hocko , Arjan van de Ven On Thu, 2017-05-25 at 19:01 -0700, Andy Lutomirski wrote: > On Thu, May 25, 2017 at 6:39 PM, Rik van Riel > wrote: > > On Thu, 2017-05-25 at 17:47 -0700, Andy Lutomirski wrote: > > > > > > +++ b/arch/x86/mm/tlb.c > > > @@ -311,7 +311,7 @@ void flush_tlb_mm_range(struct mm_struct *mm, > > > unsigned long start, > > > A A A A A A A A A A A A A A goto out; > > > A A A A A A } > > > > > > -A A A A A if (!current->mm) { > > > +A A A A A if (this_cpu_read(cpu_tlbstate.state) != TLBSTATE_OK) { > > > A A A A A A A A A A A A A A leave_mm(smp_processor_id()); > > > > Unless -mm changed leave_mm (I did not check), this > > is not quite correct yet. > > > > The reason is leave_mm (at least in the latest Linus > > tree) ignores the cpu argument for one of its checks. > > > > You should probably fix that in an earlier patch, > > assuming you haven't already done so in -mm. > > > > void leave_mm(int cpu) > > { > > A A A A A A A A struct mm_struct *active_mm = > > this_cpu_read(cpu_tlbstate.active_mm); > > A A A A A A A A if (this_cpu_read(cpu_tlbstate.state) == TLBSTATE_OK) > > A A A A A A A A A A A A A A A A BUG(); > > A A A A A A A A if (cpumask_test_cpu(cpu, mm_cpumask(active_mm))) { > > A A A A A A A A A A A A A A A A cpumask_clear_cpu(cpu, mm_cpumask(active_mm)); > > A A A A A A A A A A A A A A A A load_cr3(swapper_pg_dir); > > I agree it's odd, but what's the bug?A A Both before and after, > leave_mm > needed to be called with cpu == smp_processor_id(), and > smp_processor_id() warns if it's called in a preemptible context. Indeed, you are right. Looking at too much code at once... -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org