Skip to content

tail: reuse existing pipe when stdout is already a pipe#12463

Merged
RenjiSann merged 2 commits into
uutils:mainfrom
dragutreis:fix/tail-unnecessary-pipe
May 26, 2026
Merged

tail: reuse existing pipe when stdout is already a pipe#12463
RenjiSann merged 2 commits into
uutils:mainfrom
dragutreis:fix/tail-unnecessary-pipe

Conversation

@dragutreis

Copy link
Copy Markdown
Contributor

Summary

Fixes #12413

tail -c +1 /dev/null made an unnecessary pipe2 syscall because
splice_unbounded_broker unconditionally created a pipe before attempting
any data transfer.

Root cause

bounded_tail called print_target_section unconditionally even when the
file had no data to output. On Linux, print_target_section calls
splice_unbounded_broker, which always created a pipe regardless of whether
there was any data to transfer.

Fix

Add an is_file_exhausted helper and check it before calling
print_target_section in the unbounded case:

  • Regular files: compare stream position against metadata length.
  • Char/block devices (Unix): probe with a single-byte read; if data exists,
    write the byte directly to stdout since char devices don't support seeking.

@oech3

oech3 commented May 24, 2026

Copy link
Copy Markdown
Contributor

It is not what I was expected at the linked PR. I meant | echo should allow reusing existing pipe |.

@oech3

oech3 commented May 24, 2026

Copy link
Copy Markdown
Contributor

But it seems still useful for supressing other syscalls at some cases. Right?

@dragutreis

Copy link
Copy Markdown
Contributor Author

Oh, thanks for clarifying, I'm not really sure what to do however, should I keep this pr and open a new one to fix the actual issue?

@oech3

oech3 commented May 24, 2026

Copy link
Copy Markdown
Contributor

We should try splice_unbounded before _broker to omit unnecessary pipe broker (and myself tried to do that, but failed).

@github-actions

github-actions Bot commented May 24, 2026

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/cut/bounded-memory (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/date/resolution (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/misc/tty-eof (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/pr/bounded-memory (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/printf/printf-surprise is now being skipped but was previously passing.
Note: The gnu test tests/rm/many-dir-entries-vs-OOM is now being skipped but was previously passing.
Congrats! The gnu test tests/tail/tail-n0f is now passing!

@oech3

oech3 commented May 24, 2026

Copy link
Copy Markdown
Contributor

Actually, I was not able to fix it by splice_unbounded_auto which tries to avoid pipe2(). It might coming from different code path as we expected.

@dragutreis

Copy link
Copy Markdown
Contributor Author

Hmmm, I'll look into it and try to fix it.

@oech3

This comment was marked as resolved.

@oech3

oech3 commented May 24, 2026

Copy link
Copy Markdown
Contributor

( to clarify, strace -c -e pipe,pipe2 tail -c +1 /dev/zero should pipe2, but strace -c -e pipe,pipe2 tail -c +1 /dev/zero|echo should not pipe2)

@dragutreis

Copy link
Copy Markdown
Contributor Author

Used strace -k to track it down. The pipe2 call was coming from splice_unbounded_broker in pipes.rs, which unconditionally creates a broker pipe even when stdout is already a pipe. There's even a comment around line 109 noting it shouldn't be used when one side is already a pipe. Replacing it with splice_unbounded_auto fixes it, not sure why it didn't work when you tried.

$ strace -c -e pipe,pipe2 tail -c +1 /dev/zero
1 pipe2 (expected)
$ strace -c -e pipe,pipe2 sh -c 'tail -c +1 /dev/zero | echo'
1 pipe2 from bash only (expected)

@oech3

oech3 commented May 24, 2026

Copy link
Copy Markdown
Contributor

Strange... But it it fixed the issue for you. Confirmed it. So this PR's diff is just 1 line.

@dragutreis

Copy link
Copy Markdown
Contributor Author

Awesome, glad that fixed it.

@oech3

oech3 commented May 24, 2026

Copy link
Copy Markdown
Contributor

Would you revert 1st change, make diff 1line, and edit description to note about _auto? Thanks.

@dragutreis

Copy link
Copy Markdown
Contributor Author

Will do.

@dragutreis dragutreis changed the title tail: avoid unnecessary pipe2 syscall for empty files. tail: reuse existing pipe when stdout is already a pipe May 24, 2026
@RenjiSann

Copy link
Copy Markdown
Collaborator

Please squash your commits in a single one

@oech3

oech3 commented May 25, 2026

Copy link
Copy Markdown
Contributor

Is squash merge not allowed?

Use splice_unbounded_auto instead of splice_unbounded_broker to avoid
creating a broker pipe when stdout is already a pipe (e.g., when piping
to another command with |).
@dragutreis dragutreis force-pushed the fix/tail-unnecessary-pipe branch from da5a633 to ad33937 Compare May 25, 2026 11:49
@RenjiSann RenjiSann merged commit 83a5cd9 into uutils:main May 26, 2026
1 check was pending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tail: unnecessary pipe call

3 participants