Rework sendfile code to use a file operation

PATCH In review Christoph Hellwig 2 participants 2 messages Jul 15, 2002

Subsystems (11)

Memory management VFS Page cache ext2 file system +7 more subsystems

lore

Share & tools

lore

Christoph Hellwig

author

2002-07-15 12:49 UTC #0 lore raw

PATCH 1/1 Rework sendfile code to use a file operation

The current sendfile code only checks for the presence of a readpage method on the filesystem from which it sends but assumes it has the same locking semantics as generic_file_read. For all those filesystems that use the pagecache but use a different read implementation (or a wrapper around generic_file_read that does additional locking in most cases) this is broken and allows for races, depending on the locking performed. Filesystems for which this is an issue are tmpfs, smbfs, xfs and maybe others.

This patch adds a new sendfile member to the file operations vector and splits the current sendfile implementations in a higher level part (now in fs/read_write.c) and generic_file_sendfile that can be used for all filesystems using generic_file_read. I've also updated all filesystems using generic_file_read to use generic_file_sendfile aswell. Once this is in I will add proper sendfile support to XFS in the SGI CVS tree,

I guess the maintainers will fixup tmpfs and smbfs (at least tmpfs is very easy).

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas:

#	           ChangeSet	1.640   -> 1.641  
#	  fs/reiserfs/file.c	1.16    -> 1.17   
#	      kernel/ksyms.c	1.108   -> 1.109  
#	       fs/udf/file.c	1.10    -> 1.11   
#	       fs/bfs/file.c	1.9     -> 1.10   
#	      fs/adfs/file.c	1.5     -> 1.6    
#	fs/freevxfs/vxfs_inode.c	1.9     -> 1.10   
#	  include/linux/fs.h	1.144   -> 1.145  
#	       fs/ufs/file.c	1.7     -> 1.8    
#	       fs/jfs/file.c	1.6     -> 1.7    
#	     fs/hpfs/inode.c	1.10    -> 1.11   
#	      fs/ntfs/file.c	1.10    -> 1.11   
#	    fs/ramfs/inode.c	1.23    -> 1.24   
#	        mm/filemap.c	1.109   -> 1.110  
#	      fs/ext2/file.c	1.5     -> 1.6    
#	      fs/qnx4/file.c	1.3     -> 1.4    
#	     fs/minix/file.c	1.9     -> 1.10   
#	      fs/affs/file.c	1.19    -> 1.20   
#	 fs/jffs/inode-v23.c	1.33    -> 1.34   
#	     fs/read_write.c	1.9     -> 1.10   
#	      fs/sysv/file.c	1.9     -> 1.10   
#	      fs/ext3/file.c	1.4     -> 1.5    
#	     fs/jffs2/file.c	1.9     -> 1.10   
#	      fs/block_dev.c	1.68    -> 1.69

#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/07/15 [email protected] 1.641
# Rework sendfile code to use a file operation.
#
# This is needed because some filesystems need additional locking or

# want their own implementations of sendfile.
# --------------------------------------------
#
diff -Nru a/fs/adfs/file.c b/fs/adfs/file.c
--- a/fs/adfs/file.c	Mon Jul 15 15:37:03 2002
+++ b/fs/adfs/file.c	Mon Jul 15 15:37:03 2002
@@ -36,6 +36,7 @@
 	mmap:		generic_file_mmap,
 	fsync:		file_fsync,
 	write:		generic_file_write,
+	sendfile:	generic_file_sendfile,
 };

struct inode_operations adfs_file_inode_operations = {

fs/affs/file.c +1 -0

diff -Nru a/fs/affs/file.c b/fs/affs/file.c
@@ -50,6 +50,7 @@
 	open:		affs_file_open,
 	release:	affs_file_release,
 	fsync:		file_fsync,
+	sendfile:	generic_file_sendfile,
 };
 
 struct inode_operations affs_file_inode_operations = {

fs/bfs/file.c +5 -4

diff -Nru a/fs/bfs/file.c b/fs/bfs/file.c
@@ -18,10 +18,11 @@
 #endif
 
 struct file_operations bfs_file_operations = {
-	llseek: generic_file_llseek,
-	read:	generic_file_read,
-	write:	generic_file_write,
-	mmap:	generic_file_mmap,
+	llseek: 	generic_file_llseek,
+	read:		generic_file_read,
+	write:		generic_file_write,
+	mmap:		generic_file_mmap,
+	sendfile:	generic_file_sendfile,
 };
 
 static int bfs_move_block(unsigned long from, unsigned long to, struct super_block *sb)

fs/block_dev.c +1 -0

diff -Nru a/fs/block_dev.c b/fs/block_dev.c
@@ -731,6 +731,7 @@
 	mmap:		generic_file_mmap,
 	fsync:		block_fsync,
 	ioctl:		blkdev_ioctl,
+	sendfile:	generic_file_sendfile,
 };
 
 const char *__bdevname(kdev_t dev)

