Skip to content

Allow sync_file_range2 on supported architectures.#30971

Merged
justincormack merged 1 commit intomoby:masterfrom
dargor:allow_sync_file_range2
Feb 15, 2017
Merged

Allow sync_file_range2 on supported architectures.#30971
justincormack merged 1 commit intomoby:masterfrom
dargor:allow_sync_file_range2

Conversation

@dargor
Copy link
Contributor

@dargor dargor commented Feb 13, 2017

Using docker 1.13.1 on a ppc64le host, running postgres (tried with 9.4 to 9.6) gives the following warning when trying to flush data to disks (which happens very frequently) :
WARNING: could not flush dirty data: Operation not permitted.

A quick dig in postgres source code indicate it uses sync_file_range(2) to flush data; which on ppe64le and arm64 is translated to sync_file_range2(2) for alignements reasons.

But docker does not allow sync_file_range2(2), making postgres sad because it can not flush its buffers. arm_sync_file_range(2) is an ancient alias to sync_file_range2(2), the syscall was renamed in Linux 2.6.22 when the same syscall was added for PowerPC.

This diff make postgres, docker and ppc64le great again.

@GordonTheTurtle GordonTheTurtle added dco/no Automatically set by a bot when one of the commits lacks proper signature status/0-triage labels Feb 13, 2017
@dargor dargor force-pushed the allow_sync_file_range2 branch from f10b99e to ddcfcc8 Compare February 13, 2017 13:35
@GordonTheTurtle GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Feb 13, 2017
@cpuguy83
Copy link
Member

cpuguy83 commented Feb 13, 2017 

13:51:26 The result of go generate ./profiles/seccomp/ differs
13:51:26 
13:51:26  M profiles/seccomp/default.json
13:51:26 
13:51:26 Please re-run go generate ./profiles/seccomp/

ping @justincormack

@justincormack
Copy link
Contributor

justincormack commented Feb 13, 2017 •
edited
Loading

Yes, you need to regenerate the JSON version with go generate... it is picky, it wants the final line ending the same as the script...

@justincormack
Copy link
Contributor

justincormack commented Feb 13, 2017

If you just remove the final line feed on the json perl -pi -e 'chomp if eof' profiles/seccomp/default.json should do it...

@justincormack
Copy link
Contributor

justincormack commented Feb 13, 2017

Other than that LGTM

@justincormack justincormack added this to the 1.13.2 milestone Feb 13, 2017
@cpuguy83
Copy link
Member

cpuguy83 commented Feb 13, 2017

Please make sure to squash commits, thanks!

@dargor dargor force-pushed the allow_sync_file_range2 branch from 7084bb0 to 3d926d8 Compare February 13, 2017 16:09
@justincormack
Copy link
Contributor

justincormack commented Feb 13, 2017

Sorry, the CI is obstinately refusing to rebuild even though I poked it.

@justincormack
Copy link
Contributor

justincormack commented Feb 13, 2017

Ok, its rebuilding now should be good.

@tonistiigi
Copy link
Member

tonistiigi commented Feb 13, 2017

LGTM

@justincormack
Copy link
Contributor

justincormack commented Feb 14, 2017

Hmm, Janky still thinks that go generate ./profiles/seccomp/ needs to be run...

tonistiigi
tonistiigi reviewed Feb 14, 2017
View reviewed changes
profiles/seccomp/default.json Outdated
Copy link
Member

@tonistiigi tonistiigi Feb 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra commas here and line 642 seems to cause the validation issue.

@dargor dargor force-pushed the allow_sync_file_range2 branch from 3d926d8 to 52d8f58 Compare February 14, 2017 20:30
@justincormack
Copy link
Contributor

justincormack commented Feb 15, 2017

Thanks!

@justincormack justincormack merged commit 3b5ed56 into moby:master Feb 15, 2017
@thaJeztah
Copy link
Member

thaJeztah commented Feb 18, 2017

@justincormack I tried cherry-picking this into the 1.13.x branch (for 17.03), but it didn't apply cleanly. If you want this into that branch, could you open a PR?

@justincormack justincormack mentioned this pull request Feb 18, 2017
Merged
@thaJeztah thaJeztah mentioned this pull request Aug 24, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tonistiigi tonistiigi tonistiigi left review comments

+1 more reviewer

@justincormack justincormack justincormack approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

process/cherry-picked status/2-code-review

Projects

None yet

Milestone

17.03.0

Development

Successfully merging this pull request may close these issues.

8 participants

@dargor @cpuguy83 @justincormack @tonistiigi @thaJeztah @LK4D4 @vieux @GordonTheTurtle