Re: How to align shm in an neat way?
On 14.08.2012 23:20, Jim Jagielski wrote: slotmem handles this well, afaict ;) The fix applied in r1373270 which Joe also had a look at is very similar to the handling in slotmem. I added some macros as proposed by Joe. Fortunately the code in shmcb already did all the address calculations using macros so I only had to add the alignment there. Regards, Rainer
Re: How to align shm in an neat way?
slotmem handles this well, afaict ;) On Aug 13, 2012, at 12:32 PM, Jeff Trawick wrote: > On Mon, Aug 13, 2012 at 12:30 PM, Rainer Jung 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? > >> >> Regards, >> >> Rainer > > > > -- > Born in Roswell... married an alien... > http://emptyhammock.com/ >
Re: How to align shm in an neat way?
On Mon, Aug 13, 2012 at 10:19:47PM +0200, Rainer Jung wrote: > 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. Alignment problems never die with that code! +1, that looks good, might be simpler using some #defines for the APR_ALIGN_DEFAULT(blah) values rather than inlining? Regards, Joe
Re: How to align shm in an neat way?
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 wrote: On 13.08.2012 18:32, Jeff Trawick wrote: On Mon, Aug 13, 2012 at 12:30 PM, Rainer Jung 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
Re: How to align shm in an neat way?
On 13.08.2012 19:40, Jeff Trawick wrote: On Mon, Aug 13, 2012 at 1:27 PM, Rainer Jung wrote: On 13.08.2012 18:32, Jeff Trawick wrote: On Mon, Aug 13, 2012 at 12:30 PM, Rainer Jung 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 ;) Regards, Rainer
Re: How to align shm in an neat way?
On Mon, Aug 13, 2012 at 1:27 PM, Rainer Jung wrote: > On 13.08.2012 18:32, Jeff Trawick wrote: >> >> On Mon, Aug 13, 2012 at 12:30 PM, Rainer Jung >> 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. > > Regards, > > Rainer > -- Born in Roswell... married an alien... http://emptyhammock.com/
Re: How to align shm in an neat way?
On 13.08.2012 18:32, Jeff Trawick wrote: On Mon, Aug 13, 2012 at 12:30 PM, Rainer Jung 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. Regards, Rainer
Re: How to align shm in an neat way?
On Mon, Aug 13, 2012 at 12:30 PM, Rainer Jung 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? > > Regards, > > Rainer -- Born in Roswell... married an alien... http://emptyhammock.com/
How to align shm in an neat way?
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? Regards, Rainer
