code review 6197046: pkg/runtime: Fix semasleep on Plan 9

pkg/runtime: Fix semasleep on Plan 9

With the timed semacquire patch
(kernel-tsemacquire) for Plan 9,
we can now properly do a timed
wait for the semaphore, in
semasleep.

[akumar, 2012-05-04 23:34:40]

Hello golang-dev@googlegroups.com, rsc@golang.org, rminnich@gmail.com,
ality@pbrane.org (cc: 0intro@gmail.com, golang-dev@googlegroups.com,
john@jfloren.net, mirtchovski@gmail.com),

I'd like you to review this change to
https://code.google.com/p/go

[rminnich, 2012-05-05 00:50:42]

LGTM

[ality, 2012-05-05 03:51:37]

https://codereview.appspot.com/6197046/diff/10001/src/pkg/runtime/sys_plan9_3...
File src/pkg/runtime/sys_plan9_386.s (right):

https://codereview.appspot.com/6197046/diff/10001/src/pkg/runtime/sys_plan9_3...
src/pkg/runtime/sys_plan9_386.s:53: MOVL	$52, AX
It's unfortunate that this collides with
semsleep on Nix. Maybe we should ask them
to shift their syscall numbers up and
leave a bit of space in between.

I have a feeling they're not going to like
that though.

https://codereview.appspot.com/6197046/diff/10001/src/pkg/runtime/thread_plan9.c
File src/pkg/runtime/thread_plan9.c (right):

https://codereview.appspot.com/6197046/diff/10001/src/pkg/runtime/thread_plan...
src/pkg/runtime/thread_plan9.c:279: if(ns/1000000 > 0x7fffffffll)
Please fix the mixed tabs and spaces.

https://codereview.appspot.com/6197046/diff/10001/src/pkg/runtime/thread_plan...
src/pkg/runtime/thread_plan9.c:289: ret =
runtime·plan9_semacquire(&m->waitsemacount, 1);
Revert this change. The comment at the top of
src/pkg/runtime/lock_sema.c says we can only
return 0 if ns < 0 so we have to loop here if
the syscall is interrupted.

[akumar, 2012-05-05 06:35:56]

PTAL.

(Sorry, this CL was made in haste.)

https://codereview.appspot.com/6197046/diff/10001/src/pkg/runtime/sys_plan9_3...
File src/pkg/runtime/sys_plan9_386.s (right):

https://codereview.appspot.com/6197046/diff/10001/src/pkg/runtime/sys_plan9_3...
src/pkg/runtime/sys_plan9_386.s:53: MOVL	$52, AX
On 2012/05/05 03:51:37, ality wrote:
> It's unfortunate that this collides with
> semsleep on Nix. Maybe we should ask them
> to shift their syscall numbers up and
> leave a bit of space in between.
> 
> I have a feeling they're not going to like
> that though.

Not to worry, that will be cleared
up.

https://codereview.appspot.com/6197046/diff/10001/src/pkg/runtime/thread_plan9.c
File src/pkg/runtime/thread_plan9.c (right):

https://codereview.appspot.com/6197046/diff/10001/src/pkg/runtime/thread_plan...
src/pkg/runtime/thread_plan9.c:279: if(ns/1000000 > 0x7fffffffll)
On 2012/05/05 03:51:37, ality wrote:
> Please fix the mixed tabs and spaces.

Done.

https://codereview.appspot.com/6197046/diff/10001/src/pkg/runtime/thread_plan...
src/pkg/runtime/thread_plan9.c:289: ret =
runtime·plan9_semacquire(&m->waitsemacount, 1);
On 2012/05/05 03:51:37, ality wrote:
> Revert this change. The comment at the top of
> src/pkg/runtime/lock_sema.c says we can only
> return 0 if ns < 0 so we have to loop here if
> the syscall is interrupted.

Done.

I feel the note should be
mentioned, in any case.

[r, 2012-05-16 18:27:31]

Is it reasonable to check in a change that requires a kernel update? If users
are likely to have or be willing to install the patch, then fine. I cannot judge
this.

[rminnich, 2012-05-16 18:40:45]

On Wed, May 16, 2012 at 11:27 AM,  <r@golang.org> wrote:
> Is it reasonable to check in a change that requires a kernel update? If
> users are likely to have or be willing to install the patch, then fine.

The kernel update is being done at Murray Hill. There was no other
reasonable way to make this work.

ron

[r, 2012-05-16 22:08:01]

LGTM

[r, 2012-05-16 22:10:05]

*** Submitted as http://code.google.com/p/go/source/detail?r=d9a9bdd06284 ***

pkg/runtime: Fix semasleep on Plan 9

With the timed semacquire patch
(kernel-tsemacquire) for Plan 9,
we can now properly do a timed
wait for the semaphore, in
semasleep.

R=golang-dev, rsc, rminnich, ality, r
CC=0intro, golang-dev, john, mirtchovski
http://codereview.appspot.com/6197046

Committer: Rob Pike <r@golang.org>