Re: [GIT PULL] x86/asm for 2.6.36

["H. Peter Anvin" <hpa@zytor.com>]

On 08/06/2010 10:17 AM, Linus Torvalds wrote:
> On Thu, Aug 5, 2010 at 1:37 PM, H. Peter Anvin <hpa@linux.intel.com> wrote:
>>
>>  git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git x86-asm-for-linus
>>
>> H. Peter Anvin (7):
>>      x86, asm: Clean up and simplify set_64bit()
> 
> Hmm. This seems to cause compiler warnings for me on x86-64:
> 
>   drivers/pci/intr_remapping.c: In function ‘modify_irte’:
>   drivers/pci/intr_remapping.c:314: warning: passing argument 1 of
> ‘set_64bit’ from incompatible pointer type
>   /home/torvalds/v2.6/linux/arch/x86/include/asm/cmpxchg_64.h:6: note:
> expected ‘volatile u64 *’ but argument is of type ‘long unsigned int
> *’
> 
> and I'm not clear on the reason for that function prototype change.
> 
>                           Linus

On 64 bits the intent was to be consistent with the tightened-up 32-bit
code.  Perhaps that was a mistake, but it seems a lot more consistent to
me to have "u64" be the type

On the 32-bit side, it's not so much a function prototype change as the
previous version of the code actually explicitly cast it to an array of
unsigned ints... it has the advantage that it can accept any type, but
the disadvantage that is can accept *ANY* type.  The problem of course
is the above, since gcc will issue warnings for u64 (unsigned long long)
version the otherwise-identical "unsigned long".

It's worth noting that in this particular case the code itself looks
like this:

	set_64bit((unsigned long *)&irte->low, irte_modified->low);
	set_64bit((unsigned long *)&irte->high, irte_modified->high);

... where the existing cast is there because irte->low and irte->high
are types __u64.  In other words, with the "more logical" u64 prototype
the casts should just get removed.

On the other hand I should have seen this sooner, obviously.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.