code review 11266043: runtime: switch to os stack in windows osyield and usleep

runtime: switch to os stack in windows osyield and usleep

Fixes issue 5831

[brainman, 2013-07-16 01:23:33]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[rsc, 2013-07-16 01:45:39]

LGTM

https://codereview.appspot.com/11266043/diff/6001/src/pkg/runtime/os_windows.c
File src/pkg/runtime/os_windows.c (right):

https://codereview.appspot.com/11266043/diff/6001/src/pkg/runtime/os_windows....
src/pkg/runtime/os_windows.c:282: runtime·osyield()
s/()/(void)/

https://codereview.appspot.com/11266043/diff/6001/src/pkg/runtime/sys_windows...
File src/pkg/runtime/sys_windows_386.s (right):

https://codereview.appspot.com/11266043/diff/6001/src/pkg/runtime/sys_windows...
src/pkg/runtime/sys_windows_386.s:340: // m is not set, do not switch stack.
// Not a Go-managed thread. Do not switch stack.

https://codereview.appspot.com/11266043/diff/6001/src/pkg/runtime/sys_windows...
File src/pkg/runtime/sys_windows_amd64.s (right):

https://codereview.appspot.com/11266043/diff/6001/src/pkg/runtime/sys_windows...
src/pkg/runtime/sys_windows_amd64.s:335: // m is not set, do not switch stack.
// Not a Go-managed thread. Do not switch stack.

[brainman, 2013-07-16 02:36:13]

*** Submitted as https://code.google.com/p/go/source/detail?r=40754028598e ***

runtime: switch to os stack in windows osyield and usleep

Fixes issue 5831

R=golang-dev, rsc
CC=golang-dev
https://codereview.appspot.com/11266043

https://codereview.appspot.com/11266043/diff/6001/src/pkg/runtime/os_windows.c
File src/pkg/runtime/os_windows.c (right):

https://codereview.appspot.com/11266043/diff/6001/src/pkg/runtime/os_windows....
src/pkg/runtime/os_windows.c:282: runtime·osyield()
On 2013/07/16 01:45:39, rsc wrote:
> s/()/(void)/

Done.

https://codereview.appspot.com/11266043/diff/6001/src/pkg/runtime/sys_windows...
File src/pkg/runtime/sys_windows_386.s (right):

https://codereview.appspot.com/11266043/diff/6001/src/pkg/runtime/sys_windows...
src/pkg/runtime/sys_windows_386.s:340: // m is not set, do not switch stack.
On 2013/07/16 01:45:39, rsc wrote:
> // Not a Go-managed thread. Do not switch stack.

Done.

https://codereview.appspot.com/11266043/diff/6001/src/pkg/runtime/sys_windows...
File src/pkg/runtime/sys_windows_amd64.s (right):

https://codereview.appspot.com/11266043/diff/6001/src/pkg/runtime/sys_windows...
src/pkg/runtime/sys_windows_amd64.s:335: // m is not set, do not switch stack.
On 2013/07/16 01:45:39, rsc wrote:
> // Not a Go-managed thread. Do not switch stack.

Done.

[dvyukov, 2013-07-16 05:39:59]

I afraid many more functions need this:
https://code.google.com/p/go/issues/detail?id=5831#c10

[brainman, 2013-07-16 05:55:58]

On 2013/07/16 05:39:59, dvyukov wrote:
> I afraid many more functions need this:

Every other call to windows dll functions is made via runtime.stdcall.
runtime.stdcall does switches stacks. So I think we are safe.

We don't use runtime.stdcall for osyield and usleep, because runtime.stdcall
uses quite a bit of stack when it runs. osyield and usleep are called by few
stack-no-split functions, so the stack space is at premium here. Now, after this
CL, even osyield and usleep are covered.

Perhaps I am wrong. You tell me.

Alex

[dvyukov, 2013-07-16 06:23:45]

On 2013/07/16 05:55:58, brainman wrote:
> On 2013/07/16 05:39:59, dvyukov wrote:
> > I afraid many more functions need this:
> 
> Every other call to windows dll functions is made via runtime.stdcall.
> runtime.stdcall does switches stacks. So I think we are safe.
> 
> We don't use runtime.stdcall for osyield and usleep, because runtime.stdcall
> uses quite a bit of stack when it runs. osyield and usleep are called by few
> stack-no-split functions, so the stack space is at premium here. Now, after
this
> CL, even osyield and usleep are covered.
> 
> Perhaps I am wrong. You tell me.

Ah, OK, I did not know that we switch stack for all other syscalls.