Hello!
On Mon, Aug 27, 2018 at 02:38:36PM -0700, Ka-Hing Cheung via nginx-devel wrote:
> Thanks for all the feedback! This is an updated version of the patch.
> There may still be some coding style issues (which will be addressed),
> but should address most of the comments other than these two:
>
> > - The code bypasses open file cache, and uses a direct call
> > in the http cache code instead. While it might be ok in your
> > setup, it looks like an incomplete solution from the generic point
> > of view. A better solution would be to introduce a generic
> > interface in ngx_open_cached_file() to allow use of thread
> > pools.
>
> The complications with this just didn't seem worth it. aio_open makes
> the most impact if a large number of distinct files need to be opened
> and that's when open_file_cache is last effectiv. Instead I made
> aio_open to only take effect if open_file_cache is off.
The biggest problem with in-place solution is that it requires
in-place coding for all the places where we want to use aio_open.
Currently we use ngx_open_cached_file() to open files everywhere,
and it would be a good idea to preserve it as a starting point for
all file open operations.
This should be beneficial even if aio_open is only used when open
file cache is actually disabled. And I'm actually fine with
implementing this only when open file cache is disabled - but
implementing this in ngx_open_cached_file(), to make it easier to
add more places which use aio_open, and also makeing it possible
to further improve things by making it compatible with
open_file_cache.
Adding a code path somewhere in ngx_open_file_wrapper() should be
simple enough. Note that probably there is no need to cover
stat() as well, as well as to address various symlinks cases - I
think it would be fine if aio_open will only work when there is no
need to disable symlinks, and will only work for open(), as stat()
after an open is expected to be fast.
> > - The code calls ngx_open_and_stat_file() whithin a thread, which
> > is relatively complex function never designed to be thread safe.
> > While it looks like currently it is, a better solution would be to
> > introduce a separate simple function to execute within a thread,
> > similar to ngx_thread_read_handler().
>
> I kept using ngx_open_and_stat_file() as we are looking into moving
> other types of open into thread pool, so we will probably need to have
> similar logic anyway.
The problem is that ngx_open_and_stat_file() is never coded to be
thread safe, and this is expected to cause problems sooner or
later - as we've seen in the previous version of your patch, which
tried to use pool allocations.
A separate simple function is needed to avoid such problems - and
to make it explicitly obvious that the code is expected to be
thread safe.
>
> commit 3c67f1d972404bda7713839186a91cb18dbc96be
> Author: Ka-Hing Cheung <kahing@cloudflare.com>
> Date: Fri Aug 3 13:37:58 2018 -0700
>
> move open to thread pool
>
> At cloudflare we found that open() can block a long time, especially
> at p99 and p999. Moving it to thread pool improved overall p99 by 6x
> or so during peak time.
>
> This introduces an aio_open option. When set to 'on' the open will be
> done in a thread pool. open_file_cache has to be disabled for aio_open
> to take effect.
>
> thread pool does increase CPU usage somewhat but we have not measured
> difference in CPU usage for stock "aio threads" compared to after this
> patch.
>
> Only the cache hit open() is moved to thread pool.
>
> diff --git src/core/ngx_open_file_cache.c src/core/ngx_open_file_cache.c
> index b23ee78d..9177ebfd 100644
> --- src/core/ngx_open_file_cache.c
> +++ src/core/ngx_open_file_cache.c
> @@ -35,8 +35,6 @@ static ngx_fd_t ngx_open_file_wrapper(ngx_str_t *name,
> ngx_int_t access, ngx_log_t *log);
> static ngx_int_t ngx_file_info_wrapper(ngx_str_t *name,
> ngx_open_file_info_t *of, ngx_file_info_t *fi, ngx_log_t *log);
> -static ngx_int_t ngx_open_and_stat_file(ngx_str_t *name,
> - ngx_open_file_info_t *of, ngx_log_t *log);
> static void ngx_open_file_add_event(ngx_open_file_cache_t *cache,
> ngx_cached_open_file_t *file, ngx_open_file_info_t *of, ngx_log_t *log);
> static void ngx_open_file_cleanup(void *data);
> @@ -836,7 +834,7 @@ ngx_file_info_wrapper(ngx_str_t *name,
> ngx_open_file_info_t *of,
> }
>
>
> -static ngx_int_t
> +ngx_int_t
> ngx_open_and_stat_file(ngx_str_t *name, ngx_open_file_info_t *of,
> ngx_log_t *log)
> {
> diff --git src/core/ngx_open_file_cache.h src/core/ngx_open_file_cache.h
> index d119c129..144744c0 100644
> --- src/core/ngx_open_file_cache.h
> +++ src/core/ngx_open_file_cache.h
> @@ -7,6 +7,7 @@
>
> #include <ngx_config.h>
> #include <ngx_core.h>
> +#include <ngx_queue.h>
>
>
> #ifndef _NGX_OPEN_FILE_CACHE_H_INCLUDED_
> @@ -124,6 +125,8 @@ ngx_open_file_cache_t
> *ngx_open_file_cache_init(ngx_pool_t *pool,
> ngx_uint_t max, time_t inactive);
> ngx_int_t ngx_open_cached_file(ngx_open_file_cache_t *cache, ngx_str_t *name,
> ngx_open_file_info_t *of, ngx_pool_t *pool);
> +ngx_int_t ngx_open_and_stat_file(ngx_str_t *name,
> + ngx_open_file_info_t *of, ngx_log_t *log);
>
>
> #endif /* _NGX_OPEN_FILE_CACHE_H_INCLUDED_ */
> diff --git src/http/ngx_http_cache.h src/http/ngx_http_cache.h
> index f9e96640..8940405a 100644
> --- src/http/ngx_http_cache.h
> +++ src/http/ngx_http_cache.h
> @@ -97,6 +97,7 @@ struct ngx_http_cache_s {
>
> #if (NGX_THREADS || NGX_COMPAT)
> ngx_thread_task_t *thread_task;
> + ngx_int_t open_rv;
> #endif
>
> ngx_msec_t lock_timeout;
> @@ -114,6 +115,9 @@ struct ngx_http_cache_s {
> unsigned exists:1;
> unsigned temp_file:1;
> unsigned purged:1;
> +#if (NGX_THREADS || NGX_COMPAT)
> + unsigned opening:1;
> +#endif
> unsigned reading:1;
> unsigned secondary:1;
> unsigned background:1;
> diff --git src/http/ngx_http_core_module.c src/http/ngx_http_core_module.c
> index c57ec00c..1c7b26c2 100644
> --- src/http/ngx_http_core_module.c
> +++ src/http/ngx_http_core_module.c
> @@ -420,6 +420,13 @@ static ngx_command_t ngx_http_core_commands[] = {
> offsetof(ngx_http_core_loc_conf_t, aio_write),
> NULL },
>
> + { ngx_string("aio_open"),
> + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_FLAG,
> + ngx_conf_set_flag_slot,
> + NGX_HTTP_LOC_CONF_OFFSET,
> + offsetof(ngx_http_core_loc_conf_t, aio_open),
> + NULL },
> +
> { ngx_string("read_ahead"),
> NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1,
> ngx_conf_set_size_slot,
> @@ -3380,6 +3387,7 @@ ngx_http_core_create_loc_conf(ngx_conf_t *cf)
> clcf->subrequest_output_buffer_size = NGX_CONF_UNSET_SIZE;
> clcf->aio = NGX_CONF_UNSET;
> clcf->aio_write = NGX_CONF_UNSET;
> + clcf->aio_open = NGX_CONF_UNSET;
> #if (NGX_THREADS)
> clcf->thread_pool = NGX_CONF_UNSET_PTR;
> clcf->thread_pool_value = NGX_CONF_UNSET_PTR;
> @@ -3605,6 +3613,7 @@ ngx_http_core_merge_loc_conf(ngx_conf_t *cf,
> void *parent, void *child)
> (size_t) ngx_pagesize);
> ngx_conf_merge_value(conf->aio, prev->aio, NGX_HTTP_AIO_OFF);
> ngx_conf_merge_value(conf->aio_write, prev->aio_write, 0);
> + ngx_conf_merge_value(conf->aio_open, prev->aio_open, 0);
> #if (NGX_THREADS)
> ngx_conf_merge_ptr_value(conf->thread_pool, prev->thread_pool, NULL);
> ngx_conf_merge_ptr_value(conf->thread_pool_value, prev->thread_pool_value,
> diff --git src/http/ngx_http_core_module.h src/http/ngx_http_core_module.h
> index 4c6da7c0..8498eb63 100644
> --- src/http/ngx_http_core_module.h
> +++ src/http/ngx_http_core_module.h
> @@ -382,6 +382,7 @@ struct ngx_http_core_loc_conf_s {
> ngx_flag_t sendfile; /* sendfile */
> ngx_flag_t aio; /* aio */
> ngx_flag_t aio_write; /* aio_write */
> + ngx_flag_t aio_open; /* aio_open */
> ngx_flag_t tcp_nopush; /* tcp_nopush */
> ngx_flag_t tcp_nodelay; /* tcp_nodelay */
> ngx_flag_t reset_timedout_connection; /* reset_timedout_connection */
> diff --git src/http/ngx_http_file_cache.c src/http/ngx_http_file_cache.c
> index 56866fa4..10a29d93 100644
> --- src/http/ngx_http_file_cache.c
> +++ src/http/ngx_http_file_cache.c
> @@ -18,6 +18,8 @@ static void
> ngx_http_file_cache_lock_wait(ngx_http_request_t *r,
> ngx_http_cache_t *c);
> static ngx_int_t ngx_http_file_cache_read(ngx_http_request_t *r,
> ngx_http_cache_t *c);
> +static ngx_int_t ngx_http_file_cache_aio_open(ngx_http_request_t *r,
> + ngx_http_cache_t *c, ngx_int_t rv);
> static ssize_t ngx_http_file_cache_aio_read(ngx_http_request_t *r,
> ngx_http_cache_t *c);
> #if (NGX_HAVE_FILE_AIO)
> @@ -268,9 +270,7 @@ ngx_http_file_cache_open(ngx_http_request_t *r)
> ngx_uint_t test;
> ngx_http_cache_t *c;
> ngx_pool_cleanup_t *cln;
> - ngx_open_file_info_t of;
> ngx_http_file_cache_t *cache;
> - ngx_http_core_loc_conf_t *clcf;
>
> c = r->cache;
>
> @@ -278,6 +278,12 @@ ngx_http_file_cache_open(ngx_http_request_t *r)
> return NGX_AGAIN;
> }
>
> +#if (NGX_THREADS)
> + if (c->opening) {
> + return ngx_http_file_cache_aio_open(r, c, c->open_rv);
> + }
> +#endif
> +
While this looks slightly better than in the previous iteration,
this still leaves a lot to be desired. In particular, this still
uses duplicate "rv == NGX_DECLINED" checks.
> if (c->reading) {
> return ngx_http_file_cache_read(r, c);
> }
> @@ -339,58 +345,10 @@ ngx_http_file_cache_open(ngx_http_request_t *r)
> return NGX_ERROR;
> }
>
> - if (!test) {
> - goto done;
> - }
> -
> - clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
> -
> - ngx_memzero(&of, sizeof(ngx_open_file_info_t));
> -
> - of.uniq = c->uniq;
> - of.valid = clcf->open_file_cache_valid;
> - of.min_uses = clcf->open_file_cache_min_uses;
> - of.events = clcf->open_file_cache_events;
> - of.directio = NGX_OPEN_FILE_DIRECTIO_OFF;
> - of.read_ahead = clcf->read_ahead;
> -
> - if (ngx_open_cached_file(clcf->open_file_cache, &c->file.name,
> &of, r->pool)
> - != NGX_OK)
> - {
> - switch (of.err) {
> -
> - case 0:
> - return NGX_ERROR;
> -
> - case NGX_ENOENT:
> - case NGX_ENOTDIR:
> - goto done;
> -
> - default:
> - ngx_log_error(NGX_LOG_CRIT, r->connection->log, of.err,
> - ngx_open_file_n " \"%s\" failed", c->file.name.data);
> - return NGX_ERROR;
> - }
> + if (test) {
> + return ngx_http_file_cache_aio_open(r, c, rv);
> }
>
> - ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
> - "http file cache fd: %d", of.fd);
> -
> - c->file.fd = of.fd;
> - c->file.log = r->connection->log;
> - c->uniq = of.uniq;
> - c->length = of.size;
> - c->fs_size = (of.fs_size + cache->bsize - 1) / cache->bsize;
> -
> - c->buf = ngx_create_temp_buf(r->pool, c->body_start);
> - if (c->buf == NULL) {
> - return NGX_ERROR;
> - }
> -
> - return ngx_http_file_cache_read(r, c);
> -
> -done:
> -
> if (rv == NGX_DECLINED) {
> return ngx_http_file_cache_lock(r, c);
> }
> @@ -398,7 +356,6 @@ done:
> return rv;
> }
>
> -
> static ngx_int_t
Unrelated change and a style issue. As previously pointed out,
there should be two empty lines between functions.
> ngx_http_file_cache_lock(ngx_http_request_t *r, ngx_http_cache_t *c)
> {
> @@ -663,6 +620,106 @@ ngx_http_file_cache_read(ngx_http_request_t *r,
> ngx_http_cache_t *c)
> }
>
>
> +static ngx_int_t
> +ngx_http_file_cache_aio_open(ngx_http_request_t *r, ngx_http_cache_t *c,
> + ngx_int_t rv)
> +{
> + ngx_int_t rc;
> + ngx_open_file_info_t of = { 0 };
Style: assignments should be written separately, except some
specific cases where one assignment is allowed in a separate
variables block.
> + ngx_http_file_cache_t *cache;
> + ngx_http_core_loc_conf_t *clcf;
> +
> + clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
> +
> + of.uniq = c->uniq;
> + of.valid = clcf->open_file_cache_valid;
> + of.min_uses = clcf->open_file_cache_min_uses;
> + of.events = clcf->open_file_cache_events;
> + of.directio = NGX_OPEN_FILE_DIRECTIO_OFF;
> + of.read_ahead = clcf->read_ahead;
> +
> +#if (NGX_THREADS)
> + if (clcf->aio == NGX_HTTP_AIO_THREADS && clcf->aio_open &&
> + clcf->open_file_cache == NULL) {
Style: as long as #if contains something complex, it should be
separated with empty lines.
Style: the "&&" shouldn't be left hanging at the end of line, it
should be wrapped to the next line instead.
Style: as long as "if" condition is on multiple lines, "{" should
be on its own line.
> +
> + c->file.thread_task = c->thread_task;
> + c->file.thread_handler = ngx_http_cache_thread_handler;
> + c->file.thread_ctx = r;
> + c->open_rv = rv;
> +
> + rc = ngx_thread_open(&c->file, &of, r->pool);
> +
> + c->opening = (rc == NGX_AGAIN);
> + c->thread_task = c->file.thread_task;
> +
> + if (rc == NGX_AGAIN) {
> + return rc;
> + } else if (of.fd != NGX_INVALID_FILE) {
> + ngx_pool_cleanup_t *cln;
> + ngx_pool_cleanup_file_t *clnf;
> +
> + cln = ngx_pool_cleanup_add(r->pool,
> sizeof(ngx_pool_cleanup_file_t));
> + if (cln == NULL) {
> + ngx_close_file(of.fd);
> + goto done;
> + }
> +
> + cln->handler = ngx_pool_cleanup_file;
> + clnf = cln->data;
> +
> + clnf->fd = of.fd;
> + clnf->name = r->cache->file.name.data;
> + clnf->log = r->connection->log;
> + goto post_open;
> + }
> + }
> +#endif
> +
> + rc = ngx_open_cached_file(clcf->open_file_cache, &c->file.name,
> &of, r->pool);
> +
> +#if (NGX_THREADS)
> +post_open:
> +#endif
As previously suggested, all this logic should be hidden in the
ngx_open_cached_file() function instead. You may want to take a
look at how it is done in ngx_write_chain_to_temp_file() for an
example.
> +
> + if (rc != NGX_OK) {
> + switch (of.err) {
> + case NGX_OK:
> + return NGX_ERROR;
> + case NGX_ENOENT:
> + case NGX_ENOTDIR:
> + goto done;
> +
> + default:
> + ngx_log_error(NGX_LOG_CRIT, r->connection->log, of.err,
> + ngx_open_file_n " \"%s\" failed", c->file.name.data);
> + return NGX_ERROR;
> + }
Style: you may want to take a look at the original code to find
out how to put additional empty lines.
> + }
> +
> + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
> + "http file cache fd: %d", of.fd);
> +
> + cache = c->file_cache;
> + c->file.fd = of.fd;
> + c->file.log = r->connection->log;
> + c->uniq = of.uniq;
> + c->length = of.size;
> + c->fs_size = (of.fs_size + cache->bsize - 1) / cache->bsize;
> +
> + c->buf = ngx_create_temp_buf(r->pool, c->body_start);
> + if (c->buf == NULL) {
> + return NGX_ERROR;
> + }
> +
> + return ngx_http_file_cache_read(r, c);
> +
> +done:
> + if (rv == NGX_DECLINED) {
> + return ngx_http_file_cache_lock(r, c);
> + }
> + return rv;
Style: see above, this needs more empty lines.
Also, it might be a good idea to keep the "rv == NGX_DECLINED" /
ngx_http_file_cache_lock() processing in a single place.
> +}
> +
> static ssize_t
Style: as previously pointed out, there should be two empty lines between
functions.
> ngx_http_file_cache_aio_read(ngx_http_request_t *r, ngx_http_cache_t *c)
> {
> @@ -751,30 +808,14 @@ ngx_http_cache_aio_event_handler(ngx_event_t *ev)
> static ngx_int_t
> ngx_http_cache_thread_handler(ngx_thread_task_t *task, ngx_file_t *file)
> {
> - ngx_str_t name;
> ngx_thread_pool_t *tp;
> ngx_http_request_t *r;
> - ngx_http_core_loc_conf_t *clcf;
>
> r = file->thread_ctx;
>
> - clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
> - tp = clcf->thread_pool;
> -
> + tp = ngx_http_request_get_thread_pool(r);
> if (tp == NULL) {
> - if (ngx_http_complex_value(r, clcf->thread_pool_value, &name)
> - != NGX_OK)
> - {
> - return NGX_ERROR;
> - }
> -
> - tp = ngx_thread_pool_get((ngx_cycle_t *) ngx_cycle, &name);
> -
> - if (tp == NULL) {
> - ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
> - "thread pool \"%V\" not found", &name);
> - return NGX_ERROR;
> - }
> + return NGX_ERROR;
> }
>
> task->event.data = r;
This change looks unrelated, as ngx_http_request_get_thread_pool()
is never used elsewhere in this patch.
> diff --git src/http/ngx_http_request.c src/http/ngx_http_request.c
> index 97900915..228cda3d 100644
> --- src/http/ngx_http_request.c
> +++ src/http/ngx_http_request.c
> @@ -3700,3 +3700,39 @@ ngx_http_log_error_handler(ngx_http_request_t
> *r, ngx_http_request_t *sr,
>
> return buf;
> }
> +
> +
> +#if (NGX_THREADS)
> +
> +ngx_thread_pool_t *
> +ngx_http_request_get_thread_pool(ngx_http_request_t *r)
> +{
> + ngx_str_t name;
> + ngx_thread_pool_t *tp;
> + ngx_http_core_loc_conf_t *clcf;
> +
> + clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
> + tp = clcf->thread_pool;
> +
> + if (tp == NULL) {
> + if (ngx_http_complex_value(r, clcf->thread_pool_value, &name)
> + != NGX_OK)
> + {
> + return NULL;
> + }
> +
> + tp = ngx_thread_pool_get((ngx_cycle_t *) ngx_cycle, &name);
> +
> + if (tp == NULL) {
> + ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
> + "thread pool \"%V\" not found", &name);
> + return NULL;
> + }
> +
> + clcf->thread_pool = tp;
> + }
> +
> + return tp;
> +}
> +
> +#endif
> diff --git src/http/ngx_http_request.h src/http/ngx_http_request.h
> index 6bfff96e..12df9c76 100644
> --- src/http/ngx_http_request.h
> +++ src/http/ngx_http_request.h
> @@ -605,4 +605,10 @@ extern ngx_http_header_out_t ngx_http_headers_out[];
> ((ngx_http_log_ctx_t *) log->data)->current_request = r
>
>
> +#if (NGX_THREADS)
> +typedef struct ngx_thread_pool_s ngx_thread_pool_t;
> +
> +ngx_thread_pool_t *ngx_http_request_get_thread_pool(ngx_http_request_t *r);
> +#endif
> +
> #endif /* _NGX_HTTP_REQUEST_H_INCLUDED_ */
> diff --git src/os/unix/ngx_files.c src/os/unix/ngx_files.c
> index 482d3276..3260c2aa 100644
> --- src/os/unix/ngx_files.c
> +++ src/os/unix/ngx_files.c
> @@ -88,15 +88,94 @@ typedef struct {
>
> size_t nbytes;
> ngx_err_t err;
> +} ngx_thread_file_read_ctx_t;
> +
> +typedef struct {
> + ngx_str_t name;
> + ngx_pool_t *pool;
As previously explained, you cannot use the request pool in a
thread. As such, this field is not needed.
> + ngx_open_file_info_t of;
> + ngx_int_t rv;
> +} ngx_thread_file_open_ctx_t;
> +
> +typedef struct {
> + union {
> + ngx_thread_file_open_ctx_t open;
> + ngx_thread_file_read_ctx_t read;
> + };
> } ngx_thread_file_ctx_t;
Please keep things in a single structure instead. This will allow
to avoid many coresponding changes in the file.
>
>
> +static void
> +ngx_thread_open_handler(void *data, ngx_log_t *log)
> +{
> + ngx_int_t rc;
> + ngx_open_file_info_t *of;
> + ngx_thread_file_open_ctx_t *ctx;
> +
> + ngx_log_debug0(NGX_LOG_DEBUG_CORE, log, 0, "thread open handler");
> +
> + ctx = data;
> + of = &ctx->of;
> +
> + of->fd = NGX_INVALID_FILE;
> +
> + rc = ngx_open_and_stat_file(&ctx->name, of, log);
> + if (rc != NGX_OK && of->err == NGX_OK) {
> + of->err = rc;
> + }
> +}
> +
> +
> +ngx_int_t
> +ngx_thread_open(ngx_file_t *file, ngx_open_file_info_t *of,
> + ngx_pool_t *pool)
> +{
> + ngx_thread_task_t *task;
> + ngx_thread_file_open_ctx_t *ctx;
> +
> + ngx_log_debug1(NGX_LOG_DEBUG_CORE, file->log, 0,
> + "thread open: %s", file->name.data);
> +
> + task = file->thread_task;
> +
> + if (task == NULL) {
> + task = ngx_thread_task_alloc(pool, sizeof(ngx_thread_file_ctx_t));
> + if (task == NULL) {
> + return NGX_ERROR;
> + }
> +
> + file->thread_task = task;
> + }
> +
> + ctx = task->ctx;
> +
> + if (task->event.complete) {
> + task->event.complete = 0;
> + *of = ctx->of;
> +
> + return of->err;
> + }
> +
> + task->handler = ngx_thread_open_handler;
> +
> + ctx->pool = pool;
> + ctx->name = file->name;
> + ctx->of = *of;
> +
> + if (file->thread_handler(task, file) != NGX_OK) {
> + return NGX_ERROR;
> + }
> +
> + return NGX_AGAIN;
> +}
> +
> +
> ssize_t
> ngx_thread_read(ngx_file_t *file, u_char *buf, size_t size, off_t offset,
> ngx_pool_t *pool)
> {
> ngx_thread_task_t *task;
> - ngx_thread_file_ctx_t *ctx;
> + ngx_thread_file_read_ctx_t *ctx;
>
> ngx_log_debug4(NGX_LOG_DEBUG_CORE, file->log, 0,
> "thread read: %d, %p, %uz, %O",
> @@ -155,7 +234,7 @@ ngx_thread_read(ngx_file_t *file, u_char *buf,
> size_t size, off_t offset,
> static void
> ngx_thread_read_handler(void *data, ngx_log_t *log)
> {
> - ngx_thread_file_ctx_t *ctx = data;
> + ngx_thread_file_read_ctx_t *ctx = data;
>
> ssize_t n;
>
> @@ -478,7 +557,7 @@ ngx_thread_write_chain_to_file(ngx_file_t *file,
> ngx_chain_t *cl, off_t offset,
> ngx_pool_t *pool)
> {
> ngx_thread_task_t *task;
> - ngx_thread_file_ctx_t *ctx;
> + ngx_thread_file_read_ctx_t *ctx;
>
> ngx_log_debug3(NGX_LOG_DEBUG_CORE, file->log, 0,
> "thread write chain: %d, %p, %O",
> @@ -488,7 +567,7 @@ ngx_thread_write_chain_to_file(ngx_file_t *file,
> ngx_chain_t *cl, off_t offset,
>
> if (task == NULL) {
> task = ngx_thread_task_alloc(pool,
> - sizeof(ngx_thread_file_ctx_t));
> + sizeof(ngx_thread_file_read_ctx_t));
> if (task == NULL) {
> return NGX_ERROR;
> }
> @@ -536,7 +615,7 @@ ngx_thread_write_chain_to_file(ngx_file_t *file,
> ngx_chain_t *cl, off_t offset,
> static void
> ngx_thread_write_chain_to_file_handler(void *data, ngx_log_t *log)
> {
> - ngx_thread_file_ctx_t *ctx = data;
> + ngx_thread_file_read_ctx_t *ctx = data;
>
> #if (NGX_HAVE_PWRITEV)
>
> diff --git src/os/unix/ngx_files.h src/os/unix/ngx_files.h
> index 07872b13..2659c0cb 100644
> --- src/os/unix/ngx_files.h
> +++ src/os/unix/ngx_files.h
> @@ -12,11 +12,11 @@
> #include <ngx_config.h>
> #include <ngx_core.h>
>
> -
Unrelated (and an incorrect from style point of view) change.
> typedef int ngx_fd_t;
> typedef struct stat ngx_file_info_t;
> typedef ino_t ngx_file_uniq_t;
>
> +#include <ngx_open_file_cache.h>
>
This introduces a dependency of low-level code from high-level
OS-independant structures in ngx_open_file_cache.h. Instead, some
low-level interface should be used.
> typedef struct {
> u_char *name;
> @@ -385,6 +385,8 @@ extern ngx_uint_t ngx_file_aio;
> #endif
>
> #if (NGX_THREADS)
> +ngx_int_t ngx_thread_open(ngx_file_t *file, ngx_open_file_info_t *of,
> + ngx_pool_t *pool);
> ssize_t ngx_thread_read(ngx_file_t *file, u_char *buf, size_t size,
> off_t offset, ngx_pool_t *pool);
> ssize_t ngx_thread_write_chain_to_file(ngx_file_t *file, ngx_chain_t *cl,
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
