Skip to content

Commit 1f58ad5

Browse files
prattmicgopherbot
authored andcommitted
Revert "os: employ sendfile(2) for file-to-file copying on Linux when needed"
This reverts CL 603295. Reason for revert: can cause child exit_group to hang. This is not a clean revert. CL 603098 did a major refactoring of the tests. That refactor is kept, just the sendfile-specific tests are dropped from the linux tests. Fixes #71375. Change-Id: Ic4d6535759667c69a44bd9281bbb33d5b559f591 Reviewed-on: https://go-review.googlesource.com/c/go/+/644895 Auto-Submit: Michael Pratt <mpratt@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Andy Pan <panjf2000@gmail.com>
1 parent 90ec999 commit 1f58ad5

File tree

3 files changed

+7
-93
lines changed

3 files changed

+7
-93
lines changed

src/os/readfrom_linux_test.go

Lines changed: 2 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -242,25 +242,19 @@ func testSpliceToTTY(t *testing.T, proto string, size int64) {
242242
}
243243

244244
var (
245-
copyFileTests = []copyFileTestFunc{newCopyFileRangeTest, newSendfileOverCopyFileRangeTest}
246-
copyFileHooks = []copyFileTestHook{hookCopyFileRange, hookSendFileOverCopyFileRange}
245+
copyFileTests = []copyFileTestFunc{newCopyFileRangeTest}
246+
copyFileHooks = []copyFileTestHook{hookCopyFileRange}
247247
)
248248

249249
func testCopyFiles(t *testing.T, size, limit int64) {
250250
testCopyFileRange(t, size, limit)
251-
testSendfileOverCopyFileRange(t, size, limit)
252251
}
253252

254253
func testCopyFileRange(t *testing.T, size int64, limit int64) {
255254
dst, src, data, hook, name := newCopyFileRangeTest(t, size)
256255
testCopyFile(t, dst, src, data, hook, limit, name)
257256
}
258257

259-
func testSendfileOverCopyFileRange(t *testing.T, size int64, limit int64) {
260-
dst, src, data, hook, name := newSendfileOverCopyFileRangeTest(t, size)
261-
testCopyFile(t, dst, src, data, hook, limit, name)
262-
}
263-
264258
// newCopyFileRangeTest initializes a new test for copy_file_range.
265259
//
266260
// It hooks package os' call to poll.CopyFileRange and returns the hook,
@@ -276,20 +270,6 @@ func newCopyFileRangeTest(t *testing.T, size int64) (dst, src *File, data []byte
276270
return
277271
}
278272

279-
// newSendfileOverCopyFileRangeTest initializes a new test for sendfile over copy_file_range.
280-
// It hooks package os' call to poll.SendFile and returns the hook,
281-
// so it can be inspected.
282-
func newSendfileOverCopyFileRangeTest(t *testing.T, size int64) (dst, src *File, data []byte, hook *copyFileHook, name string) {
283-
t.Helper()
284-
285-
name = "newSendfileOverCopyFileRangeTest"
286-
287-
dst, src, data = newCopyFileTest(t, size)
288-
hook, _ = hookSendFileOverCopyFileRange(t)
289-
290-
return
291-
}
292-
293273
// newSpliceFileTest initializes a new test for splice.
294274
//
295275
// It creates source sockets and destination file, and populates the source sockets
@@ -342,34 +322,6 @@ func hookCopyFileRange(t *testing.T) (hook *copyFileHook, name string) {
342322
return
343323
}
344324

