[PATCH] add pread operation to vfs layer

Stefan Metzmacher metze at metzemix.de
Tue Dec 16 07:37:14 GMT 2003


James Peach wrote:

>Hi all,
>
>The following diff adds pread/pwrite operations to the VFS layer and makes
>use of them in the I/O path. This corresponds to bugzilla bug #889.
>
>There is not much performance advanage additional to using spinlocks (a
>consistent 2% - 7% increase in throughput), but there is a significant
>benefit to using p{read,write} with fcntl tdb locking (5% - 30% increase in
>throughput). The largest improvements are in high packet rate workloads (ie,
>small blocks sizes and meta-data workloads), as you might expect. I can
>provide detailed numbers if that would be useful.
>
>For systems without pread/pwrite (are there many of these?), the
>p{read,write} operations are emulated by a lseek/lseek/{read,write}/lseek
>sequence. The file I/O path for these systems continues to use the old
>lseek/read code.
>
>cheers
>
>--
>James Peach | jpeach at sgi.com
>
>
>Index: samba/source/smbd/fileio.c
>===================================================================
>RCS file: /cvsroot/samba/source/smbd/fileio.c,v
>retrieving revision 1.40.2.11
>diff -u -r1.40.2.11 fileio.c
>--- samba/source/smbd/fileio.c	2 Nov 2003 17:10:12 -0000	1.40.2.11
>+++ samba/source/smbd/fileio.c	16 Dec 2003 04:27:26 -0000
>@@ -95,16 +95,22 @@
> 
> 	flush_write_cache(fsp, READ_FLUSH);
> 
>+#if !defined(HAVE_PREAD) && !defined(HAVE_PREAD64)
> 	if (seek_file(fsp,pos) == -1) {
> 		DEBUG(3,("read_file: Failed to seek to %.0f\n",(double)pos));
> 		return(ret);
> 	}
>-  
>+#endif
>+
> 	if (n > 0) {
> #ifdef DMF_FIX
> 		int numretries = 3;
> tryagain:
>+#if defined(HAVE_PREAD) || defined(HAVE_PREAD64)
>+		readret = SMB_VFS_PREAD(fsp,fsp->fd,data,n,pos);
>+#else
> 		readret = SMB_VFS_READ(fsp,fsp->fd,data,n);
>+#endif
> 		if (readret == -1) {
> 			if ((errno == EAGAIN) && numretries) {
> 				DEBUG(3,("read_file EAGAIN retry in 10 seconds\n"));
>@@ -115,7 +121,11 @@
> 			return -1;
> 		}
> #else /* NO DMF fix. */
>+#if defined(HAVE_PREAD) || defined(HAVE_PREAD64)
>+		readret = SMB_VFS_PREAD(fsp,fsp->fd,data,n,pos);
>+#else
> 		readret = SMB_VFS_READ(fsp,fsp->fd,data,n);
>+#endif
> 		if (readret == -1)
> 			return -1;
> #endif
>@@ -143,10 +153,17 @@
> {
> 	ssize_t ret;
> 
>+#if defined(HAVE_PREAD) || defined(HAVE_PREAD64)
>+        if (pos == -1)
>+                ret = vfs_write_data(fsp, data, n);
>+        else
>+                ret = vfs_pwrite_data(fsp, data, n, pos);
>+#else
> 	if ((pos != -1) && (seek_file(fsp,pos) == -1))
> 		return -1;
> 
> 	ret = vfs_write_data(fsp,data,n);
>+#endif
> 
> 	DEBUG(10,("real_write_file (%s): pos = %.0f, size = %lu, returned %ld\n",
> 		fsp->fsp_name, (double)pos, (unsigned long)n, (long)ret ));
>  
>
I'd really don't like this #ifdef's and since we have the 
vfswrap_pread/vfswrap_pwrite functions which emulate pread/pwrite I 
don't see the need
of the #ifdef's in this file

-- 

metze

-------------------------------------------
Stefan (metze) Metzmacher <metze at metzemix.de>




More information about the samba-technical mailing list