To both lists, since this patch affects APR and sendfile.

The attached patch introduces the EnableSendfile directive for httpd.  Jeff and
I have seen several cases, including NFS shares and so forth, that are not
compatible with apr_sendfile.  It works similarly to EnableMMAP but the
differences are worth discussing.  {This is why I'm posting and not yet ready
to commit.}

The EnableMMAP worked by adding a flag to the bucket.  However, it seems
very silly to do so when what we really want to toggle is the file.

We also have an issue on Win32, we weren't using the correct open file semantics.
This may have had something to do with BSoD (kernel panic) using some socket
drivers on Win32.  Opening for TransmitFile should be done with sequential
optimizations and I believe it must be opened overlapped (that XTHREAD flag.)

But the XTHREAD flag means more than just sendfile - it means multiple threads
will need atomic seek-read/write operations on all platforms.  That wasn't the
right flag to play with across the board.

So this patch proposes an APR_OPEN_FOR_SENDFILE bit, where that bit
has little impact on any platform but Win32.

BUT - it does act as a sentinal that we can sendfile the flag.  This allows us, at
the apr_file_t granularity, to choose to sendfile or not.  It also can get rid of some
nasty side effects like Win9x which can't sendfile.  We decide this at runtime, so
we will simply unset that flag if requested.

Finally, we already have the semantics to recover the flags value from an apr_file_t,
giving the HTTP core output filter the final word to sendfile or not to sendfile.

Comments welcome, I'll commit midweek if nobody hollers.

Bill
Index: include/http_core.h
===================================================================
RCS file: /home/cvs/httpd-2.0/include/http_core.h,v
retrieving revision 1.68
diff -u -r1.68 http_core.h
--- include/http_core.h 3 Oct 2002 00:16:47 -0000       1.68
+++ include/http_core.h 13 Oct 2002 21:42:58 -0000
@@ -532,7 +532,12 @@
 #define ENABLE_MMAP_OFF    (0)
 #define ENABLE_MMAP_ON     (1)
 #define ENABLE_MMAP_UNSET  (2)
-    int enable_mmap;  /* whether files in this dir can be mmap'ed */
+    unsigned int enable_mmap : 2;  /* whether files in this dir can be mmap'ed */
+
+#define ENABLE_SENDFILE_OFF    (0)
+#define ENABLE_SENDFILE_ON     (1)
+#define ENABLE_SENDFILE_UNSET  (2)
+    unsigned int enable_sendfile : 2;  /* files in this dir can be mmap'ed */
 
 } core_dir_config;
 
Index: server/core.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/core.c,v
retrieving revision 1.211
diff -u -r1.211 core.c
--- server/core.c       13 Oct 2002 18:22:55 -0000      1.211
+++ server/core.c       13 Oct 2002 21:43:08 -0000
@@ -181,6 +181,7 @@
     conf->etag_remove = ETAG_UNSET;
 
     conf->enable_mmap = ENABLE_MMAP_UNSET;
+    conf->enable_sendfile = ENABLE_SENDFILE_UNSET;
 
     return (void *)conf;
 }
@@ -447,6 +448,10 @@
         conf->enable_mmap = new->enable_mmap;
     }
 
+    if (new->enable_sendfile != ENABLE_SENDFILE_UNSET) {
+        conf->enable_sendfile = new->enable_sendfile;
+    }
+
     return (void*)conf;
 }
 
@@ -1458,6 +1463,29 @@
     return NULL;
 }
 
+static const char *set_enable_sendfile(cmd_parms *cmd, void *d_,
+                                   const char *arg)
+{
+    core_dir_config *d = d_;
+    const char *err = ap_check_cmd_context(cmd, NOT_IN_LIMIT);
+
+    if (err != NULL) {
+        return err;
+    }
+
+    if (strcasecmp(arg, "on") == 0) {
+        d->enable_sendfile = ENABLE_SENDFILE_ON;
+    }
+    else if (strcasecmp(arg, "off") == 0) {
+        d->enable_sendfile = ENABLE_SENDFILE_OFF;
+    }
+    else {
+        return "parameter must be 'on' or 'off'";
+    }
+
+    return NULL;
+}
+
 static const char *satisfy(cmd_parms *cmd, void *c_, const char *arg)
 {
     core_dir_config *c = c_;
@@ -2936,6 +2964,8 @@
   "Specify components used to construct a file's ETag"),
 AP_INIT_TAKE1("EnableMMAP", set_enable_mmap, NULL, OR_FILEINFO,
   "Controls whether memory-mapping may be used to read files"),
+AP_INIT_TAKE1("EnableSendfile", set_enable_sendfile, NULL, OR_FILEINFO,
+  "Controls whether sendfile may be used to transmit files"),
 
 /* Old server config file commands */
 
@@ -3283,8 +3313,13 @@
             }
         }
 
-        if ((status = apr_file_open(&fd, r->filename, APR_READ | APR_BINARY, 0,
-                                    r->pool)) != APR_SUCCESS) {
+
+        if ((status = apr_file_open(&fd, r->filename, APR_READ | APR_BINARY
+#if APR_HAS_SENDFILE
+                             | ((d->enable_sendfile == ENABLE_SENDFILE_OFF) 
+                                                ? 0 : APR_OPEN_FOR_SENDFILE)
+#endif
+                                    , 0, r->pool)) != APR_SUCCESS) {
             ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r,
                           "file permissions deny server access: %s", r->filename);
             return HTTP_FORBIDDEN;
@@ -3306,8 +3341,9 @@
         }
 
         bb = apr_brigade_create(r->pool, c->bucket_alloc);
