[ PATCH ] Add preadv2 support with RWF_NOWAIT flag

[Maxim Dounin]

Hello!

On Tue, Jan 09, 2018 at 06:06:00PM +0300, Vadim Fedorenko wrote:

> Introduction of thread pools is really good thing, but it adds
> overhead to reading files which are already in page cache in linux.
> With preadv2 (introduced in Linux 4.6) and RWF_NOWAIT flag (introduced
> in Linux 4.14) we can eliminate this overhead. Needs glibc >= 2.26
> 
> # HG changeset patch
> # User Vadim Fedorenko <vfedorenko at yandex-team.ru>
> # Date 1515498853 -10800
> #      Tue Jan 09 14:54:13 2018 +0300
> # Node ID f955f9cddd38ce35e19c50b871558ca8739a1d4b
> # Parent  6d2e92acb013224e6ef2c71c9e61ab07f0b03271
> Add preadv2() with RWF_NOWAIT flag
> 
> Eliminate overhead with threads synchronization when cache file or
> chain is in page cache already

There should be more dots here.

> diff -r 6d2e92acb013 -r f955f9cddd38 auto/unix
> --- a/auto/unix Thu Dec 28 12:01:05 2017 +0200
> +++ b/auto/unix Tue Jan 09 14:54:13 2018 +0300
> @@ -726,6 +726,21 @@
>                    if (n == -1) return 1"
>  . auto/feature
> 
> +# preadv2() was introduced in Linux 4.6, glibc 2.26
> +# RWF_NOWAIT flag was introduced in Linux 4.14
> +
> +ngx_feature="preadv2()"
> +ngx_feature_name="NGX_HAVE_PREADV2_NONBLOCK"
> +ngx_feature_run=no
> +ngx_feature_incs='#include <sys/uio.h>'
> +ngx_feature_path=
> +ngx_feature_libs=
> +ngx_feature_test="char buf[1]; struct iovec vec[1]; ssize_t n;
> +                  vec[0].iov_base = buf;
> +                  vec[0].iov_len = 1;
> +                  n = preadv2(0, vec, 1, 0, RWF_NOWAIT);
> +                  if (n == -1) return 1"
> +. auto/feature
> 
>  ngx_feature="sys_nerr"
>  ngx_feature_name="NGX_SYS_NERR"

It might be a good idea to keep the feature name closer to the 
code.  That is, it might be a good idea to use NOWAIT instead of 
NONBLOCK.

Style: as you can see, this file uses two empty lines between 
tests.  Please do so.

Also, there are various other style issues in the code, including 
use of C99 single-line comments, missing spaces and empty lines, 
error and debug messages which are not in line with other used in 
the code.  It is a good idea to cleanup the code.

> diff -r 6d2e92acb013 -r f955f9cddd38 src/core/ngx_output_chain.c
> --- a/src/core/ngx_output_chain.c       Thu Dec 28 12:01:05 2017 +0200
> +++ b/src/core/ngx_output_chain.c       Tue Jan 09 14:54:13 2018 +0300
> @@ -577,7 +577,15 @@
>          } else
>  #endif
>  #if (NGX_THREADS)
> -        if (ctx->thread_handler) {
> +#if (NGX_HAVE_PREADV2_NONBLOCK)
> +
> +        n = ngx_preadv2_file(src->file, dst->pos, (size_t) size,
> +                             src->file_pos);
> +#else
> +        n = NGX_AGAIN;
> +#endif
> +        if (n == NGX_AGAIN && ctx->thread_handler) {
> +
>              src->file->thread_task = ctx->thread_task;
>              src->file->thread_handler = ctx->thread_handler;
>              src->file->thread_ctx = ctx->filter_ctx;

Certainly we don't want platform-specific additions before each 
ngx_thread_read() call.  The preadv2() should be transparently 
called from ngx_thread_read() instead.

[...]

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