os: RemoveAll: pass the O_NONBLOCK flag to openat(2) instead of making an extra fcntl(2) call

[aktau]

What version of Go are you using (go version)?

1.20 (pre-prelease, a813be8)

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GOARCH="amd64"

What did you do?

I called os.RemoveAll and a colleague saw syscall.fcntl in the resulting profile:

This colleague said that we could also pass O_NONBLOCK to the open(2) syscall.

Reference: https://cs.opensource.google/go/go/+/master:src/os/removeall_at.go;l=174,191;drc=3e5c2c155645ebaed62e4481430c455045b0fff5

Something like:

// openFdAt opens path relative to the directory in fd.
// Other than that this should act like openFileNolog.
// This acts like openFileNolog rather than OpenFile because
// we are going to (try to) remove the file.
// The contents of this file are not relevant for test caching.
func openFdAt(dirfd int, name string) (*File, error) {
	var r int
	for {
		var e error
-		r, e = unix.Openat(dirfd, name, O_RDONLY|syscall.O_CLOEXEC, 0)
+		r, e = unix.Openat(dirfd, name, O_RDONLY|syscall.O_CLOEXEC|O_NONBLOCK, 0)
 
		if e == nil {
			break
		}

		// See comment in openFileNolog.
		if e == syscall.EINTR {
			continue
		}

		return nil, e
	}

	if !supportsCloseOnExec {
		syscall.CloseOnExec(r)
	}

-	return newFile(uintptr(r), name, kindOpenFile), nil
+	return newFile(uintptr(r), name, kindNonBlocking), nil
}

There's probably a good reason why things are the way they are. But I couldn't think of any.

[gopherbot]

Change https://go.dev/cl/451997 mentions this issue: os: don't try to put directory into non-blocking mode

[ianlancetaylor]

In this case we know that the file is a directory, and directories aren't usually supported by the poller anyhow. So rather than open in non-blocking mode, which is kind of meaningless for a directory, we can tell newFile to skip the change to non-blocking mode. I sent https://go.dev/cl/451997 for this. Not sure it's worth doing at this stage of the release cycle, though.