-#if APR_HAS_LARGE_FILES
-        if (r->finfo.size > AP_MAX_SENDFILE) {
+#if APR_HAS_SENDFILE && APR_HAS_LARGE_FILES
+        if ((d->enable_sendfile != ENABLE_SENDFILE_OFF) &&
+            (r->finfo.size > AP_MAX_SENDFILE)) {
             /* APR_HAS_LARGE_FILES issue; must split into mutiple buckets,
              * no greater than MAX(apr_size_t), and more granular than that
              * in case the brigade code/filters attempt to read it directly.
@@ -3888,27 +3924,24 @@
             }
 
 #if APR_HAS_SENDFILE
-            if (c->keepalive == AP_CONN_CLOSE && APR_BUCKET_IS_EOS(last_e)) {
-                /* Prepare the socket to be reused */
-                flags |= APR_SENDFILE_DISCONNECT_SOCKET;
-            }
-
-            rv = sendfile_it_all(net,      /* the network information   */
-                                 fd,       /* the file to send          */
-                                 &hdtr,    /* header and trailer iovecs */
-                                 foffset,  /* offset in the file to begin
-                                              sending from              */
-                                 flen,     /* length of file            */
-                                 nbytes + flen, /* total length including
-                                                   headers                */
-                                 flags);   /* apr_sendfile flags        */
-
-            /* If apr_sendfile() returns APR_ENOTIMPL, call emulate_sendfile().
-             * emulate_sendfile() is useful to enable the same Apache binary
-             * distribution to support Windows NT/2000 (supports TransmitFile)
-             * and Win95/98 (do not support TransmitFile)
-             */
-            if (rv == APR_ENOTIMPL)
+            if (apr_file_flags_get(fd) & APR_OPEN_FOR_SENDFILE) {
+
+                if (c->keepalive == AP_CONN_CLOSE && APR_BUCKET_IS_EOS(last_e)) {
+                    /* Prepare the socket to be reused */
+                    flags |= APR_SENDFILE_DISCONNECT_SOCKET;
+                }
+
+                rv = sendfile_it_all(net,      /* the network information   */
+                                     fd,       /* the file to send          */
+                                     &hdtr,    /* header and trailer iovecs */
+                                     foffset,  /* offset in the file to begin
+                                                  sending from              */
+                                     flen,     /* length of file            */
+                                     nbytes + flen, /* total length including
+                                                       headers                */
+                                     flags);   /* apr_sendfile flags        */
+            }
+            else
 #endif
             {
                 apr_size_t unused_bytes_sent;
Index: srclib/apr/file_io/win32/open.c
===================================================================
RCS file: /home/cvs/apr/file_io/win32/open.c,v
retrieving revision 1.108
diff -u -r1.108 open.c
--- srclib/apr/file_io/win32/open.c     17 Jul 2002 03:26:29 -0000      1.108
+++ srclib/apr/file_io/win32/open.c     13 Oct 2002 21:43:10 -0000
@@ -339,11 +339,19 @@
          */
         attributes |= FILE_FLAG_OVERLAPPED;
     }
-
 #if APR_HAS_UNICODE_FS
     IF_WIN_OS_IS_UNICODE
     {
         apr_wchar_t wfname[APR_PATH_MAX];
+
+        if (flag & APR_OPEN_FOR_SENDFILE) {    
+            /* This feature is required to enable sendfile operations
+             * against the file on Win32. Also implies APR_XTHREAD.
+             */
+            flag |= APR_XTHREAD;
+            attributes |= FILE_FLAG_SEQUENTIAL_SCAN | FILE_FLAG_OVERLAPPED;
+        }
+
         if (rv = utf8_to_unicode_path(wfname, sizeof(wfname) 
                                                / sizeof(apr_wchar_t), fname))
             return rv;
@@ -352,9 +360,16 @@
     }
 #endif
 #if APR_HAS_ANSI_FS
-    ELSE_WIN_OS_IS_ANSI
+    ELSE_WIN_OS_IS_ANSI {
         handle = CreateFileA(fname, oflags, sharemode,
                              NULL, createflags, attributes, 0);
+        if (flag & APR_OPEN_FOR_SENDFILE) {    
+            /* This feature is not supported on this platform.
+             */
+            flag &= ~APR_OPEN_FOR_SENDFILE;
+        }
+
+    }
 #endif
     if (handle == INVALID_HANDLE_VALUE) {
         return apr_get_os_error();
Index: srclib/apr/include/apr_file_io.h
===================================================================
RCS file: /home/cvs/apr/include/apr_file_io.h,v
retrieving revision 1.130
diff -u -r1.130 apr_file_io.h
--- srclib/apr/include/apr_file_io.h    5 Jul 2002 17:58:09 -0000       1.130
+++ srclib/apr/include/apr_file_io.h    13 Oct 2002 21:43:16 -0000
@@ -103,7 +103,8 @@
                                         writes across process/machines */
 #define APR_FILE_NOCLEANUP  2048   /**< Do not register a cleanup when the file
                                         is opened */
- 
+#define APR_OPEN_FOR_SENDFILE 4096 /**< Open with appropriate semantics for
+                                        apr_sendfile operation */ 
 /** @} */
 
 /**
@@ -184,7 +185,9 @@
  *           APR_FILE_NOCLEANUP   Do not register a cleanup with the pool 
  *                                passed in on the <EM>cont</EM> argument (see below).
  *                                The apr_os_file_t handle in apr_file_t will not
- &                                be closed when the pool is destroyed.
+ *                                be closed when the pool is destroyed.
+ *           APR_OPEN_FOR_SENDFILE  Open with appropriate platform semantics
+ *                                for sendfile operations.
  * </PRE>
  * @param perm Access permissions for file.
  * @param cont The pool to use.

Reply via email to