From 2b8d6ad805a0132844cfbc1cf1c6988dfe8c9973 Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Wed, 11 Feb 2015 17:52:15 +0300 Subject: [PATCH] Refactored sendfile() AIO preload. This reduces layering violation and simplifies the logic of AIO preread, since it's now triggered by the send chain function itself without falling back to the copy filter. The context of AIO operation is now stored per file buffer, which makes it possible to properly handle cases when multiple buffers come from different locations, each with its own configuration. --- src/core/ngx_buf.h | 3 + src/core/ngx_connection.h | 2 - src/core/ngx_output_chain.c | 32 +++++++ src/event/ngx_event.h | 8 +- src/http/ngx_http_copy_filter_module.c | 101 +++++++++-------------- src/http/ngx_http_request.h | 3 - src/os/unix/ngx_file_aio_read.c | 42 ++++++---- src/os/unix/ngx_files.h | 1 + src/os/unix/ngx_freebsd_sendfile_chain.c | 77 +++++++++++++---- src/os/unix/ngx_linux_aio_read.c | 39 +++++---- 10 files changed, 188 insertions(+), 120 deletions(-) diff --git a/src/core/ngx_buf.h b/src/core/ngx_buf.h index 13536a69a..219894ffc 100644 --- a/src/core/ngx_buf.h +++ b/src/core/ngx_buf.h @@ -94,6 +94,9 @@ struct ngx_output_chain_ctx_s { unsigned aio:1; ngx_output_chain_aio_pt aio_handler; +#if (NGX_HAVE_FILE_AIO) + ssize_t (*aio_preload)(ngx_buf_t *file); +#endif #endif off_t alignment; diff --git a/src/core/ngx_connection.h b/src/core/ngx_connection.h index ed14e6023..143cab7b3 100644 --- a/src/core/ngx_connection.h +++ b/src/core/ngx_connection.h @@ -181,9 +181,7 @@ struct ngx_connection_s { #endif #if (NGX_HAVE_AIO_SENDFILE) - unsigned aio_sendfile:1; unsigned busy_count:2; - ngx_buf_t *busy_sendfile; #endif #if (NGX_THREADS) diff --git a/src/core/ngx_output_chain.c b/src/core/ngx_output_chain.c index 9d7a8460f..ca390e254 100644 --- a/src/core/ngx_output_chain.c +++ b/src/core/ngx_output_chain.c @@ -29,6 +29,10 @@ 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, @@ -252,6 +256,12 @@ ngx_output_chain_as_is(ngx_output_chain_ctx_t *ctx, ngx_buf_t *buf) 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; } @@ -264,6 +274,28 @@ 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) +{ + 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 index a1643a134..555cda0f4 100644 --- a/src/event/ngx_event.h +++ b/src/event/ngx_event.h @@ -168,6 +168,10 @@ struct ngx_event_aio_s { ngx_event_handler_pt handler; ngx_file_t *file; +#if (NGX_HAVE_AIO_SENDFILE) + ssize_t (*preload_handler)(ngx_buf_t *file); +#endif + ngx_fd_t fd; #if (NGX_HAVE_EVENTFD) @@ -181,10 +185,6 @@ struct ngx_event_aio_s { size_t nbytes; #endif -#if (NGX_HAVE_AIO_SENDFILE) - off_t last_offset; -#endif - ngx_aiocb_t aiocb; ngx_event_t event; }; diff --git a/src/http/ngx_http_copy_filter_module.c b/src/http/ngx_http_copy_filter_module.c index 3ad27b042..cdd7fceb6 100644 --- a/src/http/ngx_http_copy_filter_module.c +++ b/src/http/ngx_http_copy_filter_module.c @@ -20,6 +20,7 @@ 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 @@ -125,7 +126,9 @@ ngx_http_copy_filter(ngx_http_request_t *r, ngx_chain_t *in) ctx->aio_handler = ngx_http_copy_aio_handler; } #if (NGX_HAVE_AIO_SENDFILE) - c->aio_sendfile = (clcf->aio == NGX_HTTP_AIO_SENDFILE); + if (clcf->aio == NGX_HTTP_AIO_SENDFILE) { + ctx->aio_preload = ngx_http_copy_aio_sendfile_preload; + } #endif } #endif @@ -139,72 +142,19 @@ ngx_http_copy_filter(ngx_http_request_t *r, ngx_chain_t *in) ctx->aio = r->aio; #endif - for ( ;; ) { - rc = ngx_output_chain(ctx, in); + rc = ngx_output_chain(ctx, in); - if (ctx->in == NULL) { - r->buffered &= ~NGX_HTTP_COPY_BUFFERED; + if (ctx->in == NULL) { + r->buffered &= ~NGX_HTTP_COPY_BUFFERED; - } else { - r->buffered |= NGX_HTTP_COPY_BUFFERED; - } - - ngx_log_debug3(NGX_LOG_DEBUG_HTTP, c->log, 0, - "http copy filter: %i \"%V?%V\"", rc, &r->uri, &r->args); - -#if (NGX_HAVE_FILE_AIO && NGX_HAVE_AIO_SENDFILE) - - if (c->busy_sendfile) { - ssize_t n; - off_t offset; - ngx_file_t *file; - ngx_http_ephemeral_t *e; - - if (r->aio) { - c->busy_sendfile = NULL; - return rc; - } - - file = c->busy_sendfile->file; - offset = c->busy_sendfile->file_pos; - - if (file->aio) { - c->busy_count = (offset == file->aio->last_offset) ? - c->busy_count + 1 : 0; - file->aio->last_offset = offset; - - if (c->busy_count > 2) { - ngx_log_error(NGX_LOG_ALERT, c->log, 0, - "sendfile(%V) returned busy again", - &file->name); - c->aio_sendfile = 0; - } - } - - c->busy_sendfile = NULL; - e = (ngx_http_ephemeral_t *) &r->uri_start; - - n = ngx_file_aio_read(file, &e->aio_preload, 1, offset, r->pool); - - if (n > 0) { - in = NULL; - continue; - } - - rc = n; - - if (rc == NGX_AGAIN) { - file->aio->data = r; - file->aio->handler = ngx_http_copy_aio_sendfile_event_handler; - - r->main->blocked++; - r->aio = 1; - } - } -#endif - - return rc; + } else { + r->buffered |= NGX_HTTP_COPY_BUFFERED; } + + ngx_log_debug3(NGX_LOG_DEBUG_HTTP, c->log, 0, + "http copy filter: %i \"%V?%V\"", rc, &r->uri, &r->args); + + return rc; } @@ -244,6 +194,29 @@ 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) +{ + ssize_t n; + static u_char buf[1]; + ngx_event_aio_t *aio; + ngx_http_request_t *r; + + n = ngx_file_aio_read(file->file, buf, 1, file->file_pos, NULL); + + if (n == NGX_AGAIN) { + aio = file->file->aio; + aio->handler = ngx_http_copy_aio_sendfile_event_handler; + + r = aio->data; + r->main->blocked++; + r->aio = 1; + } + + return n; +} + + static void ngx_http_copy_aio_sendfile_event_handler(ngx_event_t *ev) { diff --git a/src/http/ngx_http_request.h b/src/http/ngx_http_request.h index cffab9a69..9be0c6e71 100644 --- a/src/http/ngx_http_request.h +++ b/src/http/ngx_http_request.h @@ -574,9 +574,6 @@ struct ngx_http_request_s { typedef struct { ngx_http_posted_request_t terminal_posted_request; -#if (NGX_HAVE_AIO_SENDFILE) - u_char aio_preload; -#endif } ngx_http_ephemeral_t; diff --git a/src/os/unix/ngx_file_aio_read.c b/src/os/unix/ngx_file_aio_read.c index 0bb383de5..b11cf8a3d 100644 --- a/src/os/unix/ngx_file_aio_read.c +++ b/src/os/unix/ngx_file_aio_read.c @@ -36,6 +36,28 @@ static ssize_t ngx_file_aio_result(ngx_file_t *file, ngx_event_aio_t *aio, static void ngx_file_aio_event_handler(ngx_event_t *ev); +ngx_int_t +ngx_file_aio_init(ngx_file_t *file, ngx_pool_t *pool) +{ + ngx_event_aio_t *aio; + + aio = ngx_pcalloc(pool, sizeof(ngx_event_aio_t)); + if (aio == NULL) { + return NGX_ERROR; + } + + aio->file = file; + aio->fd = file->fd; + aio->event.data = aio; + aio->event.ready = 1; + aio->event.log = file->log; + + file->aio = aio; + + return NGX_OK; +} + + ssize_t ngx_file_aio_read(ngx_file_t *file, u_char *buf, size_t size, off_t offset, ngx_pool_t *pool) @@ -48,25 +70,11 @@ ngx_file_aio_read(ngx_file_t *file, u_char *buf, size_t size, off_t offset, return ngx_read_file(file, buf, size, offset); } - aio = file->aio; - - if (aio == NULL) { - aio = ngx_pcalloc(pool, sizeof(ngx_event_aio_t)); - if (aio == NULL) { - return NGX_ERROR; - } - - aio->file = file; - aio->fd = file->fd; - aio->event.data = aio; - aio->event.ready = 1; - aio->event.log = file->log; -#if (NGX_HAVE_AIO_SENDFILE) - aio->last_offset = -1; -#endif - file->aio = aio; + if (file->aio == NULL && ngx_file_aio_init(file, pool) != NGX_OK) { + return NGX_ERROR; } + aio = file->aio; ev = &aio->event; if (!ev->ready) { diff --git a/src/os/unix/ngx_files.h b/src/os/unix/ngx_files.h index a78ec9613..a046ee756 100644 --- a/src/os/unix/ngx_files.h +++ b/src/os/unix/ngx_files.h @@ -375,6 +375,7 @@ size_t ngx_fs_bsize(u_char *name); #if (NGX_HAVE_FILE_AIO) +ngx_int_t ngx_file_aio_init(ngx_file_t *file, ngx_pool_t *pool); ssize_t ngx_file_aio_read(ngx_file_t *file, u_char *buf, size_t size, off_t offset, ngx_pool_t *pool); diff --git a/src/os/unix/ngx_freebsd_sendfile_chain.c b/src/os/unix/ngx_freebsd_sendfile_chain.c index 7199c8654..25790b6b6 100644 --- a/src/os/unix/ngx_freebsd_sendfile_chain.c +++ b/src/os/unix/ngx_freebsd_sendfile_chain.c @@ -32,19 +32,23 @@ 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]; + 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; +#endif wev = c->write; @@ -73,6 +77,11 @@ ngx_freebsd_sendfile_chain(ngx_connection_t *c, ngx_chain_t *in, off_t limit) 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; @@ -81,6 +90,9 @@ ngx_freebsd_sendfile_chain(ngx_connection_t *c, ngx_chain_t *in, off_t limit) for ( ;; ) { eintr = 0; +#if (NGX_HAVE_AIO_SENDFILE) + ebusy = 0; +#endif prev_send = send; /* create the header iovec and coalesce the neighbouring bufs */ @@ -160,7 +172,8 @@ ngx_freebsd_sendfile_chain(ngx_connection_t *c, ngx_chain_t *in, off_t limit) sent = 0; #if (NGX_HAVE_AIO_SENDFILE) - flags = c->aio_sendfile ? SF_NODISKIO : 0; + aio = file->file->aio; + flags = (aio && aio->preload_handler) ? SF_NODISKIO : 0; #endif rc = sendfile(file->file->fd, c->fd, file->file_pos, @@ -180,7 +193,7 @@ ngx_freebsd_sendfile_chain(ngx_connection_t *c, ngx_chain_t *in, off_t limit) #if (NGX_HAVE_AIO_SENDFILE) case NGX_EBUSY: - c->busy_sendfile = file; + ebusy = 1; break; #endif @@ -232,9 +245,41 @@ ngx_freebsd_sendfile_chain(ngx_connection_t *c, ngx_chain_t *in, off_t limit) in = ngx_chain_update_sent(in, sent); #if (NGX_HAVE_AIO_SENDFILE) - if (c->busy_sendfile) { + + if (ebusy) { + 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; + } + + } else { + c->busy_count = 0; + } + + rc = aio->preload_handler(file); + + if (rc > 0) { + send = prev_send + sent; + continue; + } + return in; } + + if (flags == SF_NODISKIO) { + c->busy_count = 0; + } + #endif if (eagain) { diff --git a/src/os/unix/ngx_linux_aio_read.c b/src/os/unix/ngx_linux_aio_read.c index 8273c13f9..b0a923604 100644 --- a/src/os/unix/ngx_linux_aio_read.c +++ b/src/os/unix/ngx_linux_aio_read.c @@ -24,6 +24,28 @@ io_submit(aio_context_t ctx, long n, struct iocb **paiocb) } +ngx_int_t +ngx_file_aio_init(ngx_file_t *file, ngx_pool_t *pool) +{ + ngx_event_aio_t *aio; + + aio = ngx_pcalloc(pool, sizeof(ngx_event_aio_t)); + if (aio == NULL) { + return NGX_ERROR; + } + + aio->file = file; + aio->fd = file->fd; + aio->event.data = aio; + aio->event.ready = 1; + aio->event.log = file->log; + + file->aio = aio; + + return NGX_OK; +} + + ssize_t ngx_file_aio_read(ngx_file_t *file, u_char *buf, size_t size, off_t offset, ngx_pool_t *pool) @@ -37,22 +59,11 @@ ngx_file_aio_read(ngx_file_t *file, u_char *buf, size_t size, off_t offset, return ngx_read_file(file, buf, size, offset); } - aio = file->aio; - - if (aio == NULL) { - aio = ngx_pcalloc(pool, sizeof(ngx_event_aio_t)); - if (aio == NULL) { - return NGX_ERROR; - } - - aio->file = file; - aio->fd = file->fd; - aio->event.data = aio; - aio->event.ready = 1; - aio->event.log = file->log; - file->aio = aio; + if (file->aio == NULL && ngx_file_aio_init(file, pool) != NGX_OK) { + return NGX_ERROR; } + aio = file->aio; ev = &aio->event; if (!ev->ready) {