Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(217)

Issue 6852105: code review 6852105: undo CL 6855110 / 869253ef7009 (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 7 months ago by dfc
Modified:
9 years, 7 months ago
Reviewers:
dvyukov
CC:
rsc, mikio, minux1, golang-dev
Visibility:
Public.

Description

undo CL 6855110 / 869253ef7009 64bit atomics are broken on 32bit systems. This is issue 599. linux/arm builders all broke with this change, I am concerned that the other 32bit builders are silently impacted. ««« original CL description net: fix data races on deadline vars Fixes issue 4434. R=mikioh.mikioh, bradfitz, dvyukov, alex.brainman CC=golang-dev https://codereview.appspot.com/6855110 »»»

Patch Set 1 #

Patch Set 2 : diff -r 869253ef7009 https://code.google.com/p/go #

Patch Set 3 : diff -r 869253ef7009 https://code.google.com/p/go #

Patch Set 4 : diff -r 869253ef7009 https://code.google.com/p/go #

Unified diffs Side-by-side diffs Delta from patch set Stats (+83 lines, -167 lines) Patch
R src/pkg/net/fd_posix_test.go View 1 1 chunk +0 lines, -57 lines 0 comments Download
M src/pkg/net/fd_unix.go View 1 13 chunks +53 lines, -61 lines 0 comments Download
M src/pkg/net/fd_windows.go View 1 6 chunks +10 lines, -38 lines 0 comments Download
M src/pkg/net/sendfile_freebsd.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/net/sendfile_linux.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/net/sock_posix.go View 1 1 chunk +4 lines, -2 lines 0 comments Download
M src/pkg/net/sockopt_posix.go View 1 1 chunk +14 lines, -7 lines 0 comments Download

Messages

Total messages: 4
dfc
Hello rsc@golang.org, mikioh.mikioh@gmail.com, dvyukov@google.com, minux.ma@gmail.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to ...
9 years, 7 months ago (2012-11-30 08:56:12 UTC) #1
mikio
LGTM
9 years, 7 months ago (2012-11-30 09:01:02 UTC) #2
dfc
*** Submitted as https://code.google.com/p/go/source/detail?r=697f36fec52c *** undo CL 6855110 / 869253ef7009 64bit atomics are broken on ...
9 years, 7 months ago (2012-11-30 09:02:49 UTC) #3
dvyukov
9 years, 7 months ago (2012-11-30 09:26:16 UTC) #4
Message was sent while issue was closed.
On 2012/11/30 09:02:49, dfc wrote:
> *** Submitted as https://code.google.com/p/go/source/detail?r=697f36fec52c ***
> 
> undo CL 6855110 / 869253ef7009
> 
> 64bit atomics are broken on 32bit systems. This is issue 599.
> 
> linux/arm builders all broke with this change, I am concerned that the other
> 32bit builders are silently impacted.
> 
> ««« original CL description
> net: fix data races on deadline vars
> 
> Fixes issue 4434.
> 
> R=mikioh.mikioh, bradfitz, dvyukov, alex.brainman
> CC=golang-dev
> https://codereview.appspot.com/6855110
> »»»
> 
> R=rsc, mikioh.mikioh, dvyukov, minux.ma
> CC=golang-dev
> https://codereview.appspot.com/6852105

The issue 599 causes non-atomicity of 64-bit atomics on 386. It leads to crashes
only if you you heavily stress atomicity. I do not think that it should causes
instant crashes in this case. Moreover, ARM seems to be using STREX/LDREX for
64-bit atomic loads/stores, so most likely it does not even have the problem.
Perhaps ARM builders are affected by something else.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b