| Message ID | OFED58E4A6.DD72F731-ON48257BD0.000C7F27-48257BD0.000D8AAF@zte.com.cn |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Aug 23, 2013 at 10:24:07AM +0800, Wang Haitao wrote: > Thanks for your advice. > > Hardware： > CPU：XLP832,the 64-bit OS > NOR Flash:S29GL128S 128M > Software: > Kernel:2.6.32.41 > Filesystem:JFFS2 > > When writing files, errors appear: > Write len 182 but return retlen 180 > Write of 182 bytes at 0x072c815c failed. returned -5, retlen 180 > Write len 186 but return retlen 184 > Write of 186 bytes at 0x072caff4 failed. returned -5, retlen 184 > These errors exist only in 64-bit systems,not in 32-bit systems. After > analysis, we found that the left shift operation is wrong in > map_word_load_partial. For instance： > unsigned char buf[3] ={0x9e,0x3a,0xea}; > map_bankwidth(map) is 4; > > for (i=0; i < 3; i++) { > int bitpos; > bitpos = (map_bankwidth(map)-1-i)*8; > orig.x[0] &= ~(0xff << bitpos); > orig.x[0] |= buf[i] << bitpos; > } > > The value of orig.x[0] is expected to be 0x9e3aeaff, but in this > situation(64-bit System) we'll get the wrong value of 0xffffffff9e3aeaff > due to the 64-bit sign extension: > buf[i] is defined as "unsigned char" and the left-shift operation will > convert it to the type of "signed int", so when left-shift buf[i] by 24 > bits, the final result will get the wrong value: 0xffffffff9e3aeaff. > > If the left-shift bits are less than 24, then sign extension will not > occur. Whereas the bankwidth of the nor flash we used is 4, therefore this > BUG emerges. > > Signed-off-by:Pang Xunlei <pang.xunlei@zte.com.cn> > Signed-off-by: Zhang Yi <zhang.yi20@zte.com.cn> > Signed-off-by:Lu Zhongjun <lu.zhongjun@zte.com.cn> > Reviewed-by: Jiang Biao <jiang.biao2@zte.com.cn> > Tested-by: Ma Chenggong <ma.chenggong@zte.com.cn> Well, this still wasn't formatted correctly, but it is a good fix. I fixed it up, added the Cc: <stable@vger.kernel.org> and applied it to l2-mtd.git. Thanks! Brian
diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h index aa30244..18ee717 --- a/include/linux/mtd/map.h +++ b/include/linux/mtd/map.h @@ -342,7 +342,7 @@ static inline map_word map_word_load_partial(struct map_info *map, map_word orig bitpos = (map_bankwidth(map)-1-i)*8; #endif orig.x[0] &= ~(0xff << bitpos); - orig.x[0] |= buf[i-start] << bitpos; + orig.x[0] |= (unsigned long)buf[i-start] << bitpos; } } return orig; @@ -361,7 +361,7 @@ static inline map_word map_word_ff(struct map_info *map) if (map_bankwidth(map) < MAP_FF_LIMIT) { int bw = 8 * map_bankwidth(map); - r.x[0] = (1 << bw) - 1; + r.x[0] = (1UL << bw) - 1; } else { for (i=0; i<map_words(map); i++) r.x[i] = ~0UL;
Thanks for your advice. Hardware： CPU：XLP832,the 64-bit OS NOR Flash:S29GL128S 128M Software: Kernel:2.6.32.41 Filesystem:JFFS2 When writing files, errors appear: Write len 182 but return retlen 180 Write of 182 bytes at 0x072c815c failed. returned -5, retlen 180 Write len 186 but return retlen 184 Write of 186 bytes at 0x072caff4 failed. returned -5, retlen 184 These errors exist only in 64-bit systems,not in 32-bit systems. After analysis, we found that the left shift operation is wrong in map_word_load_partial. For instance： unsigned char buf[3] ={0x9e,0x3a,0xea}; map_bankwidth(map) is 4; for (i=0; i < 3; i++) { int bitpos; bitpos = (map_bankwidth(map)-1-i)*8; orig.x[0] &= ~(0xff << bitpos); orig.x[0] |= buf[i] << bitpos; } The value of orig.x[0] is expected to be 0x9e3aeaff, but in this situation(64-bit System) we'll get the wrong value of 0xffffffff9e3aeaff due to the 64-bit sign extension: buf[i] is defined as "unsigned char" and the left-shift operation will convert it to the type of "signed int", so when left-shift buf[i] by 24 bits, the final result will get the wrong value: 0xffffffff9e3aeaff. If the left-shift bits are less than 24, then sign extension will not occur. Whereas the bankwidth of the nor flash we used is 4, therefore this BUG emerges. Signed-off-by:Pang Xunlei <pang.xunlei@zte.com.cn> Signed-off-by: Zhang Yi <zhang.yi20@zte.com.cn> Signed-off-by:Lu Zhongjun <lu.zhongjun@zte.com.cn> Reviewed-by: Jiang Biao <jiang.biao2@zte.com.cn> Tested-by: Ma Chenggong <ma.chenggong@zte.com.cn> Brian Norris <computersforpeace@gmail.com> 写于 2013-08-21 16:57:58: > Hi Wang, > > Can you please resend this patch with a few fixups? > > The description below is good, but please wrap it to ~70 characters. It > is hard to read. > > Also, you need to test your patch with scripts/checkpatch.pl both before > and after you email it -- all your whitespace is mangled (i.e., you use > spaces where there should be tabs). > > See the kernel documentation: > > Documentation/SubmittingPatches > Documentation/email-clients.txt > > On Wed, Aug 07, 2013 at 03:34:20PM +0800, Wang Haitao wrote: > > Hardware： > > CPU：XLP832,the 64-bit OS > > NOR Flash:S29GL128S 128M > > Software: > > Kernel:2.6.32.41 > > Filesystem:JFFS2 > > > > When writing files, errors appear: > > Write len 182 but return retlen 180 > > Write of 182 bytes at 0x072c815c failed. returned -5, retlen 180 > > Write len 186 but return retlen 184 > > Write of 186 bytes at 0x072caff4 failed. returned -5, retlen 184 > > These errors exist only in 64-bit systems,not in 32-bit systems. > After analysis, we found that the left shift operation is wrong in > map_word_load_partial. For instance： > > unsigned char buf[3] ={0x9e,0x3a,0xea}; > > map_bankwidth(map) is 4; > > > > for (i=0; i < 3; i++) { > > int bitpos; > > bitpos = (map_bankwidth(map)-1-i)*8; > > orig.x[0] &= ~(0xff << bitpos); > > orig.x[0] |= buf[i] << bitpos; > > } > > > > The value of orig.x[0] is expected to be 0x9e3aeaff, but in this > situation(64-bit System) we'll get the wrong value of > 0xffffffff9e3aeaff due to the 64-bit sign extension: > > buf[i] is defined as "unsigned char" and the left-shift operation > will convert it to the type of "signed int", so when left-shift > buf[i] by 24 bits, the final result will get the wrong value: > 0xffffffff9e3aeaff. > > > > If the left-shift bits are less than 24, then sign extension will > not occur. Whereas the bankwidth of the nor flash we used is 4, > therefore this BUG emerges. > > > > Signed-off-by:Pang Xunlei <pang.xunlei@zte.com.cn> > > Signed-off-by: Zhang Yi <zhang.yi20@zte.com.cn> > > Signed-off-by:Lu Zhongjun <lu.zhongjun@zte.com.cn> > > Reviewed-by: Jiang Biao <jiang.biao2@zte.com.cn> > > Tested-by: Ma Chenggong <ma.chenggong@zte.com.cn> > > [...] > > Thanks, > Brian