[PATCH 4 of 4] Support for sendfile(SF_NOCACHE)

[Sergey Kandaurov]

> On 14 Dec 2021, at 17:16, Maxim Dounin <mdounin at mdounin.ru> wrote:
> 
> Hello!
> 
> On Tue, Nov 30, 2021 at 03:15:50PM +0300, Sergey Kandaurov wrote:
> 
>> On Thu, Nov 11, 2021 at 07:21:12AM +0300, Maxim Dounin wrote:
>>> # HG changeset patch
>>> # User Maxim Dounin <mdounin at mdounin.ru>
>>> # Date 1636603897 -10800
>>> #      Thu Nov 11 07:11:37 2021 +0300
>>> # Node ID 10f96e74ae73e1c53a3fd08e7e1c26754c8969ed
>>> # Parent  98d3beb63f32cbb68d1cdcec385614d32129cad0
>>> Support for sendfile(SF_NOCACHE).
>>> 
>>> The SF_NOCACHE flag, introduced in FreeBSD 11 along with the new non-blocking
>>> sendfile() implementation by glebius@, makes it possible to use sendfile()
>>> along with the "directio" directive.
>>> 
>>> 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
>>> @@ -256,9 +256,11 @@ ngx_output_chain_as_is(ngx_output_chain_
>>>     }
>>> #endif
>>> 
>>> +#if !(NGX_HAVE_SENDFILE_NODISKIO)
>>>     if (buf->in_file && buf->file->directio) {
>>>         return 0;
>>>     }
>>> +#endif
>> 
>> This probably deserves a comment, why it depends on such a macro test.
>> Though, it should be pretty clear from the commit log.
> 
> I also tend to think that the original code is wrong (or, rather, 
> too aggressive), and it should only disable sendfile(), much like 
> the ctx->sendfile and NGX_SENDFILE_LIMIT checks below.
> 
> Changed to the following:
> 
> 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
> @@ -256,12 +256,6 @@ ngx_output_chain_as_is(ngx_output_chain_
>     }
> #endif
> 
> -#if !(NGX_HAVE_SENDFILE_NODISKIO)
> -    if (buf->in_file && buf->file->directio) {
> -        return 0;
> -    }
> -#endif
> -
>     sendfile = ctx->sendfile;
> 
> #if (NGX_SENDFILE_LIMIT)
> @@ -272,6 +266,19 @@ ngx_output_chain_as_is(ngx_output_chain_
> 
> #endif
> 
> +#if !(NGX_HAVE_SENDFILE_NODISKIO)
> +
> +    /*
> +     * With DIRECTIO, disable sendfile() unless sendfile(SF_NOCACHE)
> +     * is available.
> +     */
> +
> +    if (buf->in_file && buf->file->directio) {
> +        sendfile = 0;
> +    }
> +
> +#endif
> +
>     if (!sendfile) {
> 
>         if (!ngx_buf_in_memory(buf)) {
> 
> 
> [...]
> 

Given the sendfile condition below, I couldn't find such cases with
buffers that have both in_file and ngx_buf_in_memory() tests passed
(at least), such that the patch would have a real behaviour change
resulting in just the in_file bit cleared.
Theoretically, it looks good and more aligned with documentation.

-- 
Sergey Kandaurov