Re: [PATCH] x86: fix apic.h unused but set warnings v2

From: Cyrill Gorcunov
Date: Mon Nov 08 2010 - 17:15:35 EST


On Mon, Nov 08, 2010 at 10:57:30PM +0100, Andi Kleen wrote:
> Thomas Gleixner <tglx@xxxxxxxxxxxxx> writes:
>
> > On Mon, 8 Nov 2010, Andi Kleen wrote:
> >> static inline u32 native_apic_msr_read(u32 reg)
> >> {
> >> - u32 low, high;
> >> + u32 low;
> >>
> >> if (reg == APIC_DFR)
> >> return -1;
> >>
> >> - rdmsr(APIC_BASE_MSR + (reg >> 4), low, high);
> >> - return low;
> >> + rdmsrl(APIC_BASE_MSR + (reg >> 4), low);
> >> + return (u32)low;
> >
> > What's the point of casting u32 to u32 ?
>
> One of the earlier reviewers thought adding an explicit cast would make
> the truncation in the code clearer. I didn't full agree either, but
> still did the change.
>
> -Andi

Well, Andi I proposed to define variable as u64 and convert it back to u32 at procedure
exit point. That would be clean, and I still think so ;) Though I'm fine with
either way (I just thought about ones who will be reading this code in future,
and since most the rdmsrl callers already use u64 and unsigned long, this would
be consistent as well).

Cyrill
--
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/