345-
func hookSendFileOverCopyFileRange(t *testing.T) (*copyFileHook, string) {
346-
return hookSendFileTB(t), "hookSendFileOverCopyFileRange"
347-
}
348-
349-
func hookSendFileTB(tb testing.TB) *copyFileHook {
350-
// Disable poll.CopyFileRange to force the fallback to poll.SendFile.
351-
originalCopyFileRange := *PollCopyFileRangeP
352-
*PollCopyFileRangeP = func(dst, src *poll.FD, remain int64) (written int64, handled bool, err error) {
353-
return 0, false, nil
354-
}
355-
356-
hook := new(copyFileHook)
357-
orig := poll.TestHookDidSendFile
358-
tb.Cleanup(func() {
359-
*PollCopyFileRangeP = originalCopyFileRange
360-
poll.TestHookDidSendFile = orig
361-
})
362-
poll.TestHookDidSendFile = func(dstFD *poll.FD, src int, written int64, err error, handled bool) {
363-
hook.called = true
364-
hook.dstfd = dstFD.Sysfd
365-
hook.srcfd = src
366-
hook.written = written
367-
hook.err = err
368-
hook.handled = handled
369-
}
370-
return hook
371-
}
372-
373325
func hookSpliceFile(t *testing.T) *spliceFileHook {
374326
h := new(spliceFileHook)
375327
h.install()

src/os/readfrom_sendfile_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
//go:build linux || solaris
5+
//go:build solaris
66

77
package os_test
88

src/os/zero_copy_linux.go

Lines changed: 4 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -40,17 +40,16 @@ func (f *File) writeTo(w io.Writer) (written int64, handled bool, err error) {
4040
}
4141

4242
func (f *File) readFrom(r io.Reader) (written int64, handled bool, err error) {
43-
// Neither copy_file_range(2)/sendfile(2) nor splice(2) supports destinations opened with
43+
// Neither copy_file_range(2) nor splice(2) supports destinations opened with
4444
// O_APPEND, so don't bother to try zero-copy with these system calls.
4545
//
4646
// Visit https://man7.org/linux/man-pages/man2/copy_file_range.2.html#ERRORS and
47-
// https://man7.org/linux/man-pages/man2/sendfile.2.html#ERRORS and
4847
// https://man7.org/linux/man-pages/man2/splice.2.html#ERRORS for details.
4948
if f.appendMode {
5049
return 0, false, nil
5150
}
5251

53-
written, handled, err = f.copyFile(r)
52+
written, handled, err = f.copyFileRange(r)
5453
if handled {
5554
return
5655
}
@@ -87,7 +86,7 @@ func (f *File) spliceToFile(r io.Reader) (written int64, handled bool, err error
8786
return written, handled, wrapSyscallError("splice", err)
8887
}
8988

90-
func (f *File) copyFile(r io.Reader) (written int64, handled bool, err error) {
89+
func (f *File) copyFileRange(r io.Reader) (written int64, handled bool, err error) {
9190
var (
9291
remain int64
9392
lr *io.LimitedReader
@@ -116,44 +115,7 @@ func (f *File) copyFile(r io.Reader) (written int64, handled bool, err error) {
116115
if lr != nil {
117116
lr.N -= written
118117
}
119-
120-
if handled {
121-
return written, handled, wrapSyscallError("copy_file_range", err)
122-
}
123-
124-
// If fd_in and fd_out refer to the same file and the source and target ranges overlap,
125-
// copy_file_range(2) just returns EINVAL error. poll.CopyFileRange will ignore that
126-
// error and act like it didn't call copy_file_range(2). Then the caller will fall back
127-
// to generic copy, which results in doubling the content in the file.
128-
// By contrast, sendfile(2) allows this kind of overlapping and works like a memmove,
129-
// in this case the file content will remain the same after copying, which is not what we want.
130-
// Thus, we just bail out here and leave it to generic copy when it's a file copying itself.
131-
if f.pfd.Sysfd == src.pfd.Sysfd {
132-
return 0, false, nil
133-
}
134-
135-
sc, err := src.SyscallConn()
136-
if err != nil {
137-
return
138-
}
139-
140-
// We can employ sendfile(2) when copy_file_range(2) fails to handle the copy.
141-
// sendfile(2) enabled file-to-file copying since Linux 2.6.33 and Go requires
142-
// Linux 3.2 or later, so we're good to go.
143-
// Check out https://man7.org/linux/man-pages/man2/sendfile.2.html#DESCRIPTION for more details.
144-
rerr := sc.Read(func(fd uintptr) bool {
145-
written, err, handled = poll.SendFile(&f.pfd, int(fd), remain)
146-
return true
147-
})
148-
if lr != nil {
149-
lr.N -= written
150-
}
151-
152-
if err == nil {
153-
err = rerr
154-
}
155-
156-
return written, handled, wrapSyscallError("sendfile", err)
118+
return written, handled, wrapSyscallError("copy_file_range", err)
157119
}
158120

159121
// getPollFDAndNetwork tries to get the poll.FD and network type from the given interface

0 commit comments

Comments
 (0)