[PATCH 4 of 4] Support for sendfile(SF_NOCACHE)

Maxim Dounin mdounin at mdounin.ru
Tue Dec 14 14:16:54 UTC 2021


Hello!

On Tue, Nov 30, 2021 at 03:15:50PM +0300, Sergey Kandaurov wrote:

> On Thu, Nov 11, 2021 at 07:21:12AM +0300, Maxim Dounin wrote:
> > # HG changeset patch
> > # User Maxim Dounin <mdounin at mdounin.ru>
> > # Date 1636603897 -10800
> > #      Thu Nov 11 07:11:37 2021 +0300
> > # Node ID 10f96e74ae73e1c53a3fd08e7e1c26754c8969ed
> > # Parent  98d3beb63f32cbb68d1cdcec385614d32129cad0
> > Support for sendfile(SF_NOCACHE).
> > 
> > The SF_NOCACHE flag, introduced in FreeBSD 11 along with the new non-blocking
> > sendfile() implementation by glebius@, makes it possible to use sendfile()
> > along with the "directio" directive.
> > 
> > diff --git a/src/core/ngx_output_chain.c b/src/core/ngx_output_chain.c
> > --- a/src/core/ngx_output_chain.c
> > +++ b/src/core/ngx_output_chain.c
> > @@ -256,9 +256,11 @@ ngx_output_chain_as_is(ngx_output_chain_
> >      }
> >  #endif
> >  
> > +#if !(NGX_HAVE_SENDFILE_NODISKIO)
> >      if (buf->in_file && buf->file->directio) {
> >          return 0;
> >      }
> > +#endif
> 
> This probably deserves a comment, why it depends on such a macro test.
> Though, it should be pretty clear from the commit log.

I also tend to think that the original code is wrong (or, rather, 
too aggressive), and it should only disable sendfile(), much like 
the ctx->sendfile and NGX_SENDFILE_LIMIT checks below.

Changed to the following:

diff --git a/src/core/ngx_output_chain.c b/src/core/ngx_output_chain.c
--- a/src/core/ngx_output_chain.c
+++ b/src/core/ngx_output_chain.c
@@ -256,12 +256,6 @@ ngx_output_chain_as_is(ngx_output_chain_
     }
 #endif
 
-#if !(NGX_HAVE_SENDFILE_NODISKIO)
-    if (buf->in_file && buf->file->directio) {
-        return 0;
-    }
-#endif
-
     sendfile = ctx->sendfile;
 
 #if (NGX_SENDFILE_LIMIT)
