Tian, Kevin
2005-Nov-22 04:44 UTC
RE: [Xen-devel] __ia64__ ifdef in xmalloc.c: "Fix ar.unat handling forfast paths"
>From: Rusty Russell >Sent: 2005年11月21日 12:53 >Hi all, > > While browsing the code, I noticed this in xmalloc.c: > >#ifndef __ia64__ > BUG_ON(align > SMP_CACHE_BYTES); >#endif > > This is clearly wrong: due to header alignment we cannot give you a >greater alignment than SMP_CACHE_BYTES. Overriding this will cause the >allocation to succeed, but not give the alignment requested. It usually >indicates the caller should be fixed. > >Does someone with an ia64 box know why, or want to rip it out and see >what breaks? > >Thanks! >Rusty. >-- >A bad analogy is like a leaky screwdriver -- Richard Braakman >I didn''t note that ifdef before, and removing it doesn''t break dom0. I doubt the reason from following definition which is copied from Linux directly into Xen: #ifdef CONFIG_SMP # define SMP_CACHE_SHIFT L1_CACHE_SHIFT # define SMP_CACHE_BYTES L1_CACHE_BYTES #else /* * The "aligned" directive can only _increase_ alignment, so this is * safe and provides an easy way to avoid wasting space on a * uni-processor: */ # define SMP_CACHE_SHIFT 3 # define SMP_CACHE_BYTES (1 << 3) #endif For a UP system, SMP_CACHE_BYTES is only 8 bytes to save space. Then maybe several months ago, there''s such requirement to xmalloc a 16bytes aligned (legal on ia64 system like a long double) structure and then failed due to BUG_ON with an immediate lazy hack to add ifdef there. For now, such case seems disappearing after a quick test. However I think the preferred way is to increase SMP_CACHE_SHIFT to 4 to match maximum allowed alignment value. But I''m not sure from the comment above why "3" is defined there on native linux. CC to linux-ia64 list for standard answer. ;-) Thanks, Kevin _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Luck, Tony
2005-Nov-22 16:11 UTC
Re: [Xen-devel] __ia64__ ifdef in xmalloc.c: "Fix ar.unat handling forfast paths"
This comment:> /* > * The "aligned" directive can only _increase_ alignment, so this is > * safe and provides an easy way to avoid wasting space on a > * uni-processor: > */suggests that we only expected SMP_CACHE_BYTES to be used in "aligned" directives, where having a smaller value than the actual alignment requirement of some object would simply be ignored by the compiler. A search for SMP_CACHE_BYTES across the kernel picks up a few drivers that don''t follow this usage model. E.g. drivers/net/acenic.c might print a warning about unsupported cache line size on a UP ia64. Your usage: BUG_ON(align > SMP_CACHE_BYTES); is another instance of this define being used in a way that ia64 didn''t expect (it appears that only ia64 has this space saving trick in the definition of SMP_CACHE_BYTES ... so it is unsurprising that general users are not following our usage model). How much is this trick saving us? Static size of data area in vmlinux doesn''t change very much as SMP_CACHE_BYTES is varied: text data bss dec hex filename 8677481 1139704 1206357 11023542 a834b6 vmlinux-8 8677417 1141808 1206397 11025622 a83cd6 vmlinux-16 8677417 1146000 1206477 11029894 a84d86 vmlinux-32 8677353 1146256 1206573 11030182 a84ea6 vmlinux-64 8677353 1163152 1207085 11047590 a892a6 vmlinux-128 I''m not sure how to evaluate dynamic behavior (allocation of structures whose size is dependent on SMP_CACHE_BYTES at runtime). -Tony _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
David Mosberger-Tang
2005-Nov-22 19:34 UTC
Re: [Xen-devel] __ia64__ ifdef in xmalloc.c: "Fix ar.unat handling forfast paths"
I don''t think it''s worth going to great lengths to preserve the SMP_CACHE_BYTES == 8 for UP. We should be able to just make it 16 and be done with it (perhaps adding a comment that SMP_CACHE_BYTES must be>= max alignment of any C type).--david On 11/22/05, Luck, Tony <tony.luck@intel.com> wrote:> This comment: > > /* > > * The "aligned" directive can only _increase_ alignment, so this is > > * safe and provides an easy way to avoid wasting space on a > > * uni-processor: > > */ > suggests that we only expected SMP_CACHE_BYTES to be used in "aligned" > directives, where having a smaller value than the actual alignment > requirement of some object would simply be ignored by the compiler. > > A search for SMP_CACHE_BYTES across the kernel picks up a few drivers > that don''t follow this usage model. E.g. drivers/net/acenic.c might > print a warning about unsupported cache line size on a UP ia64. > > Your usage: > BUG_ON(align > SMP_CACHE_BYTES); > > is another instance of this define being used in a way that ia64 > didn''t expect (it appears that only ia64 has this space saving > trick in the definition of SMP_CACHE_BYTES ... so it is unsurprising > that general users are not following our usage model). > > How much is this trick saving us? Static size of data area in > vmlinux doesn''t change very much as SMP_CACHE_BYTES is varied: > > text data bss dec hex filename > 8677481 1139704 1206357 11023542 a834b6 vmlinux-8 > 8677417 1141808 1206397 11025622 a83cd6 vmlinux-16 > 8677417 1146000 1206477 11029894 a84d86 vmlinux-32 > 8677353 1146256 1206573 11030182 a84ea6 vmlinux-64 > 8677353 1163152 1207085 11047590 a892a6 vmlinux-128 > > I''m not sure how to evaluate dynamic behavior (allocation of > structures whose size is dependent on SMP_CACHE_BYTES at > runtime). > > -Tony > - > To unsubscribe from this list: send the line "unsubscribe linux-ia64" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >-- Mosberger Consulting LLC, voice/fax: 510-744-9372, http://www.mosberger-consulting.com/ 35706 Runckel Lane, Fremont, CA 94536 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel