linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Ian Lance Taylor <iant@golang.org>
Cc: Nicolas Boichat <drinkcat@chromium.org>,
	"Darrick J . Wong" <djwong@kernel.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Luis Lozano <llozano@chromium.org>,
	Dave Chinner <david@fromorbit.com>,
	linux-fsdevel@vger.kernel.org,
	lkml <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/6] fs: Add flag to file_system_type to indicate content is generated
Date: Fri, 12 Feb 2021 17:28:50 +0100	[thread overview]
Message-ID: <YCaswkcgM2NMxiMh@kroah.com> (raw)
In-Reply-To: <CAOyqgcVTYhozM-mwc400qt+fabmUuBQTsjqbcA03xDooYXXcMA@mail.gmail.com>

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...

> 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 :)

> I can never use that system call, because I don't know
> whether or not it will work.  So as a kernel user I would say that you
> should fix the system call to report failure, or document some way to
> know whether the system call will fail, or you should remove the
> system call.  But I'm not a kernel developer, I don't have all the
> information, and it's obviously your call.

That's up to the authors of that syscall, I don't know what they
intended this to be used for.  It feels like you are using this in a
very generic "all file i/o works for this" way, while that is not what
the authors of the syscall intended it for, as is evident by the
failures that older kernels would report for you.

> I'll note that to the best of my knowledge this failure started
> happening with the 5.3 kernel, as before 5.3 the problematic calls
> would report a failure (EXDEV).  Since 5.3 isn't all that old I
> personally wouldn't say that the kernel "has always worked this way."
> But I may be mistaken about this.

Testing would be good, as regressions are more serious than "it would be
nice if it would work like this instead!" type of issues.

thanks,

greg k-h

  reply	other threads:[~2021-02-12 16:31 UTC|newest]

