Rework sendfile code to use a file operation (3rd resend)

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.

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

BK patch below:

--
You can import this changeset into BK by piping this whole message to:
'| bk receive [path to repository]' or apply the patch as usual.

===================================================================

[email protected], 2002-07-27 17:24:07+02:00, [email protected]
VFS: implement sendfile file operation

Currently the sendfile syscalls hardcode assumptions about the implementation of the read file operations implementation. Although it checks for the presence of a readpage address-space operation filesystems in Linux are free to implement read differently from the generic version (generic_file_read). Many filesystems such as tmpfs, smbfs or xfs chose to implement it differently and need additional locking, revalidation or checks.

 fs/adfs/file.c           |    1 
 fs/affs/file.c           |    1 
 fs/bfs/file.c            |    9 +--
 fs/block_dev.c           |    1 
 fs/ext2/file.c           |    1 
 fs/ext3/file.c           |    1 
 fs/freevxfs/vxfs_inode.c |    1 
 fs/hpfs/inode.c          |    1 
 fs/jffs/inode-v23.c      |   15 ++---
 fs/jffs2/file.c          |    1 
 fs/jfs/file.c            |    1 
 fs/minix/file.c          |    1 
 fs/ntfs/file.c           |    1 
 fs/qnx4/file.c           |    1 
 fs/ramfs/inode.c         |    1 
 fs/read_write.c          |  111 +++++++++++++++++++++++++++++++++++++++
 fs/reiserfs/file.c       |    1 
 fs/sysv/file.c           |    1 
 fs/udf/file.c            |    1 
 fs/ufs/file.c            |    1 
 include/linux/fs.h       |    2 
 kernel/ksyms.c           |    1 
 mm/filemap.c             |  132 ++++-------------------------------------------
 23 files changed, 158 insertions, 129 deletions

diff -Nru a/fs/adfs/file.c b/fs/adfs/file.c
@@ -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 = {

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 = {

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)

diff -Nru a/fs/block_dev.c b/fs/block_dev.c
@@ -774,6 +774,7 @@
 	mmap:		generic_file_mmap,
 	fsync:		block_fsync,
 	ioctl:		blkdev_ioctl,
+	sendfile:	generic_file_sendfile,
 };
 
 int ioctl_by_bdev(struct block_device *bdev, unsigned cmd, unsigned long arg)

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 = {

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 = {

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,
 };
 
 

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 =

diff -Nru a/fs/jffs/inode-v23.c b/fs/jffs/inode-v23.c
@@ -1631,13 +1631,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,
 };
 
 

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 */

diff -Nru a/fs/jfs/file.c b/fs/jfs/file.c
@@ -98,5 +98,6 @@
 	.write		= generic_file_write,
 	.read		= generic_file_read,
 	.mmap		= generic_file_mmap,
+ 	.sendfile	= generic_file_sendfile,
 	.fsync		= jfs_fsync,
 };

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 = {

diff -Nru a/fs/ntfs/file.c b/fs/ntfs/file.c
@@ -52,6 +52,7 @@
 	llseek:		generic_file_llseek,	/* Seek inside file. */
 	read:		generic_file_read,	/* Read from file. */
 	mmap:		generic_file_mmap,	/* Mmap file. */
+ 	sendfile:	generic_file_sendfile,	/* Zero-copy data send */
 	open:		ntfs_file_open,		/* Open file. */
 };
 

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 =

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 = {

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,
 };
 
 

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 = {

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 = {

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 = {

diff -Nru a/include/linux/fs.h b/include/linux/fs.h
@@ -757,6 +757,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);
 };

@@ -1234,6 +1235,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);
 ssize_t generic_file_direct_IO(int rw, struct inode *inode, char *buf,
 				loff_t offset, size_t count);

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

===================================================================

This BitKeeper patch contains the following changesets:
1.477
## Wrapped with gzip_uu ##

