vmsplice can't work well

vmsplice can't work well

[Yi Yang]

Hi, Jens

I try to trace vmsplice and find it can't work in both ppc64 and i386, it always return -EFAULT because of the address of iovec.iov_base no matter it is page alignment or not, I don't know if I should file a bug for it, do you test it on i386 or ppc64?

This is the test program I used.

/*
* Use vmsplice to fill some user memory into a pipe. vmsplice writes
* to stdout, so that must be a pipe.
*/
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>
#include <limits.h>
#include <sys/poll.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/uio.h>
#include <error.h>
#include <errno.h>

//#include "splice.h"
#if defined(__i386__)
#define __NR_splice 313
#define __NR_tee 315
#define __NR_vmsplice 316
#elif defined(__x86_64__)
#define __NR_splice 275
#define __NR_tee 276
#define __NR_vmsplice 277
#elif defined(__powerpc__) || defined(__powerpc64__)
#define __NR_splice 283
#define __NR_tee 284
#define __NR_vmsplice 285
#else
#error unsupported arch
#endif

#define SPLICE_SIZE 4096
#ifndef F_SETPSZ
#define F_SETPSZ 15 /* for pipes. */
#define F_GETPSZ 16 /* for pipes. */
#endif

#define ALIGN_BUF

#ifdef ALIGN_BUF
#define ALIGN_MASK (65535) /* 64k-1, should just be PAGE_SIZE - 1 */
#define ALIGN(buf) (void *) (((unsigned long) (buf) + ALIGN_MASK) & ~ALIGN_MASK)
#else
#define ALIGN_MASK (0)
#define ALIGN(buf) (buf)
#endif

#define min(a,b) (((a)>(b))?(b):(a))

static inline int xerror(const char * str)
{
	int err = errno;
	perror(str);
	return err;
}

static inline long vmsplice(int fd, const struct iovec *iov,
				unsigned long nr_segs, unsigned int flags)
{
	return syscall(__NR_vmsplice, fd, iov, nr_segs, flags);
}

int do_vmsplice(int fd, void *buffer, int len)
{
	struct pollfd pfd = { .fd = fd, .events = POLLOUT, };
	int written;
	struct iovec v;

	v.iov_base = buffer;
	v.iov_len = len;

	while (len) {
		/*
		 * in a real app you'd be more clever with poll of course,
		 * here we are basically just blocking on output room and
		 * not using the free time for anything interesting.
		 */
		if (poll(&pfd, 1, -1) < 0)
		return xerror("poll");

		written = vmsplice(fd, &v, 1, 0);
		printf("here: len = %d, written = %d\n", len, written);

		if (written <= 0)
			return xerror("vmsplice");
		fprintf(stderr, "written len = %d\n", written);

		len -= written;
	}

	return 0;
}

int main(int argc, char *argv[])
{
	unsigned char *buffer;
	struct stat sb;
	long page_size;
	int i, ret;

	if (fstat(STDOUT_FILENO, &sb) < 0)
		return xerror("stat");
	if (!S_ISFIFO(sb.st_mode)) {
		fprintf(stderr, "stdout must be a pipe\n");
		return 1;
	}

	page_size = sysconf(_SC_PAGESIZE);
	if (page_size < 0)
		return xerror("_SC_PAGESIZE");

	fprintf(stderr, "getpagesize = %d\n", getpagesize());
	fprintf(stderr, "page size: %d bytes\n", page_size);

	buffer = malloc(2 * 65536);
	buffer[0]='A';
	for (i = 1; i < 2 * SPLICE_SIZE; i++)
		buffer[i] = (i & 0xff);

	do {
		/*
		 * vmsplice the first half of the buffer into the pipe
		 */
		if (do_vmsplice(STDOUT_FILENO, buffer, SPLICE_SIZE))
			break;

		/*
		 * first half is now in pipe, but we don't quite know when
		 * we can reuse it.
		 */

		/*
		 * vmsplice second half
		 */
		//if (do_vmsplice(STDOUT_FILENO, buffer + SPLICE_SIZE, SPLICE_SIZE))
			// break;

		/*
		 * We still don't know when we can reuse the second half of
		 * the buffer, but we do now know that all parts of the first
		 * half have been consumed from the pipe - so we can reuse that.
		*/
	} while (0);

	free(buffer);

	return 0;
}

