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

Jump to bottom

runtime: insufficient padding in the p structure #16477

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

runtime: insufficient padding in the p structure #16477

bcbrock opened this issue Jul 23, 2016 · 2 comments
Labels
FrozenDueToAge
Milestone
Go1.8

Comments

@bcbrock
Copy link

bcbrock 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
Copy link
Contributor

ceseo 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
@ceseo
runtime: fix padding in the 'p' struct and cache line size for ppc64x
b08d844 
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
Copy link

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

tmm1 pushed a commit to fancybits/go that referenced this issue Nov 9, 2016
@ceseo @tmm1
runtime: insufficient padding in the p structure
49b61fe 
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.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.8
Development

No branches or pull requests
4 participants
@ianlancetaylor @ceseo @gopherbot @bcbrock