[PATCH] add pread operation to vfs layer

James Peach jpeach at sgi.com
Wed Dec 17 02:39:57 GMT 2003


On Tue, 16 Dec 2003 15:27:47 -0800
"Cole, Timothy D." <t.cole at ngc.com> wrote:

> I wrote:
> > Would it be feasible to cache the effective and actual file 
> > offsets for a
> > file descriptor, to reduce the number of seeks needed for sequential
> > pwrites/preads?
> 
> Thinking about it, those offsets would likely have to be stored in the
> files_struct -- that's okay provided an fd with a files_struct attached
> can only ever be read/written/lsought via that single files_struct.
> 
> Is that a safe assumption to make, though?

This seems quite complicated and difficult to test. There is already pos and
position_information in files_struct, and adding a third player into this
isn't going to help the clarity of this code.

Another different option is to test for p* at runtime, which would allow
3rd-party vfs modules to add their owm p* ops (perhaps implementing your
caching idea). This patch (untested and cumulative wrt my p* one) hacks up
the vfs to allow operatios to be missing ...

--
James Peach | jpeach at sgi.com | SGI Australian Software Group
I don't speak for SGI.


===========================================================================
source/include/vfs_macros.h
===========================================================================

--- /usr/tmp/TmpDir.1294301-0/src/source/include/vfs_macros.h_1.5	Wed Dec 17 13:38:10 2003
+++ source/include/vfs_macros.h	Wed Dec 17 13:33:53 2003
@@ -27,6 +27,12 @@
  (Fixes should go also into the vfs_opaque_* and vfs_next_* macros!)
 ********************************************************************/
 
+extern int vfswrap_enosys(struct vfs_handle_struct *, struct files_struct *, ...);
+
+/* Define as operation as not present if there is no opaque version of it. */
+#define SMB_VFS_OP_MISSING(conn, op) \
+        (((void **)&(conn)->vfs_opaque.ops)[op] == (void *)vfswrap_enosys)
+
 /* Disk operations */    
 #define SMB_VFS_CONNECT(conn, service, user) ((conn)->vfs.ops.connect((conn)->vfs.handles.connect, (conn), (service), (user)))
 #define SMB_VFS_DISCONNECT(conn) ((conn)->vfs.ops.disconnect((conn)->vfs.handles.disconnect, (conn)))

===========================================================================
source/smbd/fileio.c
===========================================================================

--- /usr/tmp/TmpDir.1294301-0/src/source/smbd/fileio.c_1.5	Wed Dec 17 13:38:10 2003
+++ source/smbd/fileio.c	Wed Dec 17 13:36:00 2003
@@ -95,22 +95,20 @@
 
 	flush_write_cache(fsp, READ_FLUSH);
 
-#if !defined(HAVE_PREAD) && !defined(HAVE_PREAD64)
-	if (seek_file(fsp,pos) == -1) {
+	if (SMB_VFS_OP_MISSING(fsp->conn, SMB_VFS_OP_PREAD) &&
+	    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
+		readret = SMB_VFS_OP_MISSING(&fsp->conn, SMB_VFS_OP_PREAD) ?
+				SMB_VFS_READ(fsp,fsp->fd,data,n) :
+				SMB_VFS_PREAD(fsp,fsp->fd,data,n,pos);
+
 		if (readret == -1) {
 			if ((errno == EAGAIN) && numretries) {
 				DEBUG(3,("read_file EAGAIN retry in 10 seconds\n"));
@@ -121,11 +119,10 @@
 			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
+		readret = SMB_VFS_OP_MISSING(fsp->conn, SMB_VFS_OP_PREAD) ?
+				SMB_VFS_READ(fsp,fsp->fd,data,n) :
+				SMB_VFS_PREAD(fsp,fsp->fd,data,n,pos);
+
 		if (readret == -1)
 			return -1;
 #endif
@@ -153,17 +150,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;
+	if (SMB_VFS_OP_MISSING(fsp->conn, SMB_VFS_OP_PWRITE)) {
+		if ((pos != -1) && (seek_file(fsp,pos) == -1))
+			return -1;
 
-	ret = vfs_write_data(fsp,data,n);
-#endif
+		ret = vfs_write_data(fsp,data,n);
+	} else {
+		if (pos == -1)
+			ret = vfs_write_data(fsp, data, n);
+		else
+			ret = vfs_pwrite_data(fsp, data, n, pos);
+	}
 
 	DEBUG(10,("real_write_file (%s): pos = %.0f, size = %lu, returned %ld\n",
 		fsp->fsp_name, (double)pos, (unsigned long)n, (long)ret ));

===========================================================================
source/smbd/vfs-wrap.c
===========================================================================

--- /usr/tmp/TmpDir.1294301-0/src/source/smbd/vfs-wrap.c_1.6	Wed Dec 17 13:38:10 2003
+++ source/smbd/vfs-wrap.c	Wed Dec 17 12:56:16 2003
@@ -562,6 +562,12 @@
 	return 0;
 }
 
+int vfswrap_enosys(struct vfs_handle_struct * handle, struct files_struct * fsp, ...)
+{
+        errno = ENOSYS;
+        return -1;
+}
+
 int vfswrap_ftruncate(vfs_handle_struct *handle, files_struct *fsp, int fd, SMB_OFF_T len)
 {
 	int result = -1;

===========================================================================
source/smbd/vfs.c
===========================================================================

--- /usr/tmp/TmpDir.1294301-0/src/source/smbd/vfs.c_1.10	Wed Dec 17 13:38:10 2003
+++ source/smbd/vfs.c	Wed Dec 17 13:36:56 2003
@@ -46,6 +46,7 @@
    very important.  They must be in the same order as defined in
    vfs.h.  Change at your own peril. */
 
+
 static struct vfs_ops default_vfs = {
 
 	{
@@ -71,9 +72,17 @@
 		vfswrap_open,
 		vfswrap_close,
 		vfswrap_read,
+#if defined(HAVE_PREAD) || defined(HAVE_PREAD64)
 		vfswrap_pread,
+#else
+                vfswrap_enosys,
+#endif
 		vfswrap_write,
+#if defined(HAVE_PWRITE) || defined(HAVE_PWRITE64)
 		vfswrap_pwrite,
+#else
+                vfswrap_enosys,
+#endif
 		vfswrap_lseek,
 		vfswrap_sendfile,
 		vfswrap_rename,



More information about the samba-technical mailing list