On Thu, Nov 11, 2021 at 07:21:10AM +0300, Maxim Dounin wrote: > # HG changeset patch > # User Maxim Dounin <mdou...@mdounin.ru> > # Date 1636603886 -10800 > # Thu Nov 11 07:11:26 2021 +0300 > # Node ID 4a954e89b1ae8539bbe08c5afc1d5c9828d82d6f > # Parent 0fb75ef9dbca698e5e855145cf6a12180a36d400 > 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 1100000 ]; 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
We could check the exact __FreeBSD_version number 1100093 that was at the time the new sendfile() appeared, which is more accurate. https://cgit.freebsd.org/src/commit/?id=2bab0c553588 https://cgit.freebsd.org/src/tree/sys/sys/param.h?id=2bab0c553588#n48 Unfortunately, it was not bumped (same as with SF_NODISKIO in 5.2.1). > 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) After this change, ngx_file_aio_init() doesn't need to be external. It is only used in ngx_file_aio_read() in corresponding ngx_*_aio_read.c. > 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,53 +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; > - > - 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; > - > - 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_http_request_t *r; > - > - aio = ev->data; > - r = aio->data; > - > - r->main->blocked--; > - r->aio = 0; > - ev->complete = 0; > - > - r->connection->write->handler(r->connection->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,22 +32,21 @@ > 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_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_SENDFILE_NODISKIO) > + ngx_uint_t ebusy; > #endif After ngx_event_aio_t *aio variable removal, this block could be placed under "eintr, eagain" line (which by itself looks unsorted). The remaining part looks good to me. > > 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; > @@ -252,54 +245,30 @@ ngx_freebsd_sendfile_chain(ngx_connectio > > in = ngx_chain_update_sent(in, sent); > > -#if (NGX_HAVE_AIO_SENDFILE) > +#if (NGX_HAVE_SENDFILE_NODISKIO) > > if (ebusy) { > - if (aio->event.active) { > - /* > - * tolerate duplicate calls; they can happen due to > subrequests > - * or multiple calls of the next body filter from a filter > - */ > - > - if (sent) { > - c->busy_count = 0; > - } > - > - return in; > - } > - > 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 > > _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