@@ -272,6 +266,19 @@ ngx_output_chain_as_is(ngx_output_chain_
 
 #endif
 
+#if !(NGX_HAVE_SENDFILE_NODISKIO)
+
+    /*
+     * With DIRECTIO, disable sendfile() unless sendfile(SF_NOCACHE)
+     * is available.
+     */
+
+    if (buf->in_file && buf->file->directio) {
+        sendfile = 0;
+    }
+
+#endif
+
     if (!sendfile) {
 
         if (!ngx_buf_in_memory(buf)) {


[...]

> Otherwise, looks good.

Full series, updated:

# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1639425539 -10800
#      Mon Dec 13 22:58:59 2021 +0300
# Node ID 42f1674706c9922fa910a7601af83307b3633b95
# Parent  a7a77549265ef46f1f0fdb3897f4beabf9e09c40
Removed "aio sendfile", deprecated since 1.7.11.

diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c
--- a/src/http/ngx_http_core_module.c
+++ b/src/http/ngx_http_core_module.c
@@ -4568,19 +4568,6 @@ ngx_http_core_set_aio(ngx_conf_t *cf, ng
 #endif
     }
 
-#if (NGX_HAVE_AIO_SENDFILE)
-
-    if (ngx_strcmp(value[1].data, "sendfile") == 0) {
-        clcf->aio = NGX_HTTP_AIO_ON;
-
-        ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
-                           "the \"sendfile\" parameter of "
-                           "the \"aio\" directive is deprecated");
-        return NGX_CONF_OK;
-    }
-
-#endif
-
     if (ngx_strncmp(value[1].data, "threads", 7) == 0
         && (value[1].len == 7 || value[1].data[7] == '='))
     {
# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1639447743 -10800
#      Tue Dec 14 05:09:03 2021 +0300
# Node ID a149cd880ddd1473d21b66b85b9307bb17966df3
# Parent  42f1674706c9922fa910a7601af83307b3633b95
Simplified sendfile(SF_NODISKIO) usage.

Starting with FreeBSD 11, there is no need to use AIO operations to preload
data into cache for sendfile(SF_NODISKIO) to work.  Instead, sendfile()
handles non-blocking loading data from disk by itself.  It still can, however,
return EBUSY if a page is already being loaded (for example, by a different
process).  If this happens, we now post an event for the next event loop
iteration, so sendfile() is retried "after a short period", as manpage
recommends.

The limit of the number of EBUSY tolerated without any progress is preserved,
but now it does not result in an alert, since on an idle system event loop
iteration might be very short and EBUSY can happen many times in a row.
Instead, SF_NODISKIO is simply disabled for one call once the limit is
reached.

With this change, sendfile(SF_NODISKIO) is now used automatically as long as
sendfile() is enabled, and no longer requires "aio on;".

diff --git a/auto/os/freebsd b/auto/os/freebsd
--- a/auto/os/freebsd
+++ b/auto/os/freebsd
@@ -44,12 +44,10 @@ if [ $osreldate -gt 300007 ]; then
     CORE_SRCS="$CORE_SRCS $FREEBSD_SENDFILE_SRCS"
 fi
 
-if [ $NGX_FILE_AIO = YES ]; then
-    if [ $osreldate -gt 502103 ]; then
-        echo " + sendfile()'s SF_NODISKIO found"
+if [ $osreldate -gt 1100093 ]; then
+    echo " + sendfile()'s SF_NODISKIO found"
 
-        have=NGX_HAVE_AIO_SENDFILE . auto/have
-    fi
+    have=NGX_HAVE_SENDFILE_NODISKIO . auto/have
 fi
 
 # POSIX semaphores
diff --git a/src/core/ngx_buf.h b/src/core/ngx_buf.h
--- a/src/core/ngx_buf.h
+++ b/src/core/ngx_buf.h
@@ -90,9 +90,6 @@ struct ngx_output_chain_ctx_s {
 
 #if (NGX_HAVE_FILE_AIO || NGX_COMPAT)
     ngx_output_chain_aio_pt      aio_handler;
-#if (NGX_HAVE_AIO_SENDFILE || NGX_COMPAT)
-    ssize_t                    (*aio_preload)(ngx_buf_t *file);
-#endif
 #endif
 
 #if (NGX_THREADS || NGX_COMPAT)
diff --git a/src/core/ngx_connection.h b/src/core/ngx_connection.h
--- a/src/core/ngx_connection.h
+++ b/src/core/ngx_connection.h
@@ -185,7 +185,7 @@ struct ngx_connection_s {
 
     unsigned            need_last_buf:1;
 
-#if (NGX_HAVE_AIO_SENDFILE || NGX_COMPAT)
+#if (NGX_HAVE_SENDFILE_NODISKIO || NGX_COMPAT)
     unsigned            busy_count:2;
 #endif
 
diff --git a/src/core/ngx_module.h b/src/core/ngx_module.h
--- a/src/core/ngx_module.h
+++ b/src/core/ngx_module.h
@@ -41,7 +41,7 @@
 #define NGX_MODULE_SIGNATURE_3   "0"
 #endif
 
-#if (NGX_HAVE_AIO_SENDFILE || NGX_COMPAT)
+#if (NGX_HAVE_SENDFILE_NODISKIO || NGX_COMPAT)
 #define NGX_MODULE_SIGNATURE_4   "1"
 #else
 #define NGX_MODULE_SIGNATURE_4   "0"
diff --git a/src/core/ngx_output_chain.c b/src/core/ngx_output_chain.c
--- a/src/core/ngx_output_chain.c
+++ b/src/core/ngx_output_chain.c
@@ -29,10 +29,6 @@
 
 static ngx_inline ngx_int_t
     ngx_output_chain_as_is(ngx_output_chain_ctx_t *ctx, ngx_buf_t *buf);
-#if (NGX_HAVE_AIO_SENDFILE)
-static ngx_int_t ngx_output_chain_aio_setup(ngx_output_chain_ctx_t *ctx,
-    ngx_file_t *file);
-#endif
 static ngx_int_t ngx_output_chain_add_copy(ngx_pool_t *pool,
     ngx_chain_t **chain, ngx_chain_t *in);
 static ngx_int_t ngx_output_chain_align_file_buf(ngx_output_chain_ctx_t *ctx,
@@ -283,12 +279,6 @@ ngx_output_chain_as_is(ngx_output_chain_
         buf->in_file = 0;
     }
 
-#if (NGX_HAVE_AIO_SENDFILE)
-    if (ctx->aio_preload && buf->in_file) {
-        (void) ngx_output_chain_aio_setup(ctx, buf->file);
-    }
-#endif
-
     if (ctx->need_in_memory && !ngx_buf_in_memory(buf)) {
         return 0;
     }
@@ -301,28 +291,6 @@ ngx_output_chain_as_is(ngx_output_chain_
 }
 
 
-#if (NGX_HAVE_AIO_SENDFILE)
-
-static ngx_int_t
-ngx_output_chain_aio_setup(ngx_output_chain_ctx_t *ctx, ngx_file_t *file)
-{
-    ngx_event_aio_t  *aio;
-
-    if (file->aio == NULL && ngx_file_aio_init(file, ctx->pool) != NGX_OK) {
-        return NGX_ERROR;
-    }
-
-    aio = file->aio;
-
-    aio->data = ctx->filter_ctx;
-    aio->preload_handler = ctx->aio_preload;
-
-    return NGX_OK;
-}
-
-#endif
-
-
 static ngx_int_t
 ngx_output_chain_add_copy(ngx_pool_t *pool, ngx_chain_t **chain,
     ngx_chain_t *in)
diff --git a/src/event/ngx_event.h b/src/event/ngx_event.h
--- a/src/event/ngx_event.h
+++ b/src/event/ngx_event.h
@@ -147,10 +147,6 @@ struct ngx_event_aio_s {
 
     ngx_fd_t                   fd;
 
-#if (NGX_HAVE_AIO_SENDFILE || NGX_COMPAT)
-    ssize_t                  (*preload_handler)(ngx_buf_t *file);
-#endif
-
 #if (NGX_HAVE_EVENTFD)
     int64_t                    res;
 #endif
diff --git a/src/http/ngx_http_copy_filter_module.c b/src/http/ngx_http_copy_filter_module.c
--- a/src/http/ngx_http_copy_filter_module.c
+++ b/src/http/ngx_http_copy_filter_module.c
@@ -19,10 +19,6 @@ typedef struct {
 static void ngx_http_copy_aio_handler(ngx_output_chain_ctx_t *ctx,
     ngx_file_t *file);
 static void ngx_http_copy_aio_event_handler(ngx_event_t *ev);
-#if (NGX_HAVE_AIO_SENDFILE)
-static ssize_t ngx_http_copy_aio_sendfile_preload(ngx_buf_t *file);
-static void ngx_http_copy_aio_sendfile_event_handler(ngx_event_t *ev);
-#endif
 #endif
 #if (NGX_THREADS)
 static ngx_int_t ngx_http_copy_thread_handler(ngx_thread_task_t *task,
@@ -128,9 +124,6 @@ ngx_http_copy_filter(ngx_http_request_t 
 #if (NGX_HAVE_FILE_AIO)
         if (ngx_file_aio && clcf->aio == NGX_HTTP_AIO_ON) {
             ctx->aio_handler = ngx_http_copy_aio_handler;
-#if (NGX_HAVE_AIO_SENDFILE)
-            ctx->aio_preload = ngx_http_copy_aio_sendfile_preload;
-#endif
         }
 #endif
 
@@ -207,81 +200,6 @@ ngx_http_copy_aio_event_handler(ngx_even
     ngx_http_run_posted_requests(c);
 }
 
-
-#if (NGX_HAVE_AIO_SENDFILE)
-
-static ssize_t
-ngx_http_copy_aio_sendfile_preload(ngx_buf_t *file)
-{
-    ssize_t                  n;
-    static u_char            buf[1];
-    ngx_event_aio_t         *aio;
-    ngx_http_request_t      *r;
-    ngx_output_chain_ctx_t  *ctx;
-
-    aio = file->file->aio;
-    r = aio->data;
-
-    if (r->aio) {
-        /*
-         * tolerate sendfile() calls if another operation is already
-         * running; this can happen due to subrequests, multiple calls
-         * of the next body filter from a filter, or in HTTP/2 due to
-         * a write event on the main connection
-         */
-
-        return NGX_AGAIN;
-    }
-
-    n = ngx_file_aio_read(file->file, buf, 1, file->file_pos, NULL);
-
-    if (n == NGX_AGAIN) {
-        aio->handler = ngx_http_copy_aio_sendfile_event_handler;
-
-        r->main->blocked++;
-        r->aio = 1;
-
-        ctx = ngx_http_get_module_ctx(r, ngx_http_copy_filter_module);
-        ctx->aio = 1;
-    }
-
-    return n;
-}
-
-
-static void
-ngx_http_copy_aio_sendfile_event_handler(ngx_event_t *ev)
-{
-    ngx_event_aio_t     *aio;
-    ngx_connection_t    *c;
-    ngx_http_request_t  *r;
-
-    aio = ev->data;
-    r = aio->data;
-    c = r->connection;
-
-    r->main->blocked--;
-    r->aio = 0;
-    ev->complete = 0;
-
-#if (NGX_HTTP_V2)
-
-    if (r->stream) {
-        /*
-         * for HTTP/2, update write event to make sure processing will
-         * reach the main connection to handle sendfile() preload
-         */
-
-        c->write->ready = 1;
-        c->write->active = 0;
-    }
-
-#endif
-
-    c->write->handler(c->write);
-}
-
-#endif
 #endif
 
 
diff --git a/src/os/unix/ngx_freebsd_sendfile_chain.c b/src/os/unix/ngx_freebsd_sendfile_chain.c
--- a/src/os/unix/ngx_freebsd_sendfile_chain.c
+++ b/src/os/unix/ngx_freebsd_sendfile_chain.c
@@ -32,23 +32,22 @@
 ngx_chain_t *
 ngx_freebsd_sendfile_chain(ngx_connection_t *c, ngx_chain_t *in, off_t limit)
 {
-    int               rc, flags;
-    off_t             send, prev_send, sent;
-    size_t            file_size;
-    ssize_t           n;
-    ngx_uint_t        eintr, eagain;
-    ngx_err_t         err;
-    ngx_buf_t        *file;
-    ngx_event_t      *wev;
-    ngx_chain_t      *cl;
-    ngx_iovec_t       header, trailer;
-    struct sf_hdtr    hdtr;
-    struct iovec      headers[NGX_IOVS_PREALLOCATE];
-    struct iovec      trailers[NGX_IOVS_PREALLOCATE];
-#if (NGX_HAVE_AIO_SENDFILE)
-    ngx_uint_t        ebusy;
-    ngx_event_aio_t  *aio;
+    int              rc, flags;
+    off_t            send, prev_send, sent;
+    size_t           file_size;
+    ssize_t          n;
+    ngx_err_t        err;
+    ngx_buf_t       *file;
+    ngx_uint_t       eintr, eagain;
+#if (NGX_HAVE_SENDFILE_NODISKIO)
+    ngx_uint_t       ebusy;
 #endif
+    ngx_event_t     *wev;
+    ngx_chain_t     *cl;
+    ngx_iovec_t      header, trailer;
+    struct sf_hdtr   hdtr;
+    struct iovec     headers[NGX_IOVS_PREALLOCATE];
+    struct iovec     trailers[NGX_IOVS_PREALLOCATE];
 
     wev = c->write;
 
@@ -77,11 +76,6 @@ ngx_freebsd_sendfile_chain(ngx_connectio
     eagain = 0;
     flags = 0;
 
-#if (NGX_HAVE_AIO_SENDFILE && NGX_SUPPRESS_WARN)
-    aio = NULL;
-    file = NULL;
-#endif
-
     header.iovs = headers;
     header.nalloc = NGX_IOVS_PREALLOCATE;
 
@@ -90,7 +84,7 @@ ngx_freebsd_sendfile_chain(ngx_connectio
 
     for ( ;; ) {
         eintr = 0;
-#if (NGX_HAVE_AIO_SENDFILE)
+#if (NGX_HAVE_SENDFILE_NODISKIO)
         ebusy = 0;
 #endif
         prev_send = send;
@@ -179,9 +173,8 @@ ngx_freebsd_sendfile_chain(ngx_connectio
 
             sent = 0;
 
-#if (NGX_HAVE_AIO_SENDFILE)
-            aio = file->file->aio;
-            flags = (aio && aio->preload_handler) ? SF_NODISKIO : 0;
+#if (NGX_HAVE_SENDFILE_NODISKIO)
+            flags = (c->busy_count <= 2) ? SF_NODISKIO : 0;
 #endif
 
             rc = sendfile(file->file->fd, c->fd, file->file_pos,
@@ -199,7 +192,7 @@ ngx_freebsd_sendfile_chain(ngx_connectio
                     eintr = 1;
                     break;
 
-#if (NGX_HAVE_AIO_SENDFILE)
+#if (NGX_HAVE_SENDFILE_NODISKIO)
                 case NGX_EBUSY:
                     ebusy = 1;
                     break;
@@ -258,35 +251,24 @@ ngx_freebsd_sendfile_chain(ngx_connectio
             if (sent == 0) {
                 c->busy_count++;
 
-                if (c->busy_count > 2) {
-                    ngx_log_error(NGX_LOG_ALERT, c->log, 0,
-                                  "sendfile(%V) returned busy again",
-                                  &file->file->name);
-
-                    c->busy_count = 0;
-                    aio->preload_handler = NULL;
-
-                    send = prev_send;
-                    continue;
-                }
+                ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0,
+                               "sendfile() busy, count:%d", c->busy_count);
 
             } else {
                 c->busy_count = 0;
             }
 
-            n = aio->preload_handler(file);
-
-            if (n > 0) {
-                send = prev_send + sent;
-                continue;
+            if (wev->posted) {
+                ngx_delete_posted_event(wev);
             }
 
+            ngx_post_event(wev, &ngx_posted_next_events);
+
+            wev->ready = 0;
             return in;
         }
 
-        if (flags == SF_NODISKIO) {
-            c->busy_count = 0;
-        }
+        c->busy_count = 0;
 
 #endif
 
# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1639450352 -10800
#      Tue Dec 14 05:52:32 2021 +0300
# Node ID e7605da2d39b6df7c31d41afb5699277ae872490
# Parent  a149cd880ddd1473d21b66b85b9307bb17966df3
SSL: SSL_sendfile(SF_NODISKIO) support.

diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -2942,7 +2942,7 @@ ngx_ssl_sendfile(ngx_connection_t *c, ng
 {
 #ifdef BIO_get_ktls_send
 
-    int        sslerr;
+    int        sslerr, flags;
     ssize_t    n;
     ngx_err_t  err;
 
@@ -2954,8 +2954,14 @@ ngx_ssl_sendfile(ngx_connection_t *c, ng
 
     ngx_set_errno(0);
 
+#if (NGX_HAVE_SENDFILE_NODISKIO)
+    flags = (c->busy_count <= 2) ? SF_NODISKIO : 0;
+#else
+    flags = 0;
+#endif
+
     n = SSL_sendfile(c->ssl->connection, file->file->fd, file->file_pos,
-                     size, 0);
+                     size, flags);
 
     ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0, "SSL_sendfile: %d", n);
 
@@ -2974,6 +2980,10 @@ ngx_ssl_sendfile(ngx_connection_t *c, ng
             ngx_post_event(c->read, &ngx_posted_events);
         }
 
+#if (NGX_HAVE_SENDFILE_NODISKIO)
+        c->busy_count = 0;
+#endif
+
         c->sent += n;
 
         return n;
@@ -3038,6 +3048,23 @@ ngx_ssl_sendfile(ngx_connection_t *c, ng
             ngx_post_event(c->read, &ngx_posted_events);
         }
 
+#if (NGX_HAVE_SENDFILE_NODISKIO)
+
+        if (ngx_errno == EBUSY) {
+            c->busy_count++;
+
+            ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0,
+                           "SSL_sendfile() busy, count:%d", c->busy_count);
+
+            if (c->write->posted) {
+                ngx_delete_posted_event(c->write);
+            }
+
+            ngx_post_event(c->write, &ngx_posted_next_events);
+        }
+
+#endif
+
         c->write->ready = 0;
         return NGX_AGAIN;
     }
# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1639491120 -10800
#      Tue Dec 14 17:12:00 2021 +0300
# Node ID 716e3f4765d065d5f59c3740f1e69bc7ea9cd368
# Parent  e7605da2d39b6df7c31d41afb5699277ae872490
Support for sendfile(SF_NOCACHE).

The SF_NOCACHE flag, introduced in FreeBSD 11 along with the new non-blocking
sendfile() implementation by glebius@, makes it possible to use sendfile()
along with the "directio" directive.

diff --git a/src/core/ngx_output_chain.c b/src/core/ngx_output_chain.c
--- a/src/core/ngx_output_chain.c
+++ b/src/core/ngx_output_chain.c
@@ -256,10 +256,6 @@ ngx_output_chain_as_is(ngx_output_chain_
     }
 #endif
 
-    if (buf->in_file && buf->file->directio) {
-        return 0;
-    }
-
     sendfile = ctx->sendfile;
 
 #if (NGX_SENDFILE_LIMIT)
@@ -270,6 +266,19 @@ ngx_output_chain_as_is(ngx_output_chain_
 
 #endif
 
+#if !(NGX_HAVE_SENDFILE_NODISKIO)
+
+    /*
+     * With DIRECTIO, disable sendfile() unless sendfile(SF_NOCACHE)
+     * is available.
+     */
+
+    if (buf->in_file && buf->file->directio) {
+        sendfile = 0;
+    }
+
+#endif
+
     if (!sendfile) {
 
         if (!ngx_buf_in_memory(buf)) {
diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -2955,7 +2955,13 @@ ngx_ssl_sendfile(ngx_connection_t *c, ng
     ngx_set_errno(0);
 
 #if (NGX_HAVE_SENDFILE_NODISKIO)
+
     flags = (c->busy_count <= 2) ? SF_NODISKIO : 0;
+
+    if (file->file->directio) {
+        flags |= SF_NOCACHE;
+    }
+
 #else
     flags = 0;
 #endif
diff --git a/src/os/unix/ngx_freebsd_sendfile_chain.c b/src/os/unix/ngx_freebsd_sendfile_chain.c
--- a/src/os/unix/ngx_freebsd_sendfile_chain.c
+++ b/src/os/unix/ngx_freebsd_sendfile_chain.c
@@ -174,7 +174,13 @@ ngx_freebsd_sendfile_chain(ngx_connectio
             sent = 0;
 
 #if (NGX_HAVE_SENDFILE_NODISKIO)
+
             flags = (c->busy_count <= 2) ? SF_NODISKIO : 0;
+
+            if (file->file->directio) {
+                flags |= SF_NOCACHE;
+            }
+
 #endif
 
             rc = sendfile(file->file->fd, c->fd, file->file_pos,

-- 
Maxim Dounin
http://mdounin.ru/


More information about the nginx-devel mailing list