cache: move open to thread pool

Maxim Dounin mdounin at mdounin.ru
Mon Sep 3 16:09:03 UTC 2018


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 at 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 at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel

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


More information about the nginx-devel mailing list