Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime: insufficient padding in the `p` structure #16477

Closed
bcbrock opened this issue Jul 23, 2016 · 2 comments

Comments

Projects
None yet
4 participants
@bcbrock
Copy link

commented Jul 23, 2016

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)?

This issue is observed by reading the source code of the master branch.

While reading the code the other day I noticed that the p structure in src/runtime/runtime2.go includes the following at line 501:

pad [64]byte

I assume the intention is to pad by sys.CacheLineSize to avoid false sharing between the p structures. The value of 64 is currently plausible, but only because ppc64x is incorrectly configured with a cache line size of 64. (It should be 128; Someone from IBM will propose/confirm a fix - @laboger ?)

@ianlancetaylor ianlancetaylor changed the title Insufficient padding in the `p` structure runtime: insufficient padding in the `p` structure Jul 23, 2016

@ianlancetaylor ianlancetaylor added this to the Go1.8 milestone Jul 23, 2016

@ceseo

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2016

Yes, the cache line size should be 128. The only ppc64 processors I'm aware of that have a 64 byte cache line size are the e5500 and the PA6T, which, I believe, aren't supported by Golang.

I'll work on this issue.

ceseo added a commit to ceseo/go that referenced this issue Jul 29, 2016

runtime: fix padding in the 'p' struct and cache line size for ppc64x
The current padding in the 'p' struct is hardcoded at 64 bytes. This change
fixes that by making it equal to the cache line size. It also fixes the cache
line size for ppc64/ppc64le.

Fixes golang#16477
@gopherbot

This comment has been minimized.

Copy link

commented Jul 29, 2016

CL https://golang.org/cl/25370 mentions this issue.

@gopherbot gopherbot closed this in aaa6b53 Aug 29, 2016

tmm1 added a commit to fancybits/go that referenced this issue Nov 9, 2016

runtime: insufficient padding in the `p` structure
The current padding in the 'p' struct is hardcoded at 64 bytes. It should be the
cache line size. On ppc64x, the current value is only okay because sys.CacheLineSize
is wrong at 64 bytes. This change fixes that by making the padding equal to the
cache line size. It also fixes the cache line size for ppc64/ppc64le to 128 bytes.

Fixes golang#16477

Change-Id: Ib7ec5195685116eb11ba312a064f41920373d4a3
Reviewed-on: https://go-review.googlesource.com/25370
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

@golang golang locked and limited conversation to collaborators Aug 29, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.