Re: [GIT PULL] x86/asm for 2.6.36

From: H. Peter Anvin
Date: Fri Aug 06 2010 - 13:48:51 EST


On 08/06/2010 10:17 AM, Linus Torvalds wrote:
> On Thu, Aug 5, 2010 at 1:37 PM, H. Peter Anvin <hpa@xxxxxxxxxxxxxxx> 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.

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