Re: vmsplice can't work well

[Jens Axboe]

On Tue, Aug 29 2006, Yi Yang wrote:
> Hi, Jens
> 
> I try to trace vmsplice and find it can't work in both ppc64 and i386,
> it always return -EFAULT because of the address of iovec.iov_base no
> matter it is page alignment or not, I don't know if I should file a
> bug for it, do you test it on i386 or ppc64?

Please provide an strace of the problem, it works fine for me (on x86-64
and x86, I've previously also tested ppc64 and ia64). Also please see
the splice tools here for more examples:

http://brick.kernel.dk/snaps/splice-git-20060711102502.tar.gz

I patched your program to fix the x86-64 syscall number and one/two
bugs, diff attached. Output for me:

axboe@nelson:/home/axboe $ ./f | cat > /dev/null
getpagesize = 4096
page size: 4096 bytes
written len = 4096

--- f.c~	2006-08-29 16:02:21.000000000 +0200
+++ f.c	2006-08-29 16:04:41.000000000 +0200
@@ -22,7 +22,7 @@
 #elif defined(__x86_64__)
 #define __NR_splice 275
 #define __NR_tee 276
-#define __NR_vmsplice 277
+#define __NR_vmsplice 278
 #elif defined(__powerpc__) || defined(__powerpc64__)
 #define __NR_splice 283
 #define __NR_tee 284
@@ -71,23 +71,26 @@
 	v.iov_base = buffer;
 	v.iov_len = len;
 
-	while (len) {
+	while (v.iov_len) {
 		/*
 		 * in a real app you'd be more clever with poll of course,
 		 * here we are basically just blocking on output room and
 		 * not using the free time for anything interesting.
 		 */
 		if (poll(&pfd, 1, -1) < 0)
-		return xerror("poll");
+			return xerror("poll");
 
 		written = vmsplice(fd, &v, 1, 0);
 		printf("here: len = %d, written = %d\n", len, written);
 
-		if (written <= 0)
+		if (!written)
+			break;
+		else if (written < 0)
 			return xerror("vmsplice");
 		fprintf(stderr, "written len = %d\n", written);
 
-		len -= written;
+		v.iov_len -= written;
+		v.iov_base += written;
 	}
 
 	return 0;
@@ -98,7 +101,7 @@
 	unsigned char *buffer;
 	struct stat sb;
 	long page_size;
-	int i, ret;
+	int i;
 
 	if (fstat(STDOUT_FILENO, &sb) < 0)
 		return xerror("stat");
@@ -112,7 +115,7 @@
 		return xerror("_SC_PAGESIZE");
 
 	fprintf(stderr, "getpagesize = %d\n", getpagesize());
-	fprintf(stderr, "page size: %d bytes\n", page_size);
+	fprintf(stderr, "page size: %d bytes\n", (int) page_size);
 
 	buffer = malloc(2 * 65536);
 	buffer[0]='A';

-- 
Jens Axboe

Re: vmsplice can't work well

[Jens Axboe]

XFS list,

On Wed, Aug 30 2006, Jeffrey E. Hundstad wrote:
> Jens Axboe wrote:
> >On Wed, Aug 30 2006, Jeffrey E. Hundstad wrote:
> >  
> >>I tried your splie-git...tar.gz file and tried the splice-cp.  It 
> >>produced files that are the right length... but the files only contain 
> >>nulls.  Here's the straces:
> >>    
> >
> >Works for me as well. Could be an fs issue, how large was the README and
> >what filesystem did you use?
> >
> >  
> The file was 1130 bytes (it was the README in that directory.)  The 
> filesystem is XFS.
> 

I can reproduce this quite easily, doing:

nelson:~ # splice-cp sda.blktrace.0 foo

nelson:~ # md5sum sda.blktrace.0 foo
4754070ae77091468c830ea23b125d68  sda.blktrace.0
efdc7b9d00692fdfe91a691277209267  foo

'foo' contains only zeroes. Doing the same on ext3 yields the correct
results, foo contains the right data. I'm testing on
2.16.18-rc5-current, Jeffrey used 2.6.17.x latest.

-- 
Jens Axboe

Re: vmsplice can't work well

[David Chinner]

On Thu, Aug 31, 2006 at 11:24:41AM +0200, Jens Axboe wrote:
> On Wed, Aug 30 2006, Jeffrey E. Hundstad wrote:
> > Jens Axboe wrote:
> > >On Wed, Aug 30 2006, Jeffrey E. Hundstad wrote:
> > >  
> > >>I tried your splie-git...tar.gz file and tried the splice-cp.  It
> > >>produced files that are the right length... but the files only contain
> > >>nulls.  Here's the straces:
> > >>    
> > >
> > >Works for me as well. Could be an fs issue, how large was the README and
> > >what filesystem did you use?
> > >
> > >  
> > The file was 1130 bytes (it was the README in that directory.)  The
> > filesystem is XFS.
> > 
> 
> I can reproduce this quite easily, doing:
> 
> nelson:~ # splice-cp sda.blktrace.0 foo
> 
> nelson:~ # md5sum sda.blktrace.0 foo 4754070ae77091468c830ea23b125d68
> sda.blktrace.0 efdc7b9d00692fdfe91a691277209267  foo

Not good.

> 'foo' contains only zeroes. Doing the same on ext3 yields the correct
> results, foo contains the right data. I'm testing on 2.16.18-rc5-current,
> Jeffrey used 2.6.17.x latest.

I had a quick look at the code and I can't see anything obviously wrong
in the XFS code. Where can I find the splice userspace application source,
Jens?

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

Re: vmsplice can't work well

[Nathan Scott]

On Fri, Sep 01, 2006 at 09:17:02AM +1000, David Chinner wrote:
> On Thu, Aug 31, 2006 at 11:24:41AM +0200, Jens Axboe wrote:
> I had a quick look at the code and I can't see anything obviously wrong
> in the XFS code. Where can I find the splice userspace application source,
> Jens?

http://brick.kernel.dk/snaps/

cheers.

-- 
Nathan

Re: vmsplice can't work well

[David Chinner]

On Thu, Aug 31, 2006 at 11:24:41AM +0200, Jens Axboe wrote:
> XFS list,
> 
> On Wed, Aug 30 2006, Jeffrey E. Hundstad wrote:
> > Jens Axboe wrote:
> > >On Wed, Aug 30 2006, Jeffrey E. Hundstad wrote:
> > >  
> > >>I tried your splie-git...tar.gz file and tried the splice-cp.  It 
> > >>produced files that are the right length... but the files only contain 
> > >>nulls.  Here's the straces:
> > >>    
> > >
> > >Works for me as well. Could be an fs issue, how large was the README and
> > >what filesystem did you use?
> > >
> > >  
> > The file was 1130 bytes (it was the README in that directory.)  The 
> > filesystem is XFS.
> > 
> 
> I can reproduce this quite easily, doing:
> 
> nelson:~ # splice-cp sda.blktrace.0 foo
> 
> nelson:~ # md5sum sda.blktrace.0 foo
> 4754070ae77091468c830ea23b125d68  sda.blktrace.0
> efdc7b9d00692fdfe91a691277209267  foo

Busted write side - splice-in works fine, splice-out is an alias
for /dev/zero. The reason it's full of NULLs:

death:/mnt# xfs_bmap -vv foo
foo: no extents
death:/mnt#

It's a hole.  Nothing has been flushed out to disk.

Interesting - the inode is leaving pipe_to_file() dirty, the page is
dirty, the buffer head is dirty, delay, mapped and uptodate. The
page is the only page in the radix tree and the radix tree is marked
dirty.

But it never gets flushed out. Even when I use dd to seek past the
first disk block and write further into the file, I still end up
with a hole in the range where the original splice write should
be which means it was no longer in the page cache.

Copying a large file I can see dirty memory increase to tens of
megabytes.  Nothing is going to disk, writeback is not going above
zero.  Interestingly, when the write completes, the size of the page
cache drops by almost exactly the size of the file being written -
almost like a truncate_inode_pages() is occuring on file close.

Oh, look - we _are_ tossing away all the pages on close.

xfs_splice_write() hasn't updated the xfs inode size when extending the
file. The linux inode  has the correct value, but xfs thinks that it's
only got a speculative allocation EOF (i.e. 0) so we invalidate it
before it gets to disk.

The patch below just copies some code out of xfs_write() where it updates
the xfs inode size and drops it in xfs_splice_write(). It's almost certainly not
the right fix, but the bucket under the pipe will now catch most of the
bits....

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group


---
 fs/xfs/linux-2.6/xfs_lrw.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_lrw.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_lrw.c	2006-08-31 16:17:47.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_lrw.c	2006-09-01 22:48:56.463190730 +1000
@@ -390,6 +390,8 @@ xfs_splice_write(
 	xfs_inode_t		*ip = XFS_BHVTOI(bdp);
 	xfs_mount_t		*mp = ip->i_mount;
 	ssize_t			ret;
+	struct inode		*inode = outfilp->f_mapping->host;
+	xfs_fsize_t		isize;
 
 	XFS_STATS_INC(xs_write_calls);
 	if (XFS_FORCED_SHUTDOWN(ip->i_mount))
@@ -416,6 +418,20 @@ xfs_splice_write(
 	if (ret > 0)
 		XFS_STATS_ADD(xs_write_bytes, ret);
 
+	isize = i_size_read(inode);
+	if (unlikely(ret < 0 && ret != -EFAULT && *ppos > isize))
+		*ppos = isize;
+
+	if (*ppos > ip->i_d.di_size) {
+		xfs_ilock(ip, XFS_ILOCK_EXCL);
+		if (*ppos > ip->i_d.di_size) {
+			ip->i_d.di_size = *ppos;
+			i_size_write(inode, *ppos);
+			ip->i_update_core = 1;
+			ip->i_update_size = 1;
+		}
+		xfs_iunlock(ip, XFS_ILOCK_EXCL);
+	}
 	xfs_iunlock(ip, XFS_IOLOCK_EXCL);
 	return ret;
 }

Re: vmsplice can't work well

[Jens Axboe]

On Fri, Sep 01 2006, David Chinner wrote:
> On Thu, Aug 31, 2006 at 11:24:41AM +0200, Jens Axboe wrote:
> > XFS list,
> > 
> > On Wed, Aug 30 2006, Jeffrey E. Hundstad wrote:
> > > Jens Axboe wrote:
> > > >On Wed, Aug 30 2006, Jeffrey E. Hundstad wrote:
> > > >  
> > > >>I tried your splie-git...tar.gz file and tried the splice-cp.  It 
> > > >>produced files that are the right length... but the files only contain 
> > > >>nulls.  Here's the straces:
> > > >>    
> > > >
> > > >Works for me as well. Could be an fs issue, how large was the README and
> > > >what filesystem did you use?
> > > >
> > > >  
> > > The file was 1130 bytes (it was the README in that directory.)  The 
> > > filesystem is XFS.
> > > 
> > 
> > I can reproduce this quite easily, doing:
> > 
> > nelson:~ # splice-cp sda.blktrace.0 foo
> > 
> > nelson:~ # md5sum sda.blktrace.0 foo
> > 4754070ae77091468c830ea23b125d68  sda.blktrace.0
> > efdc7b9d00692fdfe91a691277209267  foo
> 
> Busted write side - splice-in works fine, splice-out is an alias
> for /dev/zero. The reason it's full of NULLs:
> 
> death:/mnt# xfs_bmap -vv foo
> foo: no extents
> death:/mnt#
> 
> It's a hole.  Nothing has been flushed out to disk.
> 
> Interesting - the inode is leaving pipe_to_file() dirty, the page is
> dirty, the buffer head is dirty, delay, mapped and uptodate. The
> page is the only page in the radix tree and the radix tree is marked
> dirty.
> 
> But it never gets flushed out. Even when I use dd to seek past the
> first disk block and write further into the file, I still end up
> with a hole in the range where the original splice write should
> be which means it was no longer in the page cache.
> 
> Copying a large file I can see dirty memory increase to tens of
> megabytes.  Nothing is going to disk, writeback is not going above
> zero.  Interestingly, when the write completes, the size of the page
> cache drops by almost exactly the size of the file being written -
> almost like a truncate_inode_pages() is occuring on file close.
> 
> Oh, look - we _are_ tossing away all the pages on close.
> 
> xfs_splice_write() hasn't updated the xfs inode size when extending the
> file. The linux inode  has the correct value, but xfs thinks that it's
> only got a speculative allocation EOF (i.e. 0) so we invalidate it
> before it gets to disk.
> 
> The patch below just copies some code out of xfs_write() where it
> updates the xfs inode size and drops it in xfs_splice_write(). It's
> almost certainly not the right fix, but the bucket under the pipe will
> now catch most of the bits....

Good analysis and fix, Dave! I don't have time to test it right now,
perhaps Jeffrey can give it a shot? Will you make sure this gets into
2.6.18?

-- 
Jens Axboe

Re: vmsplice can't work well

[Jeffrey E. Hundstad]

David Chinner wrote:
> On Thu, Aug 31, 2006 at 11:24:41AM +0200, Jens Axboe wrote:
>   
>> XFS list,
>>
>> On Wed, Aug 30 2006, Jeffrey E. Hundstad wrote:
>>     
>>> Jens Axboe wrote:
>>>       
>>>> On Wed, Aug 30 2006, Jeffrey E. Hundstad wrote:
>>>>  
>>>>         
>>>>> I tried your splie-git...tar.gz file and tried the splice-cp.  It 
>>>>> produced files that are the right length... but the files only contain 
>>>>> nulls.  Here's the straces:
>>>>>    
>>>>>           
>>>> Works for me as well. Could be an fs issue, how large was the README and
>>>> what filesystem did you use?
>>>>
>>>>  
>>>>         
>>> The file was 1130 bytes (it was the README in that directory.)  The 
>>> filesystem is XFS.
>>>
>>>       
>> I can reproduce this quite easily, doing:
>>
>> nelson:~ # splice-cp sda.blktrace.0 foo
>>
>> nelson:~ # md5sum sda.blktrace.0 foo
>> 4754070ae77091468c830ea23b125d68  sda.blktrace.0
>> efdc7b9d00692fdfe91a691277209267  foo
>>     
>
> Busted write side - splice-in works fine, splice-out is an alias
> for /dev/zero. The reason it's full of NULLs:
>
> death:/mnt# xfs_bmap -vv foo
> foo: no extents
> death:/mnt#
>
> It's a hole.  Nothing has been flushed out to disk.
>
> Interesting - the inode is leaving pipe_to_file() dirty, the page is
> dirty, the buffer head is dirty, delay, mapped and uptodate. The
> page is the only page in the radix tree and the radix tree is marked
> dirty.
>
> But it never gets flushed out. Even when I use dd to seek past the
> first disk block and write further into the file, I still end up
> with a hole in the range where the original splice write should
> be which means it was no longer in the page cache.
>
> Copying a large file I can see dirty memory increase to tens of
> megabytes.  Nothing is going to disk, writeback is not going above
> zero.  Interestingly, when the write completes, the size of the page
> cache drops by almost exactly the size of the file being written -
> almost like a truncate_inode_pages() is occuring on file close.
>
> Oh, look - we _are_ tossing away all the pages on close.
>
> xfs_splice_write() hasn't updated the xfs inode size when extending the
> file. The linux inode  has the correct value, but xfs thinks that it's
> only got a speculative allocation EOF (i.e. 0) so we invalidate it
> before it gets to disk.
>
> The patch below just copies some code out of xfs_write() where it updates
> the xfs inode size and drops it in xfs_splice_write(). It's almost certainly not
> the right fix, but the bucket under the pipe will now catch most of the
> bits....
>
> Cheers,
>
> Dave.
>   

I can confirm that this patch allows splice-cp to work as expected!

Thanks all!

-- 
Jeffrey Hundstad