fs/ext2/file.c +1 -0

diff -Nru a/fs/ext2/file.c b/fs/ext2/file.c
@@ -46,6 +46,7 @@
 	open:		generic_file_open,
 	release:	ext2_release_file,
 	fsync:		ext2_sync_file,
+	sendfile:	generic_file_sendfile,
 };
 
 struct inode_operations ext2_file_inode_operations = {

fs/ext3/file.c +1 -0

diff -Nru a/fs/ext3/file.c b/fs/ext3/file.c
@@ -84,6 +84,7 @@
 	open:		ext3_open_file,		/* BKL not held.  Don't need */
 	release:	ext3_release_file,	/* BKL not held.  Don't need */
 	fsync:		ext3_sync_file,		/* BKL held */
+	sendfile:	generic_file_sendfile,	/* BKL not held.  Don't need */
 };
 
 struct inode_operations ext3_file_inode_operations = {

fs/freevxfs/vxfs_inode.c +1 -0

diff -Nru a/fs/freevxfs/vxfs_inode.c b/fs/freevxfs/vxfs_inode.c
@@ -53,6 +53,7 @@
 	.llseek =		generic_file_llseek,
 	.read =			generic_file_read,
 	.mmap =			generic_file_mmap,
+	.sendfile =		generic_file_sendfile,
 };

fs/hpfs/inode.c +1 -0

diff -Nru a/fs/hpfs/inode.c b/fs/hpfs/inode.c
@@ -21,6 +21,7 @@
 	open:		hpfs_open,
 	release:	hpfs_file_release,
 	fsync:		hpfs_file_fsync,
+	sendfile:	generic_file_sendfile,
 };
 
 static struct inode_operations hpfs_file_iops =

fs/jffs/inode-v23.c +8 -7

diff -Nru a/fs/jffs/inode-v23.c b/fs/jffs/inode-v23.c
@@ -1632,13 +1632,14 @@
 
 static struct file_operations jffs_file_operations =
 {
-	open:	generic_file_open,
-	llseek:	generic_file_llseek,
-	read:	generic_file_read,
-	write:	generic_file_write,
-	ioctl:	jffs_ioctl,
-	mmap:	generic_file_mmap,
-	fsync:	jffs_fsync,
+	open:		generic_file_open,
+	llseek:		generic_file_llseek,
+	read:		generic_file_read,
+	write:		generic_file_write,
+	ioctl:		jffs_ioctl,
+	mmap:		generic_file_mmap,
+	fsync:		jffs_fsync,
+	sendfile:	generic_file_sendfile,
 };

fs/jffs2/file.c +1 -0

diff -Nru a/fs/jffs2/file.c b/fs/jffs2/file.c
@@ -82,6 +82,7 @@
 	ioctl:		jffs2_ioctl,
 	mmap:		generic_file_mmap,
 	fsync:		jffs2_fsync
+	sendfile:	generic_file_sendfile,
 };
 
 /* jffs2_file_inode_operations */

fs/jfs/file.c +1 -0

diff -Nru a/fs/jfs/file.c b/fs/jfs/file.c
@@ -52,6 +52,7 @@
 	read:		generic_file_read,
 	mmap:		generic_file_mmap,
 	fsync:		jfs_fsync,
+	sendfile:	generic_file_sendfile,
 };
 
 /*

fs/minix/file.c +1 -0

diff -Nru a/fs/minix/file.c b/fs/minix/file.c
@@ -21,6 +21,7 @@
 	write:		generic_file_write,
 	mmap:		generic_file_mmap,
 	fsync:		minix_sync_file,
+	sendfile:	generic_file_sendfile,
 };
 
 struct inode_operations minix_file_inode_operations = {

fs/ntfs/file.c +1 -0

diff -Nru a/fs/ntfs/file.c b/fs/ntfs/file.c
@@ -26,6 +26,7 @@
 	read:			generic_file_read,	/* Read from file. */
 	mmap:			generic_file_mmap,	/* Mmap file. */
 	open:			generic_file_open,	/* Open file. */
+	sendfile:		generic_file_sendfile,	/* Zero-copy data send */
 };
 
 struct inode_operations ntfs_file_inode_ops = {};

fs/qnx4/file.c +1 -0

diff -Nru a/fs/qnx4/file.c b/fs/qnx4/file.c
@@ -33,6 +33,7 @@
 #ifdef CONFIG_QNX4FS_RW
 	fsync:			qnx4_sync_file,
 #endif
+	sendfile:		generic_file_sendfile,
 };
 
 struct inode_operations qnx4_file_inode_operations =

fs/ramfs/inode.c +1 -0

diff -Nru a/fs/ramfs/inode.c b/fs/ramfs/inode.c
@@ -276,6 +276,7 @@
 	write:		generic_file_write,
 	mmap:		generic_file_mmap,
 	fsync:		ramfs_sync_file,