Thread overview: 136+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-12  4:43 [PATCH 0/6] Add generated flag to filesystem struct to block copy_file_range Nicolas Boichat
2021-02-12  4:44 ` [PATCH 1/6] fs: Add flag to file_system_type to indicate content is generated Nicolas Boichat
2021-02-12  7:46   ` Greg KH
2021-02-12  8:20     ` Nicolas Boichat
2021-02-12  8:37       ` Greg KH
2021-02-12 15:33         ` Ian Lance Taylor
2021-02-12 15:45           ` Greg KH
2021-02-12 15:59             ` Ian Lance Taylor
2021-02-12 16:28               ` Greg KH [this message]
2021-02-12 20:22                 ` Ian Lance Taylor
2021-02-12 23:03             ` Dave Chinner
2021-02-12 23:07               ` Ian Lance Taylor
2021-02-12 23:27                 ` Dave Chinner
2021-02-12 23:54                   ` Darrick J. Wong
2021-02-15  0:38                     ` Dave Chinner
2021-02-15  1:12                       ` Ian Lance Taylor
2021-02-15  1:25                         ` Nicolas Boichat
2021-02-15  5:56                           ` Amir Goldstein
2021-02-15  8:30                           ` Greg KH
2021-02-12  8:22     ` Amir Goldstein
2021-02-12  8:39       ` Greg KH
2021-02-12 12:05         ` Luis Henriques
2021-02-12 12:18           ` Greg KH
2021-02-12 12:41             ` Luis Henriques
2021-02-12 14:11               ` Greg KH
2021-02-12 15:01                 ` Luis Henriques
2021-02-15  6:12               ` Amir Goldstein
2021-02-15 10:39                 ` Luis Henriques
2021-02-15 12:22                   ` Luis Henriques
2021-02-15 14:23                     ` Amir Goldstein
2021-02-15 14:51                       ` Luis Henriques
2021-02-15 15:43                       ` [PATCH v2] vfs: prevent copy_file_range to copy across devices Luis Henriques
2021-02-15 16:02                         ` Trond Myklebust
2021-02-16  0:25                           ` Steve French
2021-02-15 16:34                         ` Amir Goldstein
2021-02-15 16:53                           ` Trond Myklebust
2021-02-15 17:24                             ` Amir Goldstein
2021-02-15 18:57                               ` Trond Myklebust
2021-02-15 19:43                                 ` Amir Goldstein
2021-02-16 11:17                                   ` Luis Henriques
2021-02-16 11:28                                     ` gregkh
2021-02-16 12:01                                       ` Luis Henriques
2021-02-16 12:08                                         ` Greg KH
2021-02-16 13:51                                     ` Amir Goldstein
2021-02-16 16:42                                       ` Luis Henriques
2021-02-16 17:44                                         ` Amir Goldstein
2021-02-16 18:55                                           ` Luis Henriques
2021-02-16 19:20                                             ` Amir Goldstein
2021-02-16 19:27                                               ` Anna Schumaker
2021-02-16 19:31                                                 ` Steve French
2021-02-16 19:40                                                   ` Amir Goldstein
2021-02-16 21:15                                                     ` Steve French
2021-02-17  8:08                                                       ` Amir Goldstein
2021-02-17 17:26                                                         ` [PATCH v3] vfs: fix copy_file_range regression in cross-fs copies Luis Henriques
2021-02-17 20:47                                                           ` Amir Goldstein
2021-02-18  0:56                                                           ` Nicolas Boichat
2021-02-18  5:32                                                           ` Olga Kornievskaia
2021-02-18  6:47                                                             ` Amir Goldstein
2021-02-18 16:28                                                               ` Olga Kornievskaia
2021-02-18  7:43                                                           ` Christoph Hellwig
2021-02-18  0:50                                                         ` [PATCH v2] vfs: prevent copy_file_range to copy across devices Andreas Dilger
2021-02-18  7:34                                                           ` gregkh
2021-02-16 18:54                                       ` Andreas Dilger
2021-02-17  4:45                         ` Nicolas Boichat
2021-02-18  7:42                         ` Christoph Hellwig
2021-02-18  9:10                           ` Amir Goldstein
2021-02-18 10:29                             ` Luis Henriques
2021-02-18 12:15                               ` Luis Henriques
2021-02-18 12:49                                 ` Amir Goldstein
2021-02-18 14:36                                   ` [PATCH v4] vfs: fix copy_file_range regression in cross-fs copies Luis Henriques
2021-02-18 14:58                                     ` Amir Goldstein
2021-02-18 15:17                                       ` [PATCH v5] " Luis Henriques
2021-02-18 15:53                                         ` Amir Goldstein
2021-02-18 16:35                                           ` Luis Henriques
2021-02-18 17:18                                             ` [PATCH v6] " Luis Henriques
2021-02-19 21:18                                               ` Olga Kornievskaia
2021-02-19 21:52                                                 ` Amir Goldstein
2021-02-21 19:58                                                 ` [PATCH v7] " Luis Henriques
2021-02-22  3:00                                                   ` Nicolas Boichat
2021-02-22 10:24                                                   ` [PATCH v8] " Luis Henriques
2021-02-22 10:46                                                     ` Amir Goldstein
2021-02-22 16:25                                                     ` dai.ngo
2021-02-23 10:32                                                       ` Luis Henriques
2021-02-23 15:28                                                         ` Amir Goldstein
2021-02-23 15:29                                                         ` dai.ngo
2021-02-23 16:02                                                           ` dai.ngo
2021-02-23 16:47                                                             ` Amir Goldstein
2021-02-23 16:57                                                               ` dai.ngo
     [not found]                                                                 ` <e3eed18b-fc7e-e687-608b-7f662017329c@oracle.com>
