On Tue, Nov 20, 2012 at 07:28:56AM +0300, Dan Carpenter wrote: > On Mon, Nov 19, 2012 at 06:48:34PM -0600, Russ Anderson wrote: > > On Sat, Nov 17, 2012 at 06:16:11PM +0300, Dan Carpenter wrote: > > > This is a static checker fix. The problem is that we store the bits > > > from "uv_apicid_hibits" into "apicid" (the high 16 bits) but then we > > > shift it 16 bit to the left. "apicid" is an int so it wraps and we lose > > > them. > > > > Is this the complete patch? phys_apicid is an int, but gets > > cast as unsigned long. Doesn't phys_apicid also have to be > > changed to unsigned long? And why ulong instead of uint (on x86_64)? > > Uint is 32bit across all arches in linux and unix, according to > wikipedia. But long isn't 32bit across all arches. http://software.intel.com/en-us/articles/size-of-long-integer-type-on-different-architecture-and-os > The wakeup_secondary_cpu() function pointer takes an int > so I couldn't change the parameter. Yes. The real problem is much of the apicid code is based on signed int (ie parameters in struct apic). Not sure why they chose to make it signed, but since they did that decision ripples through the rest of the code. Changing it to unsigned means changing struct apic, which likewise will ripple through the rest of the code. That is a much bigger change than your patch deals with. > > I agree with changing signed to unsigned where appropriate, but > > this looks like a partial fix. Am I missing something? > > > > I do feel a little embarrassed that I didn't use "unsigned long" > consistently. I just used ulong to make the line a bit shorter, but > I could redo it with "unsigned long" if you want. The issue isn't "ulong" vs "unsigned long". The issue is int is 32 bit and long is 64 bit on x86_64. Your patch is casting the value as an "unsigned long" (64 bit on x86_64) into an int (32 bit). I don't think that was your intent. This does highlight the problematic nature of "long" being different size on different architectures. -- Russ Anderson, OS RAS/Partitioning Project Lead SGI - Silicon Graphics Inc rja@xxxxxxx -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
