From patchwork Fri Aug 23 02:24:07 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Wang Haitao X-Patchwork-Id: 269262 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from casper.infradead.org (unknown [IPv6:2001:770:15f::2]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 6907C2C0082 for ; Fri, 23 Aug 2013 12:25:33 +1000 (EST) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VCh3x-0005uY-5p; Fri, 23 Aug 2013 02:25:21 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1VCh3v-0007Fr-DH; Fri, 23 Aug 2013 02:25:19 +0000 Received: from mx7.zte.com.cn ([202.103.147.169] helo=zte.com.cn) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1VCh3m-0007Er-0E for linux-mtd@lists.infradead.org; Fri, 23 Aug 2013 02:25:15 +0000 Received: from mse01.zte.com.cn (unknown [10.30.3.20]) by Websense Email Security Gateway with ESMTPS id 43D8A73C736; Fri, 23 Aug 2013 10:24:16 +0800 (CST) Received: from notes_smtp.zte.com.cn ([10.30.1.239]) by mse01.zte.com.cn with ESMTP id r7N2OIAO017379; Fri, 23 Aug 2013 10:24:18 +0800 (GMT-8) (envelope-from wang.haitao1@zte.com.cn) In-Reply-To: <20130821085758.GG31788@brian-ubuntu> Subject: Re: [PATCH]mtd: map: fixed bug in 64-bit systems X-KeepSent: ED58E4A6:DD72F731-48257BD0:000C7F27; type=4; name=$KeepSent To: Brian Norris X-Mailer: Lotus Notes Release 6.5.6 March 06, 2007 Message-ID: From: "Wang Haitao" Date: Fri, 23 Aug 2013 10:24:07 +0800 X-MIMETrack: Serialize by Router on notes_smtp/zte_ltd(Release 8.5.3FP1 HF212|May 23, 2012) at 2013-08-23 10:23:58 MIME-Version: 1.0 X-MAIL: mse01.zte.com.cn r7N2OIAO017379 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130822_222510_555439_09B5DEEF X-CRM114-Status: GOOD ( 20.26 ) X-Spam-Score: -1.6 (-) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-1.6 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [202.103.147.169 listed in list.dnswl.org] -2.8 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] 0.0 MIME_BASE64_BLANKS RAW: Extra blank lines in base64 encoding 1.8 FROM_MISSPACED From: missing whitespace 2.0 FROM_MISSP_EH_MATCH From misspaced, matches envelope Cc: artem.bityutskiy@linux.intel.com, dwmw2@infradead.org, linux-mtd@lists.infradead.org X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-mtd" Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org 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 Signed-off-by: Zhang Yi Signed-off-by:Lu Zhongjun Reviewed-by: Jiang Biao Tested-by: Ma Chenggong Brian Norris 写于 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 > > Signed-off-by: Zhang Yi > > Signed-off-by:Lu Zhongjun > > Reviewed-by: Jiang Biao > > Tested-by: Ma Chenggong > > [...] > > 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