From patchwork Thu Aug 22 11:32:38 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Wang Haitao X-Patchwork-Id: 269021 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 4099A2C00BD for ; Thu, 22 Aug 2013 21:34:09 +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 1VCT9I-0000eS-RZ; Thu, 22 Aug 2013 11:33:57 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1VCT9H-0005L6-4T; Thu, 22 Aug 2013 11:33:55 +0000 Received: from out1.zte.com.cn ([202.103.147.172] helo=zte.com.cn) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1VCT9C-0005Je-P8 for linux-mtd@lists.infradead.org; Thu, 22 Aug 2013 11:33:53 +0000 Received: from mse01.zte.com.cn (unknown [10.30.3.20]) by Websense Email Security Gateway with ESMTPS id 17F4ECD867B; Thu, 22 Aug 2013 19:32:43 +0800 (CST) Received: from notes_smtp.zte.com.cn ([10.30.1.239]) by mse01.zte.com.cn with ESMTP id r7MBWncE021286; Thu, 22 Aug 2013 19:32:49 +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: F5F543EF:B1AF55C4-48257BCF:003EF309; type=4; name=$KeepSent To: Brian Norris X-Mailer: Lotus Notes Release 6.5.6 March 06, 2007 Message-ID: From: "Wang Haitao" Date: Thu, 22 Aug 2013 19:32:38 +0800 X-MIMETrack: Serialize by Router on notes_smtp/zte_ltd(Release 8.5.3FP1 HF212|May 23, 2012) at 2013-08-22 19:32:31 MIME-Version: 1.0 X-MAIL: mse01.zte.com.cn r7MBWncE021286 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130822_073351_737831_546B3175 X-CRM114-Status: UNSURE ( 7.92 ) X-CRM114-Notice: Please train this message. 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.172 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 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 Brian Norris 2013-08-21 16:57 To Wang Haitao 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 > 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