Message-ID: <20080630185311.GB27398@elte.hu>
Date:	Mon, 30 Jun 2008 20:53:11 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Glauber Costa <gcosta@...hat.com>
Cc:	linux-kernel@...r.kernel.org, tglx@...utronix.de, x86@...nel.org,
	"H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH 25/39] merge common parts of uaccess.


* Glauber Costa <gcosta@...hat.com> wrote:

> Ingo Molnar wrote:
>> * Glauber Costa <gcosta@...hat.com> wrote:
>>
>>> common parts of uaccess_32.h and uaccess_64.h
>>> are put in uaccess.h.
>>
>> -tip testing found that it causes this build failure:
>>
>>   fs/binfmt_aout.c: Assembler messages:
>>   fs/binfmt_aout.c:152: Error: suffix or operands invalid for `cmp'
>>
>> with:
>>
>>   http://redhat.com/~mingo/misc/config-Mon_Jun_30_08_17_42_CEST_2008.bad
>>
>> and comparing the 32-bit and unified version is not simple and the  
>> commit is rather large.
>>
>> I'm sure the fix is simple, but this bug shows a structural problem 
>> with this unification patch. The proper way to unify files is to first 
>> bring both the 32-bit and the 64-bit version up to a unified form via  
>> finegrained changes, so that uaccess_32.h and uaccess_64.h becomes  
>> exactly the same file.
>>
>> ... _then_ only, in a final 'mechanic unification' step the two files  
>> are merged into uaccess.h. (but no change is done to the content)
>>
>> If anything breaks during such a series it's bisectable to a 
>> finegrained patch on either the 32-bit or the 64-bit side. If this 
>> commit was shaped that way i could now report to you the exact 
>> bisection result - instead of this too-broad bisection result.
>>
>> So please rework this commit in that fashion (not just to fix this  
>> breakage but in anticipation of future commits) - uaccess.h is central  
>> enough for us to be super careful about it.
>>
>> 	Ingo
> Fair.
>
> However, as I wrote in the first patch of the series, I'm not doing a  
> complete unification of uaccess.h. Part of it is left for future work,  
> since it's a little bit trickier.
>
> So I didn't have the option of a mechanical move. I did tried, however,  
> to make sure this patch was only a code move, with everything that is  
> going to the common file being equal in both files.
>
> Needless to say, I failed. ;-) This was for a very tiny piece, but still...
>
> The options I see are:
>
> * to redo the uaccess.h unification this way, making sure a diff 
> between the diffs of the arch-files report nothing different, or: * to 
> remove the topmost patches that touches uaccess*.h, and leave only the 
> ones that integrate the .c and .S files, until I can really integrate 
> the whole of it.
>
> For the second, however, although I was careful to make incremental 
> changes, some small differences may exist. Examples of these 
> differences are places in which I introduce a few ifdefs. It's close 
> to nothing, but still not mechanical. Because of that, you might want 
> me to redo the whole series.
>
> Your call.

well the primary worry is the build failure with gcc 4.3.1 that i've 
posted. If that's simple to fix we could re-try with your existing 
series.

But to be defensive it's alway useful to move one component at a time. 
Even if you dont end up doing a mechanical unification - the stuff you 
move you should be able to claim to be exactly identical. I.e. the final 
step can be mechanic in that it unifies exactly the same content (even 
though both files still have remaining bits).

Then we'll end up with nice bisection reports to the specific area that 
is impacted by a problem.

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists