On 04/26/12 17:18, Doug Ledford wrote: > On 04/26/2012 11:12 AM, Jes.Sorensen@xxxxxxxxxx wrote: >> > From: Jes Sorensen <Jes.Sorensen@xxxxxxxxxx> >> > >> > fbdef49811c9e2b54e2064d9af68cfffa77c6e77 incorrectly tried to fix sign >> > extension of the bitmap offset. However mdinfo->bitmap_offset is a u32 >> > and needs to be converted to a 32 bit signed integer before the sign >> > extension. >> > >> > Signed-off-by: Jes Sorensen <Jes.Sorensen@xxxxxxxxxx> > I was scratching my head over this patch, saying to myself "But won't > that cause us to truncate large values of bitmap_offset?" And it will, > but I see your point now, that's *exactly* the problem if we don't do > the sign conversion before the extension, the actual bitmap_offset > should really be signed in order to support negative offsets, but since > it isn't, when we save a negative offset into bitmap_offset it appears > as a really large positive offset, and then when we sign extend to long, > it keeps the large size positive offset instead of picking up the > negative offset. Gotcha. So, I see why this works, but do you think it > should be fixed this way, or by converting bitmap_offset to type int32 > instead of uint32? Heh, I have to admit I cheated too and asked Richard Henderson for help as I couldn't figure out why the sign conversion failed. Otherwise I would probably still have been scratching my head over it :) I noticed other parts of the code already handled it this way, so my fix is consistent with that, but we could do both. Neil? Cheers, Jes -- To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html