code review 57730043: runtime: adjust malloc race instrumentation for tiny allocs

runtime: adjust malloc race instrumentation for tiny allocs
Tiny alloc memory block is shared by different goroutines running on the same thread.
We call racemalloc after enabling preemption in mallocgc,
as the result another goroutine can act on not yet race-cleared tiny block.
Call racemalloc before enabling preemption.
Fixes issue 7224.

[dvyukov, 2014-01-28 12:15:26]

Hello golang-codereviews@googlegroups.com,

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

[dave_cheney.net, 2014-01-28 12:17:29]

LGTM. Is it possible to write a test for issue 7224 ?


On Tue, Jan 28, 2014 at 11:15 PM, <dvyukov@google.com> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com,
>
> I'd like you to review this change to
> https://dvyukov%40google.com@code.google.com/p/go/
>
>
> Description:
> runtime: adjust malloc race instrumentation for tiny allocs
> Tiny alloc memory block is shared by different goroutines running on the
> same thread.
> We call racemalloc after enabling preemption in mallocgc,
> as the result another goroutine can act on not yet race-cleared tiny
> block.
> Call racemalloc before enabling preemption.
> Fixes issue 7224.
>
> Please review this at https://codereview.appspot.com/57730043/
>
> Affected files (+2, -2 lines):
>   M src/pkg/runtime/malloc.goc
>
>
> Index: src/pkg/runtime/malloc.goc
> ===================================================================
> --- a/src/pkg/runtime/malloc.goc
> +++ b/src/pkg/runtime/malloc.goc
> @@ -182,6 +182,8 @@
>         m->mallocing = 0;
>         if(UseSpanType && !(flag & FlagNoScan) && typ != 0 &&
> m->settype_bufsize == nelem(m->settype_buf))
>                 runtime·settype_flush(m);
> +       if(raceenabled)
> +               runtime·racemalloc(v, size);
>         m->locks--;
>         if(m->locks == 0 && g->preempt)  // restore the preemption request
> in case we've cleared it in newstack
>                 g->stackguard0 = StackPreempt;
> @@ -208,8 +210,6 @@
>         if(!(flag & FlagNoInvokeGC) && mstats.heap_alloc >= mstats.next_gc)
>                 runtime·gc(0);
>
> -       if(raceenabled)
> -               runtime·racemalloc(v, size);
>         return v;
>  }
>
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>

[dvyukov, 2014-01-28 18:34:16]

On 2014/01/28 12:17:29, dfc wrote:
> LGTM. Is it possible to write a test for issue 7224 ?

Added the test, it fails reliably w/o the fix.

[dvyukov, 2014-01-28 18:35:42]

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

runtime: adjust malloc race instrumentation for tiny allocs
Tiny alloc memory block is shared by different goroutines running on the same
thread.
We call racemalloc after enabling preemption in mallocgc,
as the result another goroutine can act on not yet race-cleared tiny block.
Call racemalloc before enabling preemption.
Fixes issue 7224.

LGTM=dave
R=golang-codereviews, dave
CC=golang-codereviews
https://codereview.appspot.com/57730043