Re: svn commit: r220430 - head/sys/amd64/amd64

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)
_at__at_ -382,10 +382,10 _at__at_
  	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)
_at__at_ -661,7 +661,7 _at__at_
  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
_at__at_ -682,8 +682,7 _at__at_
  	 */
  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)
_at__at_ -46,7 +46,7 _at__at_
  	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