Use less syscalls in `anon_pipe()` by tbu- · Pull Request #39386 · rust-lang/rust

tbu- · 2017-01-29T10:01:33Z

Save a ENOSYS failure from pipe2 and don't try again.

Use cvt instead of cvt_r for pipe2 - EINTR is not an error
pipe2 can return.

Save a `ENOSYS` failure from `pipe2` and don't try again. Use `cvt` instead of `cvt_r` for `pipe2` - `EINTR` is not an error `pipe2` can return.

rust-highfive · 2017-01-29T10:01:46Z

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

nagisa · 2017-01-29T16:01:54Z

 // 2.6.27, however, and because we support 2.6.18 we must detect this
 // support dynamically.
- if cfg!(target_os = "linux") {
+ static TRY_PIPE2: AtomicBool = AtomicBool::new(cfg!(target_os = "linux"));


Preexisting:
pipe2 is also supported on freebsd (>= 10.0), netbsd (>= 6.0), dragonflybsd (appeared ~4.0-ish?), openbsd (>= 5.7). These probably should also appear in that cfg! above.

alexcrichton · 2017-01-29T21:16:59Z

Looking back on this, I'm not actually sure why handling ENOSYS is here at all. We're not doing a syscall manually, but instead calling a glibc function. That means if it exists we won't get ENOSYS from what I understand. In that case can we just remove that branch?

tbu- · 2017-01-30T09:30:18Z

True.

We're not calling the raw syscall but a libc function, the libc will have a compatibility layer.

tbu- · 2017-01-30T16:46:03Z

@alexcrichton Removed the branch.

alexcrichton · 2017-01-30T17:05:46Z

@bors: r+

bors · 2017-01-30T17:05:47Z

📌 Commit 4b46d2a has been approved by alexcrichton

bors · 2017-01-31T13:49:20Z

⌛ Testing commit 4b46d2a with merge 1f7bafc...

bors · 2017-01-31T14:32:06Z

💔 Test failed - status-travis

alexcrichton · 2017-01-31T16:27:03Z

@bors: retry * sccache wedged #39003

…

On Tue, Jan 31, 2017 at 6:32 AM bors ***@***.***> wrote: 💔 Test failed - status-travis <https://travis-ci.org/rust-lang/rust/builds/196950142> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#39386 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAD95JBIP5yPoli7-Tc2FqMOyS6PIQmeks5rX0XogaJpZM4LwwFj> .

bors · 2017-02-01T06:13:10Z

⌛ Testing commit 4b46d2a with merge 3b88122...

bors · 2017-02-01T07:51:03Z

💔 Test failed - status-appveyor

tbu- · 2017-02-01T11:40:39Z

Some curl failed, looks unrelated.

alexcrichton · 2017-02-01T17:17:26Z

@bors: retry

…

On Wed, Feb 1, 2017 at 3:40 AM, tbu- ***@***.***> wrote: Some curl failed, looks unrelated. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#39386 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAD95GceoiA1bLhus7KySDS65ccGPpQ-ks5rYG85gaJpZM4LwwFj> .

bors · 2017-02-02T13:04:57Z

⌛ Testing commit 4b46d2a with merge d5f5474...

Use less syscalls in `anon_pipe()` Save a `ENOSYS` failure from `pipe2` and don't try again. Use `cvt` instead of `cvt_r` for `pipe2` - `EINTR` is not an error `pipe2` can return.

bors · 2017-02-02T15:41:13Z

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing d5f5474 to master...

cuviper · 2017-06-08T01:18:43Z

Looking back on this, I'm not actually sure why handling ENOSYS is here at all. We're not doing a syscall manually, but instead calling a glibc function. That means if it exists we won't get ENOSYS from what I understand.

This is not necessarily true. Glibc has had the pipe2 symbol since 2.9, but it makes no effort to emulate the functionality if the kernel rejects it. The pipe2 stub does nothing, and the all of the arch-specific implementations are raw syscall wrappers that do nothing but copy returned errors to errno. So if your running kernel happens to be older, you'll just get ENOSYS. (I'll bet you can guess why I'm noticing this change now...)

alexcrichton · 2017-06-08T01:22:29Z

Ack sorry about that @cuviper! The fix here would be to just reinstate ENOSYS handling to fall back to pipe, right?

cuviper · 2017-06-08T01:23:31Z

Yeah, that should do it!

alexcrichton · 2017-06-08T01:27:11Z

Ok I'll send a PR for this. Looks like this made it into 1.17.0 but we can at least backport and get it fixed for 1.19.0

alexcrichton · 2017-06-08T01:32:34Z

#42521

Should help fix an accidental regression from rust-lang#39386.

std: Handle ENOSYS when calling `pipe2` Should help fix an accidental regression from rust-lang#39386.

std: Handle ENOSYS when calling `pipe2` Should help fix an accidental regression from #39386.

Should help fix an accidental regression from rust-lang#39386.

Use less syscalls in anon_pipe()
c31d5b5
Save a `ENOSYS` failure from `pipe2` and don't try again. Use `cvt` instead of `cvt_r` for `pipe2` - `EINTR` is not an error `pipe2` can return.

rust-highfive assigned sfackler Jan 29, 2017

nagisa reviewed Jan 29, 2017
View reviewed changes

Don't handle ENOSYS in anon_pipe()
4b46d2a
We're not calling the raw syscall but a libc function, the libc will have a compatibility layer.

bors merged commit 4b46d2a into rust-lang:masterFeb 2, 2017

alexcrichton mentioned this pull request Jun 8, 2017
std: Handle ENOSYS when calling pipe2#42521
Merged

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jun 8, 2017
std: Handle ENOSYS when calling pipe2
44e6406
Should help fix an accidental regression from rust-lang#39386.

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Jun 8, 2017
Rollup merge ofrust-lang#42521- alexcrichton:enosys, r=cuviper
215e2b8
std: Handle ENOSYS when calling `pipe2` Should help fix an accidental regression from rust-lang#39386.

bors added a commit that referenced this pull request Jun 9, 2017
Auto merge of#42521- alexcrichton:enosys, r=cuviper
a7ac71b
std: Handle ENOSYS when calling `pipe2` Should help fix an accidental regression from #39386.

brson pushed a commit to brson/rust that referenced this pull request Jun 12, 2017
std: Handle ENOSYS when calling pipe2
76421c0
Should help fix an accidental regression from rust-lang#39386.

Uh oh!

Uh oh!

Conversation

tbu- commented Jan 29, 2017

Uh oh!

rust-highfive commented Jan 29, 2017

Uh oh!

nagisaJan 29, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexcrichton commented Jan 29, 2017

Uh oh!

tbu- commented Jan 30, 2017

Uh oh!

tbu- commented Jan 30, 2017

Uh oh!

alexcrichton commented Jan 30, 2017

Uh oh!

bors commented Jan 30, 2017

Uh oh!

bors commented Jan 31, 2017

Uh oh!

bors commented Jan 31, 2017

Uh oh!

alexcrichton commented Jan 31, 2017 via email

Uh oh!

bors commented Feb 1, 2017

Uh oh!

bors commented Feb 1, 2017

Uh oh!

tbu- commented Feb 1, 2017

Uh oh!

alexcrichton commented Feb 1, 2017 via email

Uh oh!

bors commented Feb 2, 2017

Uh oh!

bors commented Feb 2, 2017

Uh oh!

cuviper commented Jun 8, 2017

Uh oh!

alexcrichton commented Jun 8, 2017

Uh oh!

cuviper commented Jun 8, 2017

Uh oh!

alexcrichton commented Jun 8, 2017

Uh oh!

alexcrichton commented Jun 8, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

nagisaJan 29, 2017•
edited
Loading