httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yann Ylavic <ylavic....@gmail.com>
Subject Re: httpd 2.4.25, mpm_event, ssl: segfaults
Date Thu, 23 Feb 2017 16:47:27 GMT
On Thu, Feb 23, 2017 at 5:34 PM, Yann Ylavic <ylavic.dev@gmail.com> wrote:
> On Thu, Feb 23, 2017 at 4:58 PM, Stefan Eissing
> <stefan.eissing@greenbytes.de> wrote:
>>
>>> Am 23.02.2017 um 16:38 schrieb Yann Ylavic <ylavic.dev@gmail.com>:
>>>
>>> On Wed, Feb 22, 2017 at 6:36 PM, Jacob Champion <champion.p@gmail.com>
wrote:
>>>> On 02/22/2017 12:00 AM, Stefan Eissing wrote:
>>>>>
>>>>> Just so I do not misunderstand:
>>>>>
>>>>> you increased BUCKET_BUFF_SIZE in APR from 8000 to 64K? That is what
you
>>>>> are testing?
>>>>
>>>>
>>>> Essentially, yes, *and* turn off mmap and sendfile. My hope is to disable
>>>> the mmap-optimization by default while still improving overall performance
>>>> for most users.
>>>>
>>>> Technically, Yann's patch doesn't redefine APR_BUCKET_BUFF_SIZE, it just
>>>> defines a new buffer size for use with the file bucket. It's a little less
>>>> than 64K, I assume to make room for an allocation header:
>>>>
>>>>    #define FILE_BUCKET_BUFF_SIZE (64 * 1024 - 64)
>>>
>>> Actually I'm not very pleased with this solution (or the final one
>>> that would make this size open / configurable).
>>> The issue is potentially the huge (order big-n) allocations which
>>> finally may hurt the system (fragmentation, OOM...).
>>>
>>> So I'm thinking of another way to achieve the same with the current
>>> APR_BUCKET_BUFF_SIZE (2 pages) per alloc.
>>>
>>> The idea is to have a new apr_allocator_allocv() function which would
>>> fill an iovec with what's available in the allocator's freelist (i.e.
>>> spare apr_memnodes) of at least the given min_size bytes (possibly a
>>> max too but I don't see the need for now) and up to the size of the
>>> given iovec.
>>
>> Interesting. Not only for pure files maybe.
>>
>> It would be great if there'd be a SSL_writev()...
>
> Indeed, openssl would fully fill the TLS records.
>
>> but until there is, the TLS case will either turn every iovec into
>> its own TLS record
>
> Yes, but it's more a client issue to work with these records finally
> (because from there all is networking only, and coalescing will happen
> in the core output filter from a socket POV).
>
> Anyway mod_proxy(s) could be the client, so...
>
> I've no idea how much it costs to have 8K vs 16K records, though.
> Maybe in the mod_ssl case we'd want 16K buffers, still reasonable?
>
>> or one needs another copy before that. This last strategy is used by
>> mod_http2. Since there are 9 header bytes per frame, copying data
>> into a right sized buffer gives better performance. So, it would be
>> nice to read n bytes from a bucket brigade and get iovecs back.
>> Which, as I understand it, you propose?
>
> I didn't think of apr_bucket_readv(), more focused on
> file_bucket_read() to do this internally/transparently, but once file
> buckets can do that I think we can generalize the concept, at worse
> filling only iovec[0] when it's ENOTIMPL and/or makes no sense...
>
> That'd help the mod_ssl case with something like
> apr_bucket_readv(min_size=16K), I'll try to think of it once/if I can
> have a more simple to do file_bucket_read() only working ;)

Hm no, this needs to happen on the producer side, not at final
apr_bucket_read().
So for the mod_ssl case I think we could have a simple (new)
apr_bucket_alloc_set_size(16K) in the first place if it helps more
than hurts.

As for your question about "iovecs back" (which I finally didn't
answer), all happens at the bucket alloc level, when buckets are
deleted.

Mime
View raw message