All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Christoph Hellwig <hch@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: problems with splice from /proc (was Linux 5.10-rc1)
Date: Tue, 27 Oct 2020 08:55:41 +0100	[thread overview]
Message-ID: <20201027075541.GA24429@kroah.com> (raw)
In-Reply-To: <20201027074911.GB29565@infradead.org>

On Tue, Oct 27, 2020 at 07:49:11AM +0000, Christoph Hellwig wrote:
> On Tue, Oct 27, 2020 at 07:48:32AM +0100, Greg KH wrote:
> > On Sun, Oct 25, 2020 at 03:40:27PM -0700, Linus Torvalds wrote:
> > > The most interesting - to me - change here is Christoph's setf_fs()
> > > removal (it got merged through Al Viro, as you can see in my mergelog
> > > below).  It's not a _huge_ change, but it's interesting because the
> > > whole model of set_fs() to specify whether a userspace copy actually
> > > goes to user space or kernel space goes back to pretty much the
> > > original release of Linux, and while the name is entirely historic (it
> > > hasn't used the %fs segment register in a long time), the concept has
> > > remained. Until now.
> > 
> > I told Al this yesterday, but figured I would mention it here for others
> > to see.
> > 
> > Commit 36e2c7421f02 ("fs: don't allow splice read/write without explicit
> > ops") from this patch series, is breaking the bionic test suite that
> > does the following to verify that splice is working:
> > 
> > 	int in = open("/proc/cpuinfo", O_RDONLY);
> > 	ASSERT_NE(in, -1);
> > 
> > 	TemporaryFile tf;
> > 	ssize_t bytes_read = splice(in, nullptr, pipe_fds[1], nullptr, 8*1024, SPLICE_F_MORE | SPLICE_F_MOVE);
> > 	ASSERT_NE(bytes_read, -1);
> > 
> > Before this change, all works well but now splice fails on /proc files
> > (and I'm guessing other virtual filesystems).
> > 
> > I'll ask the bionic developers if they can change their test to some
> > other file, but this is a regression and might show up in other "test
> > platforms" as well.  Using /proc for this is just so simple because
> > these files are "always there" and don't require any housekeeping for
> > test suites to worry about .
> 
> Is this just a test or a real application?   I already have the
> infrastructure to support read_iter/write_iter on procfs and seq_files,
> but due to the intrusiveness we decided to only fix instances on an as
> needed basis.  So we'll have everything ready once we pull the trigger.

This is just a test, part of the bionic test suite to verify that bionic
is working properly, and is run on new kernels as a verification that
nothing functional broke in the kernel update.

I don't know about "real applications" yet.

Do you have to implement this on a per-proc-file-basis, or will it work
for the whole filesystem?

And are the patches public anywhere that I could test them out?

thanks,

greg k-h

  reply	other threads:[~2020-10-27  7:54 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-25 22:40 Linux 5.10-rc1 Linus Torvalds
2020-10-25 23:43 ` linux-next: stats Stephen Rothwell
2020-10-27  6:48 ` problems with splice from /proc (was Linux 5.10-rc1) Greg KH
2020-10-27  7:49   ` Christoph Hellwig
2020-10-27  7:55     ` Greg KH [this message]
2020-10-27  8:07       ` Christoph Hellwig
2020-10-27  8:14         ` Greg KH
2020-10-27  8:14           ` Christoph Hellwig
2020-10-27  9:17           ` Greg KH
2020-10-27 16:32             ` Christoph Hellwig
2020-10-27 17:43               ` Greg KH
2020-10-28 16:00               ` Greg KH
2020-10-28 18:33                 ` Greg KH
2020-10-28 18:34                   ` Christoph Hellwig
2020-10-28 18:35                   ` [PATCH] proc "single files": switch to ->read_iter Greg Kroah-Hartman

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=20201027075541.GA24429@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.