begin 664 bkpatch8946 M'XL(`+CP0ST``]5<>W/;-A+_6_P4N.E,SDXM&2\2I'/..&F2.T_=II,^[C&= MT8D:*F62!U)^>(>[[O?A0E2B0M2FDFXSST,'%[OX6NXL%J*_0SYE*+P:3 M8&)]A?Z69/G%(/-'?A8F\6P:JU&2WL*%#TD"%\Y3M4C.H7GY<4A'M@47?I!Y M,$'W*LTN!F3$UBWYPT)=##Z\_>O/-Z\^6-;E)?IF(N-;]:/*T>6EE2?IO9R% MV97,)[,D'N6IC+.YRN4H2.;%NFM!,:;PUR:"8=LIB(.Y*(2$B(Y42&FW'6X MM9#+V3*[RN3<EYKA70*""LR80VF!B>-AZPTB(RX$PO0<BW,J$!$7E%]@\36F M%Q@C4,;5KA+0UY2A(;9>HS^6]V^L/WR[L<+-)TO9FJNXAQE*@ZCZ4PA\Y(L M5"KS:1)#1_CWS3)-H=/L>43M>F:/62!G,TR-)%I&"2A0C++EO.%OC%#TD^6 MN;X!"*S',411$AE"J9+ASGC93M<10J]F^219WDXTF1P%$Q7<92A*4D-BD2I@ M)U":I#0$%_(6^A#N)-LX4,MJ71PP'?N9K#4#&ZT7:%9IRITJ!FFLJ,>R% MTRA2*^&C-)D#"3WNK8I5.@V,$6J)3E8-8TU_K.\\!=:_D_'#UHC9,M!RRSE M\T64G:%L[D<9`F$^PELP2;(='D#B.@<R#E&L5*AIA.%4RR1G:)8$=]/X]@PX M!B.9ABLEIRMEC9#U+7*8*ZP?-A/"&A[XQ[*PQ-;+/98XGY]K>>=R,0KJQN@Q MK\"<,U&$4OK*B^#5=X3KAZV&WT('YA.Q*:&X<!@C?"\G=RJ-U>S\+GN89TU> MX!VS0H2"1:X,0CMRJ2>"=E[:*&VX81AS>R\W47;^6P0OTQBFR?">LB9+3%"/ M%5$HIBTS(X\%0:"M;/426[%%P$).67[\9K&P6P9JM*_GD?9:+++%G%L0,W! MRG/=4$11Z/C*:>>JB]B&*8=3V^VC+%_;]#A4][MZ<L&=>LPNA..[A&#;)UC* M@'>841NE#3?,Q9Z&[O6WNF.<PXLV.NA(/($Q@;\,>H#EPA=6J$A%WUMX2KJ MTZ;FCJEG2$)>.1^"M@AL0&$,L'A%?QYY-#(#Z52=D2[Y>_DA3)&G3Z\J(\Y M;67&!7DXA!E/4<'#D2NE"&U.WEI$-HPPQGG^\T5:,BP0S,N".0Z)-R)>$N M)\P//>)V<].@5(/)=FQM&0N=5W2RHD/&/3CN<_TR-G.QSI1+'.JUZ/8+;Q0 MA`$0MQV?.QZ/.IEZA&:-/4?07E:T#*,.*R+<HZ!U5S'IALQW!5<B[-;5+J&:
M%>DLIP\O.BB._Y-.\Q;D,"A+%#9UN()<Q8W"2+C"[>2F26K##V:>2]^Y+SR MH$WU<$X\I[!MQD$SGB!"<$:Z)UD+K1I'#G=ZH26C3M-FG&!<1($G?&8[$GNA M=%FW%34HU>("!LWVX6:RZ%(/3'M;Z+Q22$%M)IDC?$A'.L)G*ZFZ=C6>Z=:
MJJ:P9&A3CXT9+2A#>M?.$+.Q^F%'$NHVYE5@M2#%P)'TT]._X(^_B[L0 M?V&2,<7`+Y(0+#IR_$Z.&I1J<YT*F^[5C\X&&CZ:PK32>1=$;E$007WN1V#) M*O1=W@U6DU1]LMO"63-S-U/3.SE?C.0RRZ?Q:.K/-_SL0$4IL3&Q28&%ZS+0 M,=@R"\%UV*[MVX^E.9WA2X!\>Q4#(8<U]:(1!DYL&T,@M5WFN@$!5@+LDL>" M%^M4BR>\/A8#ZX#[KDCJ84(*5ZC(Q1F"*&.DMTNL$&IQ@WWO%ZA=#Z-IQ\[ MV,&.:XO"#\+<8BG'"<(/=7)39-2C1V;V*Q7L.KV?H0+\%R.X%)BQ51A:-$ M=[#JM!G&./5,96[3=/[email protected]$M.)257@805JS^*80W>F@&:U0+@"HIE$UT M-0#BR58Q@%P0[]%B/E<M8A6F;K-6VYA*T6^;!T+'/6]VB8_L?\@[7@#SOZ M.V(U>6US1"PTJ:Z&+2.?S8X?X[^I=)D&"2+!P1K6VD*$.CY>85C;6)J'/]@ M?V!I+*9QE,H0LHY.9Z"=K<-A+0BQT>:.`=A^&OB6WJN);TW&8_!U'<"W%[RO MO[U!<9*CB9J%(X3>)/&?<U/FJ('<FA?OA_L34O1VX!\A:%-&/!M\54$@;;3+ M.8Z?A@V4*XJF#;2*>]1LM[4UC-:UP\M!AT%4@-?3D/TX'Y[_6/)N,;_Z?;K0 M)""#::?"V0I8N)]3]J0P+9.U)J9U*8^:V+S/Q*YPW$FV]T-Y3*IOW8*=QE>@ M05_&X2B6<UUR?92BS3"DYZ#BPO8\IW381#P-9,L52A/9'4&/9=P3X=E^-,7 MX-]:L/T#5P96-I&WMP^'D!0P7VWF4E902!]+1^P^#63+14W;G/TD4$%,G6NM MW>_@LIV#-:BUVMC^K;-CJG)6*._5;U<9*&,4JE82$$K!Z;I:G=3&U,#H/T8 MR_IA$\::C,?@R-Q#/&^M"-0/PT/+3^T94H-,E1?1@@M!R^E(GT@,+:ME+4!& MGP2D30\!TC\,QT.W#MIAW*6R01$\%!/'94(V&O(OX57-3D<31?]30'Q#">+6 M->7(M@:S6:;4W07:1K)L/;,&NF!]L9/QZC:X9,K8N]=,(UR<S^5B]YIN.SO M>#;[7_VLY]"=-^M^FB97<ZW6F3+D0J7'=MNQIN#_3#N\=*;"_8TO$"Y4=AB M/QLAC_$"0O1:(M?J'/00/W#HMETS)+=0\"CA$-L*CJDKGE)(+C<86ZL<G[(8 MX@>%Y/I&2"\,#]^$:??F33J5/[<+0-+FI3\G3P-+LV74A+(NY#%84G8(EKL' M,'KA>=PAD'87VW4"1/M93^_T$5CA&ES9H95(%PW%E\BVS)F5]HI%7=)CHC5Q M&$<"UD(.Q\BU!LE"Q;NA5;>=K8/YX\.YM,DR&=P44LS-E\>C_!1]A'57_S MY8"P7]^1Z669!V\&M3N:)IF-HZ'P'3^M$IK9NVH:9%W*S^]I:KO#O:\>%^Z M&?E;2)217Q1Z+VQUG/5I@%CNH#=!K,EXU&+<WL9P7QVT?D2D%XQ''%#9$)TD M<]6+I,".KI+K<\H,)FFY.G\BR)J#-2UET+J41TU/<5!65S^.U_9@X]"Z0+W M[=5=FLA)-Q'(XG+78P+APL7'[G)3#XCF"WGS3>'R%5JUEJZ.-A6VMY(>Q2D MI!>BUYR!G@BV_F?]:F7Z#'JLFSZNQKG*$S6'4^F($*RS,=1>(;TYVEL/LZ2 M*(*>SQ>+)(-@/1CH.GIY=YLXWS=8RX_GEK_!8;R=!GDY2'XYX:*9@,^&N+P M^<6ZC['GLI/Y6/4R7Z#;BC*,K.]9C9JJ'$!\<(,SI];`[CEKTJ36BS+0<_, MD?)[4$6D#_=+<^)\FJ'D;C32W<]UNJ-)H$LT?/OZU9MW0'S%)C1%MRH_,;*? MZO8(G?QI=?$4I+]-\D2S6%TZ65T;OHS&<RW,,_3NN_=OWHX_O'WUYG1]1[0P M4KW8&OKZ^U]>W91CEXJX1#5J(9A4^C!\&:ZU4?%BOK>0KO.J*20+5!1HNV7X MLL+[,=YT#2;ERH<2[#3DW<W[[_Y=OS+VP_7[_YI9#NK@;;&N#:4MI;20"HU MEM2;PV[C"+K])"K(ZN0+"UZ#65UN4M])U6'!J!__W#]T]L=1$M#:(.T/E8- MB^VF$@S]=$<;V?5$,KU^YJ&T1<W(\+99H:=K>F>;8W0#SO#Y:\K6;5WT)?U M.S#R;-L2-OV,DQ@,XUZ0<IY4EG1\.5TG/F@D3%<\Q]RE=48;5X\-23+P8QK MZL9A"9'@3LT>3G3OOR#<BF&CX]>EM!+X]=.$3BV^@#O0:6@V[_#K>9>?<O+ M2[02KT%^4!(S'2!FE/QJ?[RAV3%-3S80K:?98@N@E6J-%E;L=O'ZJU7Q=%K M,[email protected](YO1=6V1T(+M-X[7TU\S*;0Y"]TT\G5<X90M_>B+(**/"6 M*0@?]6!BLB6UR\_P^MV!%B+75(I\=D"$2;_&!*M5/<P8^IN!OM*E.';=Z]^ MOOE)PU,:4CF*O@Q?ZJ%Q(X1Y>[;1_QGZ[M4_QM^__WY\\^['TQ>[3&B5&B;, M'7N8V&BX9ARZY1%6OO_YYJ:#E1X(.7Q?U.\'TD'Z$-O8_W<6:F99S75E.,D MT4E)_?0Q51T$?[2P&@&5HEU[9!KK[SZX..U[86U!AF;4LC*X7\!_X_=^_I2 M)0QS&KB94->$/&Z)=,@*:?/X2"\<#WULI;T4M4NE*D3ILV!$/*V*=_F431/% MC8Q'E3<<5-8W.PNA^,!Q[H[H"QL1%=GK1U"H;AMJ=TNJ<\?MZ"XB<=)CG MMOI-Q>83FON!//(1T=V*1??#H571PL;@JLI9:1_J7.EGK%G"B()EX%ZY!GY MH7X<&RU2R&3UCQ"8Q].[BU;Z&=@=(VAJYZ@-9;LTA56&<?)\O7Y%)UN5!L@= MMK]6*4R5O$,OB:4Z?UI]3%7$+XKHJUB'4Y=6^/V@]7[+?&81[H?K8BV/MF- M;6S;7!]<<QRGBO7>KCU2_*4<C/JX2-(.'+2/,<^C[YC7MJ!'Q7NJCQ"^_<</ M[S_\-/[QG]^]?G]STLK""MOZ_S[D3W\9P.L#&CETVPF_:R#A*#-'@6I["Y MZZ[.;I/&5N@^)#D:$N)^)C!3-4_NP9?,PH[?R&C42=?>IOMPL/YYA!T#J&OG MJ-U30O61M6OS3JT#G,%Z(;[5NEZHKXJD\&>K=KJSAM+C<\3,^.;,C"D'ARH+ MTND"<-'56?CRPG1T$<!UK8^X*=E)65%9KTZP:8G%\ASM)_C%'G60%,8Z1)S MKF)8,$&?LJDJ1YCWJM%?1M!T$DQDBIZ?UJJUYJI*4X@A@8L@&#ET]@L/MDI M4#S3]YVAM2+',@"YJI)2G;6:&/7F%]96J^'@Q>;7<LJ?EG.+P/7\7PNA/5_ (U8]%(9'````
`
end