Re: [PATCH 1/6] fs: Add flag to file_system_type to indicate content is generated

[Ian Lance Taylor <iant@golang.org>]

[-- Attachment #1: Type: text/plain, Size: 3550 bytes --]

On Fri, Feb 12, 2021 at 8:28 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Fri, Feb 12, 2021 at 07:59:04AM -0800, Ian Lance Taylor wrote:
> > On Fri, Feb 12, 2021 at 7:45 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Fri, Feb 12, 2021 at 07:33:57AM -0800, Ian Lance Taylor wrote:
> > > > On Fri, Feb 12, 2021 at 12:38 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > >
> > > > > Why are people trying to use copy_file_range on simple /proc and /sys
> > > > > files in the first place?  They can not seek (well most can not), so
> > > > > that feels like a "oh look, a new syscall, let's use it everywhere!"
> > > > > problem that userspace should not do.
> > > >
> > > > This may have been covered elsewhere, but it's not that people are
> > > > saying "let's use copy_file_range on files in /proc."  It's that the
> > > > Go language standard library provides an interface to operating system
> > > > files.  When Go code uses the standard library function io.Copy to
> > > > copy the contents of one open file to another open file, then on Linux
> > > > kernels 5.3 and greater the Go standard library will use the
> > > > copy_file_range system call.  That seems to be exactly what
> > > > copy_file_range is intended for.  Unfortunately it appears that when
> > > > people writing Go code open a file in /proc and use io.Copy the
> > > > contents to another open file, copy_file_range does nothing and
> > > > reports success.  There isn't anything on the copy_file_range man page
> > > > explaining this limitation, and there isn't any documented way to know
> > > > that the Go standard library should not use copy_file_range on certain
> > > > files.
> > >
> > > But, is this a bug in the kernel in that the syscall being made is not
> > > working properly, or a bug in that Go decided to do this for all types
> > > of files not knowing that some types of files can not handle this?
> > >
> > > If the kernel has always worked this way, I would say that Go is doing
> > > the wrong thing here.  If the kernel used to work properly, and then
> > > changed, then it's a regression on the kernel side.
> > >
> > > So which is it?
> >
> > I don't work on the kernel, so I can't tell you which it is.  You will
> > have to decide.
>
> As you have the userspace code, it should be easier for you to test this
> on an older kernel.  I don't have your userspace code...

Sorry, I'm not sure what you are asking.

I've attached a sample Go program.  On kernel version 2.6.32 this
program exits 0.  On kernel version 5.7.17 it prints

got "" want "./foo\x00"

and exits with status 1.

This program hardcodes the string "/proc/self/cmdline" for
convenience, but of course the same results would happen if this were
a generic copy program that somebody invoked with /proc/self/cmdline
as a command line option.

I could write the same program in C easily enough, by explicitly
calling copy_file_range.  Would it help to see a sample C program?


> > From my perspective, as a kernel user rather than a kernel developer,
> > a system call that silently fails for certain files and that provides
> > no way to determine either 1) ahead of time that the system call will
> > fail, or 2) after the call that the system call did fail, is a useless
> > system call.
>
> Great, then don't use copy_file_range() yet as it seems like it fits
> that category at the moment :)

That seems like an unfortunate result, but if that is the determining
opinion then I guess that is what we will have to do in the Go
standard library.

Ian

[-- Attachment #2: foo7.go --]
[-- Type: text/x-go, Size: 972 bytes --]

package main

import (
	"bytes"
	"fmt"
	"io"
	"io/ioutil"
	"os"
)

func main() {
	tmpfile, err := ioutil.TempFile("", "copy_file_range")
	if err != nil {
		fmt.Fprint(os.Stderr, err)
		os.Exit(1)
	}
	status := copy(tmpfile)
	os.Remove(tmpfile.Name())
	os.Exit(status)
}

func copy(tmpfile *os.File) int {
	cmdline, err := os.Open("/proc/self/cmdline")
	if err != nil {
		fmt.Fprintln(os.Stderr, err)
		return 1
	}
	defer cmdline.Close()
	if _, err := io.Copy(tmpfile, cmdline); err != nil {
		fmt.Fprintf(os.Stderr, "copy failed: %v\n", err)
		return 1
	}
	if err := tmpfile.Close(); err != nil {
		fmt.Fprintln(os.Stderr, err)
		return 1
	}
	old, err := ioutil.ReadFile("/proc/self/cmdline")
	if err != nil {
		fmt.Fprintln(os.Stderr, err)
		return 1
	}
	new, err := ioutil.ReadFile(tmpfile.Name())
	if err != nil {
		fmt.Fprintln(os.Stderr, err)
		return 1
	}
	if !bytes.Equal(old, new) {
		fmt.Fprintf(os.Stderr, "got %q want %q\n", new, old)
		return 1
	}
	return 0
}