Re: restarting SYSCALL system call on amd64 loses arguments

2013-09-24 Thread Jilles Tjoelker
On Tue, Sep 24, 2013 at 10:29:09PM +0300, Konstantin Belousov wrote:
> On Tue, Sep 24, 2013 at 09:19:49PM +0200, Jilles Tjoelker wrote:
> > On Tue, Sep 24, 2013 at 12:37:30AM +0300, Konstantin Belousov wrote:
> > > On Mon, Sep 23, 2013 at 10:26:13PM +0200, Tijl Coosemans wrote:
> > > > Has anyone taken a look at this PR yet?

> > > > http://www.freebsd.org/cgi/query-pr.cgi?pr=182161

> > > This looks like a valid bug, but probably not a valid testcase.

> > > Let me elaborate.  When a signal is delivered, return from the signal
> > > handler is performed by the sigreturn(2), which reloads the whole
> > > register file when crossing kernel->user boundary due to sys_sigreturn(9)
> > > setting PCB_FULL_IRET flag.  As result, the whole trap frame at the
> > > time of the syscall entry is restored, and ERESTART return is not
> > > exercised.

> > > I was not able to reproduce the issue with the supplied test program
> > > on HEAD.  I suspect that the program actually exposed the bug in the
> > > signal delivery in the threaded processes, which I introduced for 9.1
> > > and fixed in r251047 & r251365.

> > The ERESTART return happens if there is no signal or no longer a signal.
> > The latter is how the bug in the PR occurs: a SIGCHLD delivery via
> > handler in one thread races with a SIGCHLD acceptance in wait4() in
> > another thread. Note wait4() returning a value in the other thread in
> > the fourth line of the kdump output in the PR.

> > For some reason, I can reproduce this easily on my local quad-core
> > r255729 stable/9 system but not on ref9-amd64.freebsd.org or
> > ref10-amd64.freebsd.org.

> > I can also reproduce the bug on my local system by racing signal
> > delivery via handler with acceptance in sigtimedwait().

> So, could you, please, check the r255844 on your machine ?

I cannot reproduce it with that (patch applied to stable/9 kernel). The
test programs run fine for minutes.

-- 
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: restarting SYSCALL system call on amd64 loses arguments

2013-09-24 Thread Konstantin Belousov
On Tue, Sep 24, 2013 at 09:19:49PM +0200, Jilles Tjoelker wrote:
> On Tue, Sep 24, 2013 at 12:37:30AM +0300, Konstantin Belousov wrote:
> > On Mon, Sep 23, 2013 at 10:26:13PM +0200, Tijl Coosemans wrote:
> > > Has anyone taken a look at this PR yet?
> 
> > > http://www.freebsd.org/cgi/query-pr.cgi?pr=182161
> 
> > This looks like a valid bug, but probably not a valid testcase.
> 
> > Let me elaborate.  When a signal is delivered, return from the signal
> > handler is performed by the sigreturn(2), which reloads the whole
> > register file when crossing kernel->user boundary due to sys_sigreturn(9)
> > setting PCB_FULL_IRET flag.  As result, the whole trap frame at the
> > time of the syscall entry is restored, and ERESTART return is not
> > exercised.
> 
> > I was not able to reproduce the issue with the supplied test program
> > on HEAD.  I suspect that the program actually exposed the bug in the
> > signal delivery in the threaded processes, which I introduced for 9.1
> > and fixed in r251047 & r251365.
> 
> The ERESTART return happens if there is no signal or no longer a signal.
> The latter is how the bug in the PR occurs: a SIGCHLD delivery via
> handler in one thread races with a SIGCHLD acceptance in wait4() in
> another thread. Note wait4() returning a value in the other thread in
> the fourth line of the kdump output in the PR.
> 
> For some reason, I can reproduce this easily on my local quad-core
> r255729 stable/9 system but not on ref9-amd64.freebsd.org or
> ref10-amd64.freebsd.org.
> 
> I can also reproduce the bug on my local system by racing signal
> delivery via handler with acceptance in sigtimedwait().

So, could you, please, check the r255844 on your machine ?


pgpcOoMcZXi28.pgp
Description: PGP signature


Re: restarting SYSCALL system call on amd64 loses arguments

