Hi Thomas et al.
After spending several hours fiddeling with and improving
the current Makefile for x86_64 I decided to take a closer look
at the x86 merge og i386 and x86_64.
I took a closer look at x86/pci. There are 16 C files.
>From the mails and discussions I expected it be be
obvious what was i386 only, what was shared and what was x86_64 only.
But see following table
Filename i386 x86_64
acpi.c X X
common.c X X
direct.c X X
early.c X X
fixup.c X X
i386.c X X
init.c X X
irq.c X
k8-bus.c X
legacy.c X X
mmconfig_32.c X
mmconfig_64.c X
mmconfig-shared.c X X
numa.c X
pcbios.c X
visws.c X
In the filename there is NOTHING for most of
the non-shared code that tell that this file is
used by only i386 or x86_64.
The exception is mmconfig that is prefixed with _32 versus _64.
But as I have understood the mails floating around using _32,_64
is a way to say here are a potential candidate for futher merging.
In a meged x86 tree it would be very beneficial to either include
in the filename that a specific file is i386 or x86_64 specific or
stuff them in a separate subdirectory.
If legacy.c numa.c, pcibios.c and visws.c placed in a directory named i386
then it would be obvious that this is i386 only.
Or they could be named filename_32 (or the uglier filename_i386).
As it stands out today the filename are kept but thier relationship are lost.
I dunno if this will address the concern of Andi about mixing i386 and x86_64
but to me at least things would be so much more obvious if the original
relationship are spelled out.
Sam
Sam,
On Tue, 2007-09-11 at 22:12 +0200, Sam Ravnborg wrote:
> Hi Thomas et al.
>
> After spending several hours fiddeling with and improving
> the current Makefile for x86_64 I decided to take a closer look
> at the x86 merge og i386 and x86_64.
>
> I took a closer look at x86/pci. There are 16 C files.
>
> From the mails and discussions I expected it be be
> obvious what was i386 only, what was shared and what was x86_64 only.
>
> But see following table
>
> Filename i386 x86_64
> acpi.c X X
> common.c X X
> direct.c X X
> early.c X X
> fixup.c X X
> i386.c X X
> init.c X X
> irq.c X
> k8-bus.c X
> legacy.c X X
> mmconfig_32.c X
> mmconfig_64.c X
> mmconfig-shared.c X X
> numa.c X
> pcbios.c X
> visws.c X
>
> In the filename there is NOTHING for most of
> the non-shared code that tell that this file is
> used by only i386 or x86_64.
True. I tried to unify the Makefile by using
obj-$(CONFIG_X86_32) += ....
and
obj-$(CONFIG_X86_64) += ....
but I did fail due to link order problems in that code. I had not yet
time to go down and figure that out yet, but it is on my todo list.
> The exception is mmconfig that is prefixed with _32 versus _64.
> But as I have understood the mails floating around using _32,_64
> is a way to say here are a potential candidate for futher merging.
Yes, if it is possible and makes sense.
> In a meged x86 tree it would be very beneficial to either include
> in the filename that a specific file is i386 or x86_64 specific or
> stuff them in a separate subdirectory.
>
> If legacy.c numa.c, pcibios.c and visws.c placed in a directory named i386
> then it would be obvious that this is i386 only.
> Or they could be named filename_32 (or the uglier filename_i386).
> As it stands out today the filename are kept but thier relationship are lost.
We concentrated first on the move to make it simple and binary
equivalent. The cleanup of code (merging, location, makefile updates)
are definitely on our todo list.
> I dunno if this will address the concern of Andi about mixing i386 and x86_64
> but to me at least things would be so much more obvious if the original
> relationship are spelled out.
Good point.
tglx
On Tue, Sep 11, 2007 at 10:12:19PM +0200, Sam Ravnborg wrote:
>...
> In a meged x86 tree it would be very beneficial to either include
> in the filename that a specific file is i386 or x86_64 specific or
> stuff them in a separate subdirectory.
>
> If legacy.c numa.c, pcibios.c and visws.c placed in a directory named i386
> then it would be obvious that this is i386 only.
> Or they could be named filename_32 (or the uglier filename_i386).
> As it stands out today the filename are kept but thier relationship are lost.
>...
I'm not agreeing with you on this.
It seems artificial to think 32bit<->64bit was the only interesting
distinction on x86 machines.
You might as well create different directories for i386<->i486+ or
pre-i686<->i686+.
As an example, visws.c is as much non-64bit as it is pre-i686.
> Sam
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
> In the filename there is NOTHING for most of
> the non-shared code that tell that this file is
> used by only i386 or x86_64.
Exactly my point from KS. The big mash-up will not really make much difference
in terms of Makefile clarity or whatever Thomas' point was. Apparently he
wanted to eliminate a few lines of code from the Makefile and merge
the header files completely?
Anyways, the end result will be roughly the same as it is now: i386 and x86-64
are shared in not completely obvious ways and if you change one you have to
test compile the other too.
Same old, same old, as always.
> I dunno if this will address the concern of Andi about mixing i386 and
> x86_64 but to me at least things would be so much more obvious if the
> original relationship are spelled out.
In the end it won't make much difference where the files are located
(although I frankly don't see the advantage of this intrusive move).
You always have to at least compile test both if you change one and I doubt
most people will be able to avoid this no matter how the Makefile looks
or where the files are.
Even if everything was merged together and only ifdefs remained that
fundamental fact would not change either.
A few more files could be also definitely shared, no argument. e.g.
the time subsystem will likely to be shared soon anyways. And probably
a few others. That should be better all done carefully step by step and
properly reviewed though, not in some kind of brutal "rewrite the world"
event.
My concern is mostly that he seems to want to merge some things between 32bit
and 64bit (like the APIC drivers or the crappy i386 maze-of-inlines
subarchitecture design) which ought not be together. I think I managed to
keep x86-64 a relatively clean port over-all, but I see this now going down
the drain :/
-Andi
>
> As an example, visws.c is as much non-64bit as it is pre-i686.
Bad example...
visws is a visws specific file that should naver have allowed
anywhere outside mach-visws.
Any link-order issues should have been dealt with in other ways.
Sam
On Tue, Sep 11, 2007 at 11:05:16PM +0200, Sam Ravnborg wrote:
> >
> > As an example, visws.c is as much non-64bit as it is pre-i686.
> Bad example...
> visws is a visws specific file that should naver have allowed
> anywhere outside mach-visws.
Exactly, it's not about 64bit at all.
Similar, e.g. legacy.c is not about 32bit<->64bit.
> Any link-order issues should have been dealt with in other ways.
>
> Sam
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Tue, Sep 11, 2007 at 09:38:10PM +0100, Andi Kleen wrote:
>...
> In the end it won't make much difference where the files are located
> (although I frankly don't see the advantage of this intrusive move).
>
> You always have to at least compile test both if you change one and I doubt
> most people will be able to avoid this no matter how the Makefile looks
> or where the files are.
>...
The important point is:
People do not expect code under arch/i386/ to be used by code under
arch/x86_64/ and vice versa.
That regularly results in people sending patches that don't compile on
the other architecture.
With one architecture it's much more obvious that the code is shared.
> -Andi
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Tue, 11 Sep 2007, Sam Ravnborg wrote:
>
> From the mails and discussions I expected it be be obvious what was i386
> only, what was shared and what was x86_64 only.
The problem right now is the *reverse* - even though they are in different
subdirectories (and thus *look* like they are all separate), they aren't.
So the merged end result is much better: at a first approximation
everything is shared (largely true), and the ones that aren't shared can
be made more obvious.
> But see following table
>
> Filename i386 x86_64
> acpi.c X X
> common.c X X
> direct.c X X
> early.c X X
> fixup.c X X
> i386.c X X
> init.c X X
> legacy.c X X
> mmconfig-shared.c X X
So sharing is the default, as it should be.
> irq.c X
> k8-bus.c X
> numa.c X
> pcbios.c X
And these are examples of things that likely *should* be shared, and have
nothing what-so-ever to do with 32- vs 64-bit issues. For example, neither
NUMA nor k8 is in *any* way a 32-bit vs 64-bit issue, they are issues with
chips that can be used as either.
> mmconfig_32.c X
> mmconfig_64.c X
And this is obvious, and correct.
> visws.c X
Now, arguably this should be entirely elsewhere (ie in the "mach-visw"
directory).
> In the filename there is NOTHING for most of
> the non-shared code that tell that this file is
> used by only i386 or x86_64.
.. and that's because of historical issues, and has nothing to do with the
merge, and everything to do with all the problems that the merge is
supposed to eventually help us FIX!
> The exception is mmconfig that is prefixed with _32 versus _64.
> But as I have understood the mails floating around using _32,_64
> is a way to say here are a potential candidate for futher merging.
Yes. But so are others.
> If legacy.c numa.c, pcibios.c and visws.c placed in a directory named i386
> then it would be obvious that this is i386 only.
But none of it is "i386 only" and putting it in a directory of its own
would be stupid and wrong. The visws.c thing is platform-specific thing,
and the fact that the platform happens to be 32-bit is totally secondary
to the much bigger issue of the *platform*, so again, it would be totally
wrong to split it up by wordsize.
> I dunno if this will address the concern of Andi about mixing i386 and x86_64
> but to me at least things would be so much more obvious if the original
> relationship are spelled out.
I obviously disagree violently. We should absolutely *not* make any of
this depend on word-size. Quite the reverse!
Linus
On Tue, 11 Sep 2007, Thomas Gleixner wrote:
>
> I tried to unify the Makefile by using
>
> obj-$(CONFIG_X86_32) += ....
> and
> obj-$(CONFIG_X86_64) += ....
Don't do that.
I think it would be much better to instead do something like
obj-y += mmconfig_$(CONFIG_WORD_SIZE).o
to make it clear when we have a file that is conceptually the same, but
has different implementations.
That also makes the unification (assuming/hoping it gets done) of such
files much cleaner - you just merge them, and the obj-y line can just drop
the $(CONFIG_WORD_SIZE) thing. Very logical.
> but I did fail due to link order problems in that code.
.. the above approach also gets rid of any link order problems.
Linus
>
> People do not expect code under arch/i386/ to be used by code under
> arch/x86_64/ and vice versa.
>
> That regularly results in people sending patches that don't compile on
> the other architecture.
>
> With one architecture it's much more obvious that the code is shared.
Will that cause people to compile test both? I have my doubts that
will really work.
e.g. a similar example would be CONFIG_MMU=n. The code
is mostly shared and in the same directories, but people still
break the MMUless architectures all the time.
I don't expect this to be different with 32bit/64bit.
-Andi
On Tue, Sep 11, 2007 at 10:34:23PM +0100, Andi Kleen wrote:
>
> >
> > People do not expect code under arch/i386/ to be used by code under
> > arch/x86_64/ and vice versa.
> >
> > That regularly results in people sending patches that don't compile on
> > the other architecture.
> >
> > With one architecture it's much more obvious that the code is shared.
>
> Will that cause people to compile test both? I have my doubts that
> will really work.
>...
You will see that it could be shared, and it'll be much easier to see
all configurations it's used in.
Currently, there are 6 or 7 different ways how a function under
arch/i386/ could be used by a function under arch/x86_64/ (and vice
versa) and it's non-trivial to figure out all usages when grep'ing for
users.
> -Andi
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Tue, 11 Sep 2007, Andi Kleen wrote:
>
> Will that cause people to compile test both? I have my doubts that
> will really work.
If people don't compile-test both now, then why would they compile-test
things when merged?
So no, that's not the point.
But at least things like "grep" will work sanely, and people will be
*aware* that "Oh, this touches a file that may be used by the other
word-size".
Right now, we have people changing "i386-only" files that turn out to be
used by x86-64 too - through very subtle Makefile things that the person
who only looks into the i386 Makefile will never even *see*.
THAT is the problem (well, at least part of it).
Linus
On Tue, Sep 11, 2007 at 10:34:23PM +0100, Andi Kleen wrote:
> > People do not expect code under arch/i386/ to be used by code under
> > arch/x86_64/ and vice versa.
> >
> > That regularly results in people sending patches that don't compile on
> > the other architecture.
> >
> > With one architecture it's much more obvious that the code is shared.
>
> Will that cause people to compile test both? I have my doubts that
> will really work.
>
> e.g. a similar example would be CONFIG_MMU=n. The code
> is mostly shared and in the same directories, but people still
> break the MMUless architectures all the time.
>
As I was the first one to do CONFIG_MMU=y/n in the same arch directory,
since 2.5, I can tell you that that's simply crap. The only reason
CONFIG_MMU=n gets broken all the time is because people don't think about
it in generic code, it's rarely broken in the architecture code, and even
with the most occasional of build tests most of that gets caught in a
hurry.
You do of course have to consider both cases when writing new code, but
those things tend to be pretty obvious. It's a bit more work for the arch
maintainer, but it's certainly far less confusing and problematic than
separating things out.
In fact, going the opposite route is what leads to endless trouble in the
long run, since you brought up the MMUless example, m68knommu is a good
example. Rather than beginning life in arch/m68k, it was forked off,
mostly to deal with the ColdFire CPUs that weren't planned to have MMUs.
Now that the product line has moved along, adding an MMU to it is in the
roadmap, which means that inevitably they're both going to have to be
combined anyways. Simply dealing with the initial trouble of having them
combined initially would have solved a lot of that mess.
You can ignore the added maintenance for as long as possible, but sooner
or later it's going to be a problem. Procrastination is not something
that bodes particularly well for divergent hardware support.
On Tue, Sep 11, 2007 at 10:34:13PM +0200, Adrian Bunk wrote:
> As an example, visws.c is as much non-64bit as it is pre-i686.
Bullshit. visws were shipped with P3s.
On Wed, Sep 12, 2007 at 10:27:16AM +0100, Christoph Hellwig wrote:
> On Tue, Sep 11, 2007 at 10:34:13PM +0200, Adrian Bunk wrote:
> > As an example, visws.c is as much non-64bit as it is pre-i686.
>
> Bullshit. visws were shipped with P3s.
Certainly true, but still not 64bit and never will be.
--
Len Sorensen
On Sep 11 2007 14:51, Linus Torvalds wrote:
>On Tue, 11 Sep 2007, Andi Kleen wrote:
>>
>> Will that cause people to compile test both? I have my doubts that
>> will really work.
>
>If people don't compile-test both now, then why would they compile-test
>things when merged?
>
>So no, that's not the point.
>
>But at least things like "grep" will work sanely, and people will be
>*aware* that "Oh, this touches a file that may be used by the other
>word-size".
Speaking of which, how does sparc/sparc64 handle things?
Jan
--
>
> > If legacy.c numa.c, pcibios.c and visws.c placed in a directory named i386
> > then it would be obvious that this is i386 only.
>
> But none of it is "i386 only" and putting it in a directory of its own
> would be stupid and wrong. The visws.c thing is platform-specific thing,
> and the fact that the platform happens to be 32-bit is totally secondary
> to the much bigger issue of the *platform*, so again, it would be totally
> wrong to split it up by wordsize.
In other words - of all directories I used the worst one to prove my point.
I had no idea that legacy.c, numa.c and pcbios.c was candidates for x86_64 usage.
The point I try to make and which seems to have been lost in platform/wordsize
inputs are that there is a reason for being able to see which of the two
architectures a given file belong to.
Try to grep for csum_partial in x86/lib and you will get this:
checksum.S: * Changes: Ingo Molnar, converted csum_partial_copy() to 2.1 exception
checksum.S:unsigned int csum_partial(const unsigned char * buff, int len, unsigned int sum)
checksum.S:ENTRY(csum_partial)
checksum.S:ENDPROC(csum_partial)
checksum.S:ENTRY(csum_partial)
checksum.S:ENDPROC(csum_partial)
checksum.S:unsigned int csum_partial_copy_generic (const char *src, char *dst,
checksum.S: * Copy from ds while checksumming, otherwise like csum_partial
checksum.S:ENTRY(csum_partial_copy_generic)
checksum.S:ENDPROC(csum_partial_copy_generic)
checksum.S:ENTRY(csum_partial_copy_generic)
checksum.S:ENDPROC(csum_partial_copy_generic)
csum-copy.S:ENTRY(csum_partial_copy_generic)
csum-copy.S:ENDPROC(csum_partial_copy_generic)
csum-partial.c:__wsum csum_partial(const void *buff, int len, __wsum sum)
csum-partial.c:EXPORT_SYMBOL(csum_partial);
csum-partial.c: return csum_fold(csum_partial(buff,len,0));
csum-wrappers.c: * csum_partial_copy_from_user - Copy and checksum from user space.
csum-wrappers.c:csum_partial_copy_from_user(const void __user *src, void *dst,
csum-wrappers.c: isum = csum_partial_copy_generic((__force const void *)src,
csum-wrappers.c:EXPORT_SYMBOL(csum_partial_copy_from_user);
csum-wrappers.c: * csum_partial_copy_to_user - Copy and checksum to user space.
csum-wrappers.c:csum_partial_copy_to_user(const void *src, void __user *dst,
csum-wrappers.c: return csum_partial_copy_generic(src, (void __force *)dst,len,isum,NULL,errp);
csum-wrappers.c:EXPORT_SYMBOL(csum_partial_copy_to_user);
csum-wrappers.c: * csum_partial_copy_nocheck - Copy and checksum.
csum-wrappers.c:csum_partial_copy_nocheck(const void *src, void *dst, int len, __wsum sum)
csum-wrappers.c: return csum_partial_copy_generic(src,dst,len,sum,NULL,NULL);
csum-wrappers.c:EXPORT_SYMBOL(csum_partial_copy_nocheck);
OK - maybe this is obvious for you and a few others. But for me I get utterly confused
about where to look for the x86_64 version.
Diving into the Makefile I can figure it out.
But thats one indirection too much.
As an example where this plays out better are in x86/crypto.
When grepping for aes_dec_blk I got following output:
aes_32.c:asmlinkage void aes_dec_blk(struct crypto_tfm *tfm, u8 *dst, const u8 *src);
aes_32.c: aes_dec_blk(tfm, dst, src);
aes_64.c:asmlinkage void aes_dec_blk(struct crypto_tfm *tfm, u8 *out, const u8 *in);
aes_64.c: aes_dec_blk(tfm, dst, src);
aes-i586-asm.S:/* void aes_dec_blk(struct crypto_tfm *tfm, u8 *out_blk, const u8 *in_blk) */
aes-i586-asm.S:.global aes_dec_blk
aes-i586-asm.S:aes_dec_blk:
aes-x86_64-asm.S:/* void aes_dec_blk(struct crypto_tfm *tfm, u8 *out, const u8 *in) */
aes-x86_64-asm.S: entry(aes_dec_blk,240,dec128,dec192)
See how obvious it is what are x86_64 specific and what are i386 specific.
And that despite the mixed naming convention used.
All files that _truely_ belongs to only one of the two architectures ought to
be named such that this is obvious when grepping like the above examples shows.
Using the wordsize to distingush the filename seems to cause confusion since there
are files that _truely_ only belongs to i386 but is not 32 bit specific because
they exist only on i386 because they are not needed on any x86_64 system.
Sam
On Tue, 11 Sep 2007 23:14:22 +0200 Adrian Bunk <[email protected]> wrote:
> People do not expect code under arch/i386/ to be used by code under
> arch/x86_64/ and vice versa.
[OT: it drives me batshit that we ended up including stuff in both directions]
On Wed, 12 Sep 2007 09:29:53 +0900 Paul Mundt <[email protected]> wrote:
> As I was the first one to do CONFIG_MMU=y/n in the same arch directory,
> since 2.5, I can tell you that that's simply crap. The only reason
> CONFIG_MMU=n gets broken all the time is because people don't think about
> it in generic code, it's rarely broken in the architecture code, and even
> with the most occasional of build tests most of that gets caught in a
> hurry.
oy. I do sh allmodconfig test builds fairly regularly.
<does one>
It all went OK, up until moddep:
ERROR: "_ebss" [drivers/mtd/maps/uclinux.ko] undefined!
On Sat, Sep 15, 2007 at 02:32:58AM -0700, Andrew Morton wrote:
> On Tue, 11 Sep 2007 23:14:22 +0200 Adrian Bunk <[email protected]> wrote:
>
> > People do not expect code under arch/i386/ to be used by code under
> > arch/x86_64/ and vice versa.
>
> [OT: it drives me batshit that we ended up including stuff in both directions]
Why?
Anyways, i wouldn't have a problem with putting the already shared
files into a different directory or move it over to one of the architectures,
although I must admit I personally wouldn't see a big benefit from it. But if
it gives people a warm fuzzy feeling I'm all for it.
-Andi
On Sat, 15 Sep 2007 20:36:23 +0200 Andi Kleen <[email protected]> wrote:
> On Sat, Sep 15, 2007 at 02:32:58AM -0700, Andrew Morton wrote:
> > On Tue, 11 Sep 2007 23:14:22 +0200 Adrian Bunk <[email protected]> wrote:
> >
> > > People do not expect code under arch/i386/ to be used by code under
> > > arch/x86_64/ and vice versa.
> >
> > [OT: it drives me batshit that we ended up including stuff in both directions]
>
> Why?
It's more complex, obviously. More surprising. It used to be the case that
arch/x86^4 files were xx86_64 and arch/i386 files were i386 and possibly
x86_64. Now it's the case that arch/x86_64 files are x86_64 and maybe i386
and arch/i386 files are i386 and maybe x86_64. Additional and quite
unnecessary complexity.
I mean, how often do x86_64 changes in your tree break i386? Once every
3ish weeks would be my guess. Often this will be because the person making
(and reviewing) the x86_64 change didn't know (or forgot) that the file is
also used by x86_64.
> Anyways, i wouldn't have a problem with putting the already shared
> files into a different directory or move it over to one of the architectures,
> although I must admit I personally wouldn't see a big benefit from it. But if
> it gives people a warm fuzzy feeling I'm all for it.
Doing something like that would reduce complexity, reduce surprise and
increase maintainability. That's more than warm-and-fuzzies.