From owner-freebsd-current@FreeBSD.ORG Fri Apr 8 13:09:00 2011 Return-Path: Delivered-To: freebsd-current@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 328301065670; Fri, 8 Apr 2011 13:09:00 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id DE30E8FC18; Fri, 8 Apr 2011 13:08:59 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 77E8646B49; Fri, 8 Apr 2011 09:08:59 -0400 (EDT) Received: from jhbmac.hudson-trading.com (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id D09AC8A027; Fri, 8 Apr 2011 09:08:58 -0400 (EDT) Message-ID: <4D9F08EA.9090209@FreeBSD.org> Date: Fri, 08 Apr 2011 09:08:58 -0400 From: John Baldwin User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.15) Gecko/20110303 Thunderbird/3.1.9 MIME-Version: 1.0 To: Andriy Gapon References: <201104072132.p37LWPuu052536@svn.freebsd.org> <4D9EE221.40200@FreeBSD.org> In-Reply-To: <4D9EE221.40200@FreeBSD.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (bigwig.baldwin.cx); Fri, 08 Apr 2011 09:08:59 -0400 (EDT) Cc: svn-src-head@FreeBSD.org, FreeBSD-Current , svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r220430 - head/sys/amd64/amd64 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Apr 2011 13:09:00 -0000 On 4/8/11 6:23 AM, Andriy Gapon wrote: > on 08/04/2011 00:32 John Baldwin said the following: >> Author: jhb >> Date: Thu Apr 7 21:32:25 2011 >> New Revision: 220430 >> URL: http://svn.freebsd.org/changeset/base/220430 >> >> Log: >> If a system call does not request a full interrupt return, use a fast >> path via the sysretq instruction to return from the system call. This was >> removed in 190620 and not quite fully restored in 195486. This resolves >> most of the performance regression in system call microbenchmarks between >> 7 and 8 on amd64. >> >> Reviewed by: kib >> MFC after: 1 week > > I think that this commit (plus r220431) has broken something in my environment. > After updating to the most recent head I started to get semi-random problems in > various areas: > - named would consistently fail to start, but with different errors (assertions) > - ^Z and fg result in a process getting SIGSEGV > - X sometimes fails to start complaining about failed VT switch > > Reverting just these two commits restores sanity. > > Just in case, my processor is AMD (arch is obviously amd64). I think I've found this (I got a bunch of weird core dumps overnight, too). The problem is that ast() can context switch in which case PCB_FULL_IRET might get set, but this code would still do the fast path after ast() returned. I fixed it to recheck the PCB_FULL_IRET flag after returning from ast() and that has fixed the core dumps I was seeing overnight. I also fixed a bug where PCB_FULL_IRET wasn't updated in some of the ia32 compat code, a typo in a comment, and trimmed an extra mov from the doreti path: Index: amd64/exception.S =================================================================== --- amd64/exception.S (revision 221092) +++ amd64/exception.S (working copy) @@ -382,10 +382,10 @@ FAKE_MCOUNT(TF_RIP(%rsp)) movq %rsp,%rdi call syscall - movq PCPU(CURPCB),%rax +1: movq PCPU(CURPCB),%rax testl $PCB_FULL_IRET,PCB_FLAGS(%rax) - jne 3f -1: /* Check for and handle AST's on return to userland. */ + jnz 3f + /* Check for and handle AST's on return to userland. */ cli movq PCPU(CURTHREAD),%rax testl $TDF_ASTPENDING | TDF_NEEDRESCHED,TD_FLAGS(%rax) @@ -661,7 +661,7 @@ doreti_ast: /* * Check for ASTs atomically with returning. Disabling CPU - * interrupts provides sufficient locking eve in the SMP case, + * interrupts provides sufficient locking even in the SMP case, * since we will be informed of any new ASTs by an IPI. */ cli @@ -682,8 +682,7 @@ */ doreti_exit: MEXITCOUNT - movq PCPU(CURTHREAD),%r8 - movq TD_PCB(%r8),%r8 + movq PCPU(CURPCB),%r8 /* * Do not reload segment registers for kernel. Index: ia32/ia32_exception.S =================================================================== --- ia32/ia32_exception.S (revision 221079) +++ ia32/ia32_exception.S (working copy) @@ -46,7 +46,7 @@ subq $TF_ERR,%rsp /* skip over tf_trapno */ movq %rdi,TF_RDI(%rsp) movq PCPU(CURPCB),%rdi - movb $0,PCB_FULL_IRET(%rdi) + andl $~PCB_FULL_IRET,PCB_FLAGS(%rdi) movw %fs,TF_FS(%rsp) movw %gs,TF_GS(%rsp) movw %es,TF_ES(%rsp) -- John Baldwin