Matt Brown <matthew.brown....@gmail.com> writes: > On Wed, May 24, 2017 at 11:36 PM, Paul Clarke <p...@us.ibm.com> wrote: >> On 05/23/2017 06:45 PM, Matt Brown wrote: >>> The xor_vmx.c file is used for the RAID5 xor operations. In these functions >>> altivec is enabled to run the operation and then disabled. However due to >>> compiler instruction reordering, altivec instructions are being run before >>> enable_altivec() and after disable_altivec(). >> >> If altivec instructions can be reordered after disable_altivec(), then >> disable_altivec() is broken, I'd think. >> >> Could it be because the isync in mtmsr_isync() is after the mtmsr? >> >> disable_kernel_altivec >> - msr_check_and_clear >> - __msr_check_and_clear >> - mtmsr_isync > > So it turns out the enable / disable functions don't actually enable > or disable the use of vector instructions. > If we have marked the file to be compiled with altivec the compiler > has free reign to reorder the vector instructions wherever it likes. > Including reordering it before or after the enable/disable_altivec > commands.
That's true. Though it's not that the compiler is reordering vector instructions, it's that it's generating them directly to save/restore vector arguments. In enable_kernel_altivec() we have a mtmsr() which is implemented with inline asm, with a "memory" clobber, so that is effectively a full compiler barrier. ie. the compiler will not reorder code across that. > The enable_kernel_altivec and disable_kernel_altivec functions are > mainly there to empty and restore the vector registers which could > have been used in user-space. So those functions work as intended, > although not particularly intuitive. Yeah, perhaps "allow" would be better. In fact the whole API is a bit crap and we need to rework it. cheers