+	sendfile:	generic_file_sendfile,
 };
 
 static struct inode_operations ramfs_dir_inode_operations = {

fs/read_write.c +111 -0

121 lines not on this page · raw message

fs/reiserfs/file.c +1 -0

diff -Nru a/fs/reiserfs/file.c b/fs/reiserfs/file.c
@@ -147,6 +147,7 @@
     mmap:	generic_file_mmap,
     release:	reiserfs_file_release,
     fsync:	reiserfs_sync_file,
+    sendfile:	generic_file_sendfile,
 };

fs/sysv/file.c +1 -0

diff -Nru a/fs/sysv/file.c b/fs/sysv/file.c
@@ -25,6 +25,7 @@
 	write:		generic_file_write,
 	mmap:		generic_file_mmap,
 	fsync:		sysv_sync_file,
+	sendfile:	generic_file_sendfile,
 };
 
 struct inode_operations sysv_file_inode_operations = {

fs/udf/file.c +1 -0

diff -Nru a/fs/udf/file.c b/fs/udf/file.c
@@ -365,6 +365,7 @@
 	write:			udf_file_write,
 	release:		udf_release_file,
 	fsync:			udf_fsync_file,
+	sendfile:		generic_file_sendfile,
 };
 
 struct inode_operations udf_file_inode_operations = {

fs/ufs/file.c +1 -0

diff -Nru a/fs/ufs/file.c b/fs/ufs/file.c
@@ -47,6 +47,7 @@
 	write:		generic_file_write,
 	mmap:		generic_file_mmap,
 	open:           generic_file_open,
+	sendfile:	generic_file_sendfile,
 };
 
 struct inode_operations ufs_file_inode_operations = {

include/linux/fs.h +2 -0

diff -Nru a/include/linux/fs.h b/include/linux/fs.h
@@ -750,6 +750,7 @@
 	int (*lock) (struct file *, int, struct file_lock *);
 	ssize_t (*readv) (struct file *, const struct iovec *, unsigned long, loff_t *);
 	ssize_t (*writev) (struct file *, const struct iovec *, unsigned long, loff_t *);
+	ssize_t (*sendfile) (struct file *, struct file *, loff_t *, size_t);
 	ssize_t (*sendpage) (struct file *, struct page *, int, size_t, loff_t *, int);
 	unsigned long (*get_unmapped_area)(struct file *, unsigned long, unsigned long, unsigned long, unsigned long);
 };

@@ -1228,6 +1229,7 @@
 extern int file_read_actor(read_descriptor_t * desc, struct page *page, unsigned long offset, unsigned long size);
 extern ssize_t generic_file_read(struct file *, char *, size_t, loff_t *);
 extern ssize_t generic_file_write(struct file *, const char *, size_t, loff_t *);
+extern ssize_t generic_file_sendfile(struct file *, struct file *, loff_t *, size_t);
 extern void do_generic_file_read(struct file *, loff_t *, read_descriptor_t *, read_actor_t);
 extern ssize_t generic_file_direct_IO(int, struct inode *, char *, loff_t, size_t);
 extern int generic_direct_IO(struct address_space *, struct dio *, get_block_t *);

kernel/ksyms.c +1 -0

diff -Nru a/kernel/ksyms.c b/kernel/ksyms.c
@@ -218,6 +218,7 @@
 EXPORT_SYMBOL(block_truncate_page);
 EXPORT_SYMBOL(generic_block_bmap);
 EXPORT_SYMBOL(generic_file_read);
+EXPORT_SYMBOL(generic_file_sendfile);
 EXPORT_SYMBOL(do_generic_file_read);
 EXPORT_SYMBOL(generic_file_write);
 EXPORT_SYMBOL(generic_file_mmap);

mm/filemap.c +14 ~~-110~~

133 lines not on this page · raw message

Urban Widmark

2002-07-15 17:45 UTC #1 lore raw

Quoted 4 lines · Christoph Hellwig

splits the current sendfile implementations in a higher level part (now in fs/read_write.c) and generic_file_sendfile that can be used for all filesystems using generic_file_read. I've also updated all filesystems using generic_file_read to use generic_file_sendfile aswell. Once this

smbfs uses generic_file_read, it just does a revalidate first.

Do you have some testprogram you used on your changes? I think the code for smbfs should be like below, but I'd like to test it.

static ssize_t
smb_file_sendfile(struct file *in_file, struct file *out_file,
loff_t *ppos, size_t count)

{
	struct dentry *dentry = in_file->f_dentry;
	ssize_t	status;

	status = smb_revalidate_inode(dentry);
	if (status) {
		goto out;
	}

	status = generic_file_sendfile(in_file, out_file, ppos, count);
out:
	return status;
}

/Urban