From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 1/4] x86: move syscall trampolines off the stack Date: Wed, 20 May 2015 14:58:28 +0100 Message-ID: <555C9304.2080702@citrix.com> References: <5559DAFF020000780007AFE2@mail.emea.novell.com> <5559FB2E020000780007B196@mail.emea.novell.com> <555B6BE7.508@citrix.com> <555CAA3F020000780007C38E@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Yv4W3-0001c1-3p for xen-devel@lists.xenproject.org; Wed, 20 May 2015 13:58:35 +0000 In-Reply-To: <555CAA3F020000780007C38E@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel , Keir Fraser List-Id: xen-devel@lists.xenproject.org On 20/05/15 14:37, Jan Beulich wrote: >>>> On 19.05.15 at 18:59, wrote: >> On 18/05/15 13:46, Jan Beulich wrote: >>> @@ -616,6 +653,24 @@ static void cpu_smpboot_free(unsigned in >>> free_cpumask_var(per_cpu(cpu_sibling_mask, cpu)); >>> free_cpumask_var(per_cpu(cpu_core_mask, cpu)); >>> >>> + if ( per_cpu(stubs.addr, cpu) ) >>> + { >>> + unsigned long mfn = per_cpu(stubs.mfn, cpu); >>> + unsigned char *stub_page = map_domain_page(mfn); >>> + unsigned int i; >>> + >>> + memset(stub_page + (cpu & (STUBS_PER_PAGE - 1)) * STUB_BUF_SIZE, >>> + 0xcc, STUB_BUF_SIZE); >>> + for ( i = 0; i < STUBS_PER_PAGE; ++i ) >>> + if ( stub_page[i * STUB_BUF_SIZE] != 0xcc) >>> + break; >> There is a race condition between allocate and free, as >> write_stub_trampoline() is written by the cpu itself. Just finding 0xcc >> here doesn't mean there isn't a different cpu in the process of coming >> up and intending to use the stub page. > No, there is no race afaict: percpu_traps_init() gets called before > a CPU is marked online and hence before __cpu_up() returns, and > thus before the CPU hotplug lock gets dropped, so there can't be > any CPUs going down concurrently. Thinking out loud... cpu_up/down have excusion due to the recursive spinlock used by cpu_hotplug_begin/end. The allocate is done by the CPU_UP_PREPARE notifier chain, and __cpu_up() does wait until the AP has marked itself as online. Therefore, so long as no CPU_UP_PREPARE action triggers a CPU_UP_CANCELED or CPU_DEAD chain against the same cpu, we are safe. ~Andrew