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
-

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 


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 


Now, arguably this should be entirely elsewhere (ie in the "mach-visw" 

.. 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 


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 

I obviously disagree violently. We should absolutely *not* make any of 
this depend on word-size. Quite the reverse!

		Linus
-

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 a...
[ message continues ]

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.


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
-

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.


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

-

[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
-

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

Doing something like that would reduce complexity, reduce surprise and
increase maintainability.  That's more than warm-and-fuzzies.
-

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
-

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.
-

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!

-

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
-

Speaking of which, how does sparc/sparc64 handle things?




	Jan
-- 
-

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 

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

-

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+.


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

-

Bullshit.  visws were shipped with P3s.

-

Certainly true, but still not 64bit and never will be.

--
Len Sorensen
-

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
-

Exactly, it's not about 64bit at all.


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

-

Sam,


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


We concentrated first on the move to make it simple and binary
equivalent. The cleanup of code (merging, location, makefile updates)

Good point.

	tglx


-

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 above approach also gets rid of any link order problems.

			Linus
-