Messages in this thread | | | Date | Thu, 27 Jan 2000 00:06:31 -0800 | From | Chris Sears <> | Subject | <linux/linkage.h> generates incorrect cache alignments for 486 and above |
| |
Mike,
here is the code from <linux/linkage.h>
#if !defined(__i486__) && !defined(__i586__) #define __ALIGN .align 4,0x90 #define __ALIGN_STR ".align 4,0x90" #else /* __i486__/__i586__ */ #define __ALIGN .align 16,0x90 #define __ALIGN_STR ".align 16,0x90" #endif /* __i486__/__i586__ */
You can see that __i686__ is missing from the list so it defaults to the i386 case.
The 16 should be 32 as cache.h points out.
Here is the quotation from the gas manual:
.align abs-expr , abs-expr
Pad the location counter (in the current subsection) to a particular storage boundary. The first expression (which must be absolute) is the number of low-order zero bits the location counter must have after advancement. For example `.align 3' advances the location counter until it a multiple of 8. If the location counter is already a multiple of 8, no change is needed.
Well now, this is a docubug. The 16 is aligning to 16 byte boundaries, I've looked at the readelf output. Also, setting this to 5 generates assembler complaints and 2^16 is large even for a disk page, let alone a cache line. The comment at the beginning of entry.S gets it wrong as well.
* I changed all the .align's to 4 (16 byte alignment), as that's faster * on a 486.
> afaik, __ALIGN has nothing to do with cache-line size.. L1_CACHE_BYTES. > (hmm.. am I out to lunch or should __ALIGN be set to 0 for pentia?)
I'm definitely not with you on setting __ALIGN to 0. ALIGN represents cache line alignment. It really does. It is used sparingly in entry.S and head.S
Here is asm-i386/cache.h
/* * include/asm-i386/cache.h */ #ifndef __ARCH_I386_CACHE_H #define __ARCH_I386_CACHE_H
/* bytes per L1 cache line */ #if CPU==586 || CPU==686 #define L1_CACHE_BYTES 32 #else #define L1_CACHE_BYTES 16 #endif
#endif
You are very right. I think it makes excellent sense to use L1_CACHE_BYTES in <linux/linkage.h> But note that this is inconsistant with <linux/linkage.h> which needs to get fixed.
> That's done in specs:predefines. __i386__ is always defined, and as > soon as you tell it which cpu you're using, (see cpp_cpu) cpp adds > to it's defines accordingly.
I don't know what you mean but I'm guessing that this is a gcc configuration file. Just where I want to look to know about linux. I think Alan Cox is always railing about being conservative about compiler releases. You want to insulate yourself from gcc's whims.
> I don't see anything at all wrong with using __ix86__ because the > kernel isn't going to be compiled by anything other than gcc without > doing a truckload of rewriting.
I did NOT say that the kernel would/should/could be compiled on something other than gcc. But that doesn't mean that it should rely on gcc for configuration. make xconfig creates <linux/autoconf.h> The defines are consistant and I know where to look and what they mean. Why use TWO mechanisms? Why not THREE?
In summary, in addition to my posted patch, linkage.h should use cache.h Thanks for forcing me to work through it one more time. It definitely improved it. I'll post a mo' better patch in the morning.
thanks,
Chris Sears cbsears@ix.netcom.com
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |