Re: Weird fuse_operations.read calls with Linux 5.4

[Miklos Szeredi <miklos@szeredi.hu>]

On Wed, Jan 15, 2020 at 01:24:52PM +0100, Ondrej Holy wrote:
> st 15. 1. 2020 v 12:41 odesílatel Miklos Szeredi <miklos@szeredi.hu> napsal:
> >
> > On Wed, Jan 15, 2020 at 9:28 AM Ondrej Holy <oholy@redhat.com> wrote:
> > >
> > > Hi,
> > >
> > > I have been directed here from https://github.com/libfuse/libfuse/issues/488.
> > >
> > > My issue is that with Linux Kernel 5.4, one read kernel call (e.g.
> > > made by cat tool) triggers two fuse_operations.read executions and in
> > > both cases with 0 offset even though that first read successfully
> > > returned some bytes.
> > >
> > > For gvfs, it leads to redundant I/O operations, or to "Operation not
> > > supported" errors if seeking is not supported. This doesn't happen
> > > with Linux 5.3. Any idea what is wrong here?
> > >
> > > $ strace cat /run/user/1000/gvfs/ftp\:host\=server\,user\=user/foo
> > > ...
> > > openat(AT_FDCWD, "/run/user/1000/gvfs/ftp:host=server,user=user/foo",
> >
> > Hi, I'm trying to reproduce this on fedora30, but even failing to get
> > that "cat" to work.  I've  replaced "server" with a public ftp server,
> > but it's not even getting to the ftp backend.  Is there a trick to
> > enable the ftp backend?  Haven't found the answer by googling...
> 
> Hi Miklos,
> 
> you need gvfs and gvfs-fuse packages installed. Then it should be
> enough to mount some share, e.g. over Nautilus, or using just "gio
> mount ftp://user@server/". Once some share is mounted, then you should
> see it in /run/user/$UID/gvfs. I can reproduce on Fedora 31 with
> kernel-5.4.10-200.fc31.x86_64, whereas kernel-5.3.16-300.fc31.x86_64
> works without any issues.

Thanks, I was missing the "gio mount ..." command.

Here's a patch that should fix it.  Will go into 5.5-rc7 and will be backported
to 5.4.x stable series.

Thanks,
Miklos

---
From: Miklos Szeredi <mszeredi@redhat.com>
Subject: fuse: fix fuse_send_readpages() in the syncronous read case

Buffered read in fuse normally goes via:

 -> generic_file_buffered_read()
   -> fuse_readpages()
     -> fuse_send_readpages()
       ->fuse_simple_request() [called since v5.4]

In the case of a read request, fuse_simple_request() will return a
non-negative bytecount on success or a negative error value.  A positive
bytecount was taken to be an error and the PG_error flag set on the page.
This resulted in generic_file_buffered_read() falling back to ->readpage(),
which would repeat the read request and succeed.  Because of the repeated
read succeeding the bug was not detected with regression tests or other use
cases.

The FTP module in GVFS however fails the second read due to the
non-seekable nature of FTP downloads.

Fix by checking and ignoring positive return value from
fuse_simple_request().

Reported-by: Ondrej Holy <oholy@redhat.com>
Link: https://gitlab.gnome.org/GNOME/gvfs/issues/441
Fixes: 134831e36bbd ("fuse: convert readpages to simple api")
Cc: <stable@vger.kernel.org> # v5.4
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/file.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -882,6 +882,7 @@ static void fuse_send_readpages(struct f
 	struct fuse_args_pages *ap = &ia->ap;
 	loff_t pos = page_offset(ap->pages[0]);
 	size_t count = ap->num_pages << PAGE_SHIFT;
+	ssize_t res;
 	int err;
 
 	ap->args.out_pages = true;
@@ -896,7 +897,8 @@ static void fuse_send_readpages(struct f
 		if (!err)
 			return;
 	} else {
-		err = fuse_simple_request(fc, &ap->args);
+		res = fuse_simple_request(fc, &ap->args);
+		err = res < 0 ? res : 0;
 	}
 	fuse_readpages_end(fc, &ap->args, err);
 }