2013-09-24 Thread Jilles Tjoelker
On Tue, Sep 24, 2013 at 12:37:30AM +0300, Konstantin Belousov wrote:
> On Mon, Sep 23, 2013 at 10:26:13PM +0200, Tijl Coosemans wrote:
> > Has anyone taken a look at this PR yet?

> > http://www.freebsd.org/cgi/query-pr.cgi?pr=182161

> This looks like a valid bug, but probably not a valid testcase.

> Let me elaborate.  When a signal is delivered, return from the signal
> handler is performed by the sigreturn(2), which reloads the whole
> register file when crossing kernel->user boundary due to sys_sigreturn(9)
> setting PCB_FULL_IRET flag.  As result, the whole trap frame at the
> time of the syscall entry is restored, and ERESTART return is not
> exercised.

> I was not able to reproduce the issue with the supplied test program
> on HEAD.  I suspect that the program actually exposed the bug in the
> signal delivery in the threaded processes, which I introduced for 9.1
> and fixed in r251047 & r251365.

The ERESTART return happens if there is no signal or no longer a signal.
The latter is how the bug in the PR occurs: a SIGCHLD delivery via
handler in one thread races with a SIGCHLD acceptance in wait4() in
another thread. Note wait4() returning a value in the other thread in
the fourth line of the kdump output in the PR.

For some reason, I can reproduce this easily on my local quad-core
r255729 stable/9 system but not on ref9-amd64.freebsd.org or
ref10-amd64.freebsd.org.

I can also reproduce the bug on my local system by racing signal
delivery via handler with acceptance in sigtimedwait().

-- 
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: restarting SYSCALL system call on amd64 loses arguments

2013-09-24 Thread Russ Cox
I don't have a machine at hand that I can rebuild the kernel on, but I
agree that setting PCB_FULL_IRET should fix the problem.

Russ
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: restarting SYSCALL system call on amd64 loses arguments

2013-09-23 Thread Konstantin Belousov
On Mon, Sep 23, 2013 at 10:26:13PM +0200, Tijl Coosemans wrote:
> Has anyone taken a look at this PR yet?
> 
> http://www.freebsd.org/cgi/query-pr.cgi?pr=182161

This looks like a valid bug, but probably not a valid testcase.

Let me elaborate.  When a signal is delivered, return from the signal
handler is performed by the sigreturn(2), which reloads the whole
register file when crossing kernel->user boundary due to sys_sigreturn(9)
setting PCB_FULL_IRET flag.  As result, the whole trap frame at the
time of the syscall entry is restored, and ERESTART return is not
exercised.

I was not able to reproduce the issue with the supplied test program
on HEAD.  I suspect that the program actually exposed the bug in the
signal delivery in the threaded processes, which I introduced for 9.1
and fixed in r251047 & r251365.

On the other hand, I agree that at least arguments should be restored
for ERESTART case, but in fact it is easier and probably less error-prone
to restore whole register file again.  Please try the patch below for
your Go code.

diff --git a/sys/amd64/amd64/vm_machdep.c b/sys/amd64/amd64/vm_machdep.c
index b7c2b67..1e3d8f5 100644
--- a/sys/amd64/amd64/vm_machdep.c
+++ b/sys/amd64/amd64/vm_machdep.c
@@ -400,9 +400,13 @@ cpu_set_syscall_retval(struct thread *td, int error)
 * for the next iteration.
 * %r10 restore is only required for freebsd/amd64 processes,
 * but shall be innocent for any ia32 ABI.
+*
+* Require full context restore to get the arguments
+* in the registers reloaded at return to usermode.
 */
td->td_frame->tf_rip -= td->td_frame->tf_err;
td->td_frame->tf_r10 = td->td_frame->tf_rcx;
+   set_pcb_flags(td->td_pcb, PCB_FULL_IRET);
break;
 
case EJUSTRETURN:


pgpnbv9LS1IxZ.pgp
Description: PGP signature


restarting SYSCALL system call on amd64 loses arguments

2013-09-23 Thread Tijl Coosemans
Has anyone taken a look at this PR yet?

http://www.freebsd.org/cgi/query-pr.cgi?pr=182161


signature.asc
Description: PGP signature