* Re: [PATCH]mtd: map: fixed bug in 64-bit systems
2013-08-21 8:57 ` [PATCH]mtd: map: fixed bug in 64-bit systems Brian Norris
@ 2013-08-22 11:32 ` Wang Haitao
2013-08-23 2:24 ` Wang Haitao
1 sibling, 0 replies; 5+ messages in thread
From: Wang Haitao @ 2013-08-22 11:32 UTC (permalink / raw)
To: Brian Norris; +Cc: artem.bityutskiy, dwmw2, linux-mtd
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>
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;
Brian Norris <computersforpeace@gmail.com>
2013-08-21 16:57 To
Wang Haitao <wang.haitao1@zte.com.cn>
cc
linux-mtd@lists.infradead.org, artem.bityutskiy@linux.intel.com, dwmw2@infradead.org
Subject
Re: [PATCH]mtd: map: fixed bug in 64-bit systems
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH]mtd: map: fixed bug in 64-bit systems
2013-08-21 8:57 ` [PATCH]mtd: map: fixed bug in 64-bit systems Brian Norris
2013-08-22 11:32 ` Wang Haitao
@ 2013-08-23 2:24 ` Wang Haitao
2013-10-23 1:43 ` Brian Norris
1 sibling, 1 reply; 5+ messages in thread
From: Wang Haitao @ 2013-08-23 2:24 UTC (permalink / raw)
To: Brian Norris; +Cc: artem.bityutskiy, dwmw2, linux-mtd
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>
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;
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
^ permalink raw reply related [flat|nested] 5+ messages in thread