2021-02-23 17:33                                                                   ` Amir Goldstein
2021-02-24  0:13                                                                     ` dai.ngo
2021-02-23 17:56                                                                 ` Luis Henriques
2021-02-23 17:13                                                             ` Olga Kornievskaia
2021-02-24  1:00                                                     ` Olga Kornievskaia
2021-02-24 10:23                                                       ` Luis Henriques
2021-02-24 10:44                                                         ` Nicolas Boichat
2021-04-09  5:23                                                           ` Nicolas Boichat
2021-04-09 13:39                                                             ` Luis Henriques
2021-04-09 13:50                                                               ` Amir Goldstein
2021-04-23  4:40                                                                 ` Nicolas Boichat
2021-05-03  8:54                                                                   ` Luis Henriques
2021-05-10  4:59                                                                     ` Amir Goldstein
2021-05-10  9:10                                                                       ` Luis Henriques
2021-02-24 14:23                                                     ` [PATCH] copy_file_range.2: Kernel v5.12 updates Luis Henriques
2021-02-24 16:10                                                       ` Amir Goldstein
2021-02-25 10:21                                                         ` Luis Henriques
2021-02-26 10:13                                                           ` Alejandro Colomar (man-pages)
2021-02-26 10:34                                                             ` Amir Goldstein
2021-02-26 11:15                                                               ` Alejandro Colomar (man-pages)
2021-02-26 13:59                                                                 ` Jeff Layton
2021-02-26 21:26                                                                   ` Alejandro Colomar (man-pages)
2021-02-26 22:18                                                         ` Alejandro Colomar (man-pages)
2021-02-27  5:41                                                           ` Amir Goldstein
2021-02-27 12:20                                                             ` Alejandro Colomar (man-pages)
2021-02-27 13:49                                                               ` [RFC v2] copy_file_range.2: Update cross-filesystem support for 5.12 Alejandro Colomar
2021-02-27 16:00                                                                 ` Amir Goldstein
2021-02-27 23:08                                                             ` [PATCH] copy_file_range.2: Kernel v5.12 updates Steve French
2021-02-28  7:35                                                               ` Amir Goldstein
2021-02-28 22:25                                                                 ` Steve French
2021-03-01  6:18                                                                   ` Amir Goldstein
2021-03-01 14:41                                                       ` [RFC v3] copy_file_range.2: Update cross-filesystem support for 5.12 Alejandro Colomar
2021-03-01 14:58                                                         ` Amir Goldstein
2021-03-04  9:38                                                       ` [RFC v4] " Alejandro Colomar
2021-03-04 17:13                                                         ` Darrick J. Wong
2021-03-04 18:24                                                           ` Alejandro Colomar (man-pages)
2021-03-04 23:50                                                             ` Darrick J. Wong
2021-02-24  7:15                                     ` [PATCH v4] vfs: fix copy_file_range regression in cross-fs copies Amir Goldstein
2021-02-24  8:30                                       ` Petr Vorel
2021-02-18 20:41                             ` [PATCH v2] vfs: prevent copy_file_range to copy across devices Steve French
2021-02-12 23:15       ` [PATCH 1/6] fs: Add flag to file_system_type to indicate content is generated Dave Chinner
2021-02-12  7:54   ` Amir Goldstein
2021-02-12  4:44 ` [PATCH 2/6] proc: Add FS_GENERATED_CONTENT to filesystem flags Nicolas Boichat
2021-02-12  4:44 ` [PATCH 6/6] vfs: Disallow copy_file_range on generated file systems Nicolas Boichat
2021-02-12  4:53   ` Darrick J. Wong
2021-02-12  4:59     ` Darrick J. Wong
2021-02-12  5:24       ` Nicolas Boichat
2021-02-14 23:09 ` [PATCH 0/6] Add generated flag to filesystem struct to block copy_file_range Al Viro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YCaswkcgM2NMxiMh@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=david@fromorbit.com \
    --cc=djwong@kernel.org \
    --cc=drinkcat@chromium.org \
    --cc=iant@golang.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llozano@chromium.org \
    --cc=viro@zeniv.linux.org.uk \
    --subject='Re: [PATCH 1/6] fs: Add flag to file_system_type to indicate content is generated' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).