httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rainer Jung <rainer.j...@kippdata.de>
Subject Re: How to align shm in an neat way?
Date Mon, 13 Aug 2012 20:19:47 GMT
On 13.08.2012 21:02, Rainer Jung wrote:
> On 13.08.2012 19:40, Jeff Trawick wrote:
>> On Mon, Aug 13, 2012 at 1:27 PM, Rainer Jung <rainer.jung@kippdata.de>
>> wrote:
>>> On 13.08.2012 18:32, Jeff Trawick wrote:
>>>>
>>>> On Mon, Aug 13, 2012 at 12:30 PM, Rainer Jung <rainer.jung@kippdata.de>
>>>> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> PR 53040 reveals, that mod_socache_shmcb has an alignment problem.
>>>>> One of
>>>>> the three structs mapped into shm contains an apr_time_t member,
>>>>> which at
>>>>> least on Sparc is 8 Bytes, whereas for 32 bit builds long is only 4
>>>>> Bytes.
>>>>>
>>>>> Currently everything is aligned for 4 Bytes, so we get bus
>>>>> errors/crashes
>>>>> when trying to assign the apr_time_t to an address that is only
>>>>> divisible
>>>>> by
>>>>> 4 instead of 8.
>>>>>
>>>>> I can easily reproduce the problem.
>>>>>
>>>>> A possible solution is to pad the three structures SHMCBHeader,
>>>>> SHMCBSubcache and SHMCBIndex to a multiple of 8 Bytes length. For
>>>>> Subcache
>>>>> and Index this is already true by coincidence, SHMCBHeader needs
>>>>> another
>>>>> 4
>>>>> Bytes.
>>>>>
>>>>> I wonder what the right solution is. In the patch
>>>>>
>>>>> http://people.apache.org/~rjung/patches/mod_socache_shmcb-padding.patch
>>>>>
>>>>>
>>>>> I hard coded the padding, but I don't really like it, because it
>>>>> breaks
>>>>> if
>>>>> members are added to the struct. I could add a sizeof() test during
>>>>> startup
>>>>> or probably even compilation to warn or err, if the padding is wrong.
>>>>>
>>>>> I see several recipes for alignment using pragmas and attribute,
>>>>> but all
>>>>> of
>>>>> them are compiler specific.
>>>>>
>>>>> One could also wrap the struct in a wrapped struct, so that one
>>>>> could use
>>>>> the sizeof() of the inner struct to determine the padding of the outer
>>>>> struct. That would make the code convoluted.
>>>>>
>>>>> I checked other parts of the code, but couldn't find a simple
>>>>> solution.
>>>>> Any
>>>>> hints how to do this nicely?
>>>>
>>>>
>>>> APR_ALIGN_DEFAULT?
>>>
>>>
>>> I think it doesn't solve this problem, does it? It only gives me a
>>> need way
>>> to round up sizes to multiples of 8 bytes.
>>
>> It should be possible to use use APR_ALIGN_DEFAULT(sizeof(foo)) in
>> place of sizeof(foo), instead of adding explicit padding to the
>> structures. Any other pointer arithmetic which doesn't use sizeof(foo)
>> would also need to use the macro.
>
> Understood. Unfortunately currently the code doesn't really care about
> alignment. So it just puts structs SHMCBHeader, SHMCBSubcache and an
> array of SHMCBIndex directly after each other in shm. No sizeof() involved.
>
> So yes, option 3 would be to rewrite the code to calculate the
> gaps/padding between the structs using APR_ALIGN_DEFAULT() and adjust
> memory alignment in shm. I wanted to get around changing that part of
> the code ;)

I went the "choose right alignment" way now:

http://people.apache.org/~rjung/patches/mod_socache_shmcb-alignment.patch

It actually wasn't that complicated.

Regards,

Rainer


Mime
View raw message