Optimization with sendfile

[Shlomi Yaakobovich]

Hi,

> > We have been doing some performance tests (streaming 
> read/write), and I've noticed that whenever we use sendfile 
> (in reply.c/send_file_readX), there is a call to 
> SMB_VFS_FSTAT before the actual sendfile call. If I 
> understand it correctly, then this is to figure out what size 
> of data we're going to handle, and if the file has enough 
> data to send (so we can build an appropriate header).
> 
> Yes.

My suggestion was to get rid of the stat call before the sendfile. It is required only once, and from then on the value can be used. For example, consider this naive patch:
=======================================================
--- smb.h       2006-09-07 09:53:23.000000000 +0300
+++ patch_sendfile/smb.h        2006-09-07 09:54:36.000000000 +0300
@@ -426,6 +426,7 @@
        uint16 rap_print_jobid;
        SMB_DEV_T dev;
        SMB_INO_T inode;
+       SMB_BIG_UINT size_for_sendfile; /* For sendfile optimization. */
        SMB_BIG_UINT initial_allocation_size; /* Faked up initial allocation on disk. */
        mode_t mode;
        uint16 file_pid;
--- reply.c     2006-09-07 09:42:12.000000000 +0300
+++ patch_sendfile/reply.c      2006-09-07 09:50:42.000000000 +0300
@@ -2528,8 +2528,13 @@
                SMB_STRUCT_STAT sbuf;
                DATA_BLOB header;

-               if(SMB_VFS_FSTAT(fsp,fsp->fh->fd, &sbuf) == -1)
-                       return(UNIXERROR(ERRDOS,ERRnoaccess));
+               if (fsp->size_for_sendfile && (EXCLUSIVE_OPLOCK_TYPE(fsp->oplock_type)||LEVEL_II_OPLOCK_TYPE(fsp->oplock_type) )) {
+                       sbuf.st_size = fsp->size_for_sendfile;
+               } else {
+                       if(SMB_VFS_FSTAT(fsp,fsp->fh->fd, &sbuf) == -1)
+                               return(UNIXERROR(ERRDOS,ERRnoaccess));
+                       fsp->size_for_sendfile = sbuf.st_size;
+               }

                if (startpos > sbuf.st_size)
                        goto normal_read;
--- files.c     2006-09-07 10:04:56.000000000 +0300
+++ patch_sendfile/files.c      2006-09-07 10:06:28.000000000 +0300
@@ -103,6 +103,7 @@
        fsp->fh->ref_count = 1;
        fsp->fh->fd = -1;

+       fsp->size_for_sendfile = 0;
        fsp->conn = conn;
        fsp->fh->file_id = get_gen_count();
        GetTimeOfDay(&fsp->open_time);
=======================================================
This extra flag,  size_for_sendfile, will be set to 0 initially. In the first call to sendfile, stat will be performed, but further calls to sendfile will not issue another any stat call. To be safe (paranoid ?) I've also checked oplocks here.
The complete patch should also include a check after each write/truncate to the file, resetting the size_for_sendfile parameter to 0 again, to force another stat call (since the size has changed, either grown due to a write or truncated). If this is interesting and sounds logical, I will prepare a full patch and send it.

Shlomi