code review 112640043: runtime: don't lock mheap on user goroutine

runtime: don't lock mheap on user goroutine
This is bad for 2 reasons:
1. if the code under lock ever grows stack,
it will deadlock as stack growing acquires mheap lock.
2. It currently deadlocks with SetCPUProfileRate:
scavenger locks mheap, receives prof signal and tries to lock prof lock;
meanwhile SetCPUProfileRate locks prof lock and tries to grow stack
(presumably in runtime.unlock->futexwakeup). Boom.
Let's assume that it
Fixes issue 8407.

[dvyukov, 2014-07-23 13:20:50]

Hello golang-codereviews@googlegroups.com (cc: khr@golang.org, rsc@golang.org),

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

[rsc, 2014-07-23 13:39:09]

LGTM

Note that this introduces a race with the GC-forcing code. It might trigger an
extra GC once in a while. It shouldn't matter.

[dvyukov, 2014-07-23 13:46:39]

On Wed, Jul 23, 2014 at 5:39 PM,  <rsc@golang.org> wrote:
> LGTM
>
> Note that this introduces a race with the GC-forcing code. It might
> trigger an extra GC once in a while. It shouldn't matter.


mheap lock does not protect last_gc time, so it is the same way now
the code would need to take worldsema, but then you can't call GC
holding worldsema; so we would need to modify runtime.GC to acquire
worldsema but then do GC iff it was not done for 5 mins
let it be racy :)

[dvyukov, 2014-07-23 14:52:34]

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

runtime: don't lock mheap on user goroutine
This is bad for 2 reasons:
1. if the code under lock ever grows stack,
it will deadlock as stack growing acquires mheap lock.
2. It currently deadlocks with SetCPUProfileRate:
scavenger locks mheap, receives prof signal and tries to lock prof lock;
meanwhile SetCPUProfileRate locks prof lock and tries to grow stack
(presumably in runtime.unlock->futexwakeup). Boom.
Let's assume that it
Fixes issue 8407.

LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews, khr
https://codereview.appspot.com/112640043

[gobot, 2014-07-23 16:03:19]

This CL appears to have broken the linux-arm-cheney-panda builder.
See http://build.golang.org/log/397b6a0bf90f2ba2158a77389dffab7a820e8dfd

[minux, 2014-07-23 20:30:11]

+dfc.

On Wed, Jul 23, 2014 at 12:03 PM, <gobot@golang.org> wrote:

> This CL appears to have broken the linux-arm-cheney-panda builder.
> See http://build.golang.org/log/397b6a0bf90f2ba2158a77389dffab7a820e8dfd

Seems real. This builder has been a little flaky recently. Could be a
memory corruption
that usually happens on pandaboards?

# GOMAXPROCS=2 runtime -cpu=1,2,4
runtime: newstack called from g=0x753e0
m=0xb64fed80 m->curg=0xbe96000 m->g0=0x0 m->gsignal=0x2000
panic during panic

runtime stack:
runtime.startpanic()
/usr/local/go/src/pkg/runtime/panic.c:408 +0xc4
runtime.throw(0x52748a)
 /usr/local/go/src/pkg/runtime/panic.c:520 +0x3c
stopm()
/usr/local/go/src/pkg/runtime/proc.c:956 +0x3c
exitsyscall0(0x107392c0)
/usr/local/go/src/pkg/runtime/proc.c:1728 +0xc4
runtime.mcall(0x10712500)
/usr/local/go/src/pkg/runtime/asm_arm.s:172 +0x50

[dave_cheney.net, 2014-07-23 22:52:32]

On Thu, Jul 24, 2014 at 6:29 AM, minux <minux@golang.org> wrote:

> +dfc.
>
> On Wed, Jul 23, 2014 at 12:03 PM, <gobot@golang.org> wrote:
>
>> This CL appears to have broken the linux-arm-cheney-panda builder.
>> See http://build.golang.org/log/397b6a0bf90f2ba2158a77389dffab7a820e8dfd
>
> Seems real. This builder has been a little flaky recently. Could be a
> memory corruption
> that usually happens on pandaboards?
>

Yes, this is true, this is a pandaboard. I'm not seeing similar failures on
my other arm builder, the -imx6 one which runs linux/arm and nacl/arm.

I've rebooted the pandaboard builder, maybe that will help.

[dvyukov, 2014-07-24 07:17:19]

This change is quite harmless in itself. But on darwin it exposed a
very strange scheduling behavior on runtime tests that was not
happening ever before:
https://code.google.com/p/go/source/detail?r=7c3b57e843c665b59ff58b604b21d632...
I would not be surprised if it exposes some other latent bug as well.





On Thu, Jul 24, 2014 at 2:52 AM, Dave Cheney <dave@cheney.net> wrote:
>
>
>
> On Thu, Jul 24, 2014 at 6:29 AM, minux <minux@golang.org> wrote:
>>
>> +dfc.
>>
>> On Wed, Jul 23, 2014 at 12:03 PM, <gobot@golang.org> wrote:
>>>
>>> This CL appears to have broken the linux-arm-cheney-panda builder.
>>> See http://build.golang.org/log/397b6a0bf90f2ba2158a77389dffab7a820e8dfd
>>
>> Seems real. This builder has been a little flaky recently. Could be a
>> memory corruption
>> that usually happens on pandaboards?
>
>
> Yes, this is true, this is a pandaboard. I'm not seeing similar failures on
> my other arm builder, the -imx6 one which runs linux/arm and nacl/arm.
>
> I've rebooted the pandaboard builder, maybe that will help.