From patchwork Sat Jul 31 02:32:42 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zhihao Cheng X-Patchwork-Id: 1511852 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.infradead.org (client-ip=2607:7c80:54:e::133; helo=bombadil.infradead.org; envelope-from=linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=aMAhrZA1; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:e::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Gc7NQ0Tt5z9sT6 for ; Sat, 31 Jul 2021 12:23:18 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:CC:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=bpDWxdJf4NTmIRMVGInxh8ZwZ/Xxt90t99rI+7tCUq0=; b=aMAhrZA13oNDqJ kTH2I9wQHGsTUpB2Z9QxqOGt7IMcJ9oL6DmbC0jo2MNkXMyJ9pPHhvc6cJBfFLwRkqZREHCgJJ3fv dzMXP1eBM7Tceemnb+Z/jFU15u6LIULekvkWLz0RMxuNoTG6p2MiM/UU4H9JMv6tdOUbnP4N0zTdg S2M7H8uVQEzbrVl1aljnPT+hbPI5wVxIKJggWOsmMhNdq8PS/9AUKI6cY+0EJFbKHH2MxQLzGRmz4 nfVLy43DfctkmDgtx6RuDuPex3AP8weBtdUuxYzpRLIQaQ1cSeYxZ/CJS+xXjGuo+8egis0NI/L8T 2GjcnpipSNxp2H9joUoQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1m9edy-00AlFO-9F; Sat, 31 Jul 2021 02:22:30 +0000 Received: from szxga01-in.huawei.com ([45.249.212.187]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1m9edU-00Al7o-OG for linux-mtd@lists.infradead.org; Sat, 31 Jul 2021 02:22:03 +0000 Received: from dggemv703-chm.china.huawei.com (unknown [172.30.72.53]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4Gc7Cx40v3zYjZh; Sat, 31 Jul 2021 10:15:57 +0800 (CST) Received: from dggema761-chm.china.huawei.com (10.1.198.203) by dggemv703-chm.china.huawei.com (10.3.19.46) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.2176.2; Sat, 31 Jul 2021 10:21:56 +0800 Received: from huawei.com (10.175.127.227) by dggema761-chm.china.huawei.com (10.1.198.203) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2176.2; Sat, 31 Jul 2021 10:21:56 +0800 From: Zhihao Cheng To: , , , CC: , , , Subject: [PATCH 1/2] mtd: mtdconcat: Judge callback function existence getting from master for each partition Date: Sat, 31 Jul 2021 10:32:42 +0800 Message-ID: <20210731023243.3977104-2-chengzhihao1@huawei.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210731023243.3977104-1-chengzhihao1@huawei.com> References: <20210731023243.3977104-1-chengzhihao1@huawei.com> MIME-Version: 1.0 X-Originating-IP: [10.175.127.227] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To dggema761-chm.china.huawei.com (10.1.198.203) X-CFilter-Loop: Reflected X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210730_192201_164017_59651ED3 X-CRM114-Status: GOOD ( 13.76 ) X-Spam-Score: -2.3 (--) X-Spam-Report: Spam detection software, running on the system "bombadil.infradead.org", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: Since commit 46b5889cc2c5("mtd: implement proper partition handling") applied, mtd partition device won't hold some callback functions, such as _block_isbad, _block_markbad, etc. Besides, function mtd [...] Content analysis details: (-2.3 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 0.0 RCVD_IN_MSPIKE_H4 RBL: Very Good reputation (+4) [45.249.212.187 listed in wl.mailspike.net] -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at https://www.dnswl.org/, medium trust [45.249.212.187 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.0 RCVD_IN_MSPIKE_WL Mailspike good senders X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.34 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 Since commit 46b5889cc2c5("mtd: implement proper partition handling") applied, mtd partition device won't hold some callback functions, such as _block_isbad, _block_markbad, etc. Besides, function mtd_block_isbad() will get mtd device's master mtd device, then invokes master mtd device's callback function. So, following process may result mtd_block_isbad() always return 0, even though mtd device has bad blocks: 1. Split a mtd device into 3 partitions: PA, PB, PC [ Each mtd partition device won't has callback function _block_isbad(). ] 2. Concatenate PA and PB as a new mtd device PN [ mtd_concat_create() finds out each subdev has no callback function _block_isbad(), so PN won't be assigned callback function concat_block_isbad(). ] Then, mtd_block_isbad() checks "!master->_block_isbad" is true, will always return 0. Reproducer: // reproduce.c static int __init init_diy_module(void) { struct mtd_info *mtd[2]; struct mtd_info *mtd_combine = NULL; mtd[0] = get_mtd_device_nm("NAND simulator partition 0"); if (!mtd[0]) { pr_err("cannot find mtd1\n"); return -EINVAL; } mtd[1] = get_mtd_device_nm("NAND simulator partition 1"); if (!mtd[1]) { pr_err("cannot find mtd2\n"); return -EINVAL; } put_mtd_device(mtd[0]); put_mtd_device(mtd[1]); mtd_combine = mtd_concat_create(mtd, 2, "Combine mtd"); if (mtd_combine == NULL) { pr_err("comnoine fail\n"); return -EINVAL; } mtd_device_register(mtd_combine, NULL, 0); pr_info("Combine success\n"); return 0; } 1. ID="0x20,0xac,0x00,0x15" 2. modprobe nandsim id_bytes=$ID parts=50,100 badblocks=100 3. insmod reproduce.ko 4. flash_erase /dev/mtd3 0 0 libmtd: error!: MEMERASE64 ioctl failed for eraseblock 100 (mtd3) error 5 (Input/output error) // Should be "flash_erase: Skipping bad block at 00c80000" Fixes: 46b5889cc2c54bac ("mtd: implement proper partition handling") Signed-off-by: Zhihao Cheng --- drivers/mtd/mtdconcat.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c index 6e4d0017c0bd..ea130eeb54d5 100644 --- a/drivers/mtd/mtdconcat.c +++ b/drivers/mtd/mtdconcat.c @@ -641,6 +641,7 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c int i; size_t size; struct mtd_concat *concat; + struct mtd_info *subdev_master = NULL; uint32_t max_erasesize, curr_erasesize; int num_erase_region; int max_writebufsize = 0; @@ -679,17 +680,19 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c concat->mtd.subpage_sft = subdev[0]->subpage_sft; concat->mtd.oobsize = subdev[0]->oobsize; concat->mtd.oobavail = subdev[0]->oobavail; - if (subdev[0]->_writev) + + subdev_master = mtd_get_master(subdev[0]); + if (subdev_master->_writev) concat->mtd._writev = concat_writev; - if (subdev[0]->_read_oob) + if (subdev_master->_read_oob) concat->mtd._read_oob = concat_read_oob; - if (subdev[0]->_write_oob) + if (subdev_master->_write_oob) concat->mtd._write_oob = concat_write_oob; - if (subdev[0]->_block_isbad) + if (subdev_master->_block_isbad) concat->mtd._block_isbad = concat_block_isbad; - if (subdev[0]->_block_markbad) + if (subdev_master->_block_markbad) concat->mtd._block_markbad = concat_block_markbad; - if (subdev[0]->_panic_write) + if (subdev_master->_panic_write) concat->mtd._panic_write = concat_panic_write; concat->mtd.ecc_stats.badblocks = subdev[0]->ecc_stats.badblocks; @@ -721,14 +724,15 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c subdev[i]->flags & MTD_WRITEABLE; } + subdev_master = mtd_get_master(subdev[i]); concat->mtd.size += subdev[i]->size; concat->mtd.ecc_stats.badblocks += subdev[i]->ecc_stats.badblocks; if (concat->mtd.writesize != subdev[i]->writesize || concat->mtd.subpage_sft != subdev[i]->subpage_sft || concat->mtd.oobsize != subdev[i]->oobsize || - !concat->mtd._read_oob != !subdev[i]->_read_oob || - !concat->mtd._write_oob != !subdev[i]->_write_oob) { + !concat->mtd._read_oob != !subdev_master->_read_oob || + !concat->mtd._write_oob != !subdev_master->_write_oob) { kfree(concat); printk("Incompatible OOB or ECC data on \"%s\"\n", subdev[i]->name); From patchwork Sat Jul 31 02:32:43 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zhihao Cheng X-Patchwork-Id: 1511850 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.infradead.org (client-ip=2607:7c80:54:e::133; helo=bombadil.infradead.org; envelope-from=linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=Cu8icMLt; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:e::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Gc7NL2RNbz9sT6 for ; Sat, 31 Jul 2021 12:23:14 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:CC:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=JHZJ2eamHRoAK7k3snnQlu6snwWsKFHmFLapZ8Xta8o=; b=Cu8icMLtjrCC1x DPOBss8ckrgvNl/r/YGkC8CNbgxt26N4EVcB8zBnMIZcm7MRI5UnGg5pTh8bWha2Kle7ncuklqCWl Pa8YMNLRo4XyeCf0FEeGf5SnjJ55C6Z0u/MuSI2qhPGkcVjJAh2EkjzvSjgkZsofEoFP/zliSv1vG ImO25wKU4GX9VzVQXSVNcnz5xhDi+eghckTOR9yakn3kHo88yec4Ww3maTnU1BqpVT3N41dinTuVT u2wmqHBck8kv3PBcq71Dgst7HyU+d6bmwzkn6HtCzcQ923wBTrPgrZsHGmf+XgqVvETcD2VaqOxQq QRMy+GqER7E94CiDJxAg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1m9edl-00AlE9-Cp; Sat, 31 Jul 2021 02:22:17 +0000 Received: from szxga01-in.huawei.com ([45.249.212.187]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1m9edU-00Al7p-OF for linux-mtd@lists.infradead.org; Sat, 31 Jul 2021 02:22:02 +0000 Received: from dggemv711-chm.china.huawei.com (unknown [172.30.72.54]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4Gc7Gq1D3GzZqqc; Sat, 31 Jul 2021 10:18:27 +0800 (CST) Received: from dggema761-chm.china.huawei.com (10.1.198.203) by dggemv711-chm.china.huawei.com (10.1.198.66) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.2176.2; Sat, 31 Jul 2021 10:21:57 +0800 Received: from huawei.com (10.175.127.227) by dggema761-chm.china.huawei.com (10.1.198.203) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2176.2; Sat, 31 Jul 2021 10:21:56 +0800 From: Zhihao Cheng To: , , , CC: , , , Subject: [PATCH 2/2] mtd: mtdconcat: Remove concat_{read|write}_oob Date: Sat, 31 Jul 2021 10:32:43 +0800 Message-ID: <20210731023243.3977104-3-chengzhihao1@huawei.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210731023243.3977104-1-chengzhihao1@huawei.com> References: <20210731023243.3977104-1-chengzhihao1@huawei.com> MIME-Version: 1.0 X-Originating-IP: [10.175.127.227] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To dggema761-chm.china.huawei.com (10.1.198.203) X-CFilter-Loop: Reflected X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210730_192201_161029_611CC562 X-CRM114-Status: GOOD ( 20.13 ) X-Spam-Score: -2.3 (--) X-Spam-Report: Spam detection software, running on the system "bombadil.infradead.org", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: Since 2431c4f5b46c3("mtd: Implement mtd_{read,write}() as wrappers around mtd_{read, write}_oob()") don't allow _write|_read and _write_oob|_read_oob existing at the same time. We should stop these two [...] Content analysis details: (-2.3 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at https://www.dnswl.org/, medium trust [45.249.212.187 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 RCVD_IN_MSPIKE_H4 RBL: Very Good reputation (+4) [45.249.212.187 listed in wl.mailspike.net] 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.0 RCVD_IN_MSPIKE_WL Mailspike good senders X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.34 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 Since 2431c4f5b46c3("mtd: Implement mtd_{read,write}() as wrappers around mtd_{read,write}_oob()") don't allow _write|_read and _write_oob|_read_oob existing at the same time. We should stop these two callback functions assigning, otherwise following warning occurs while making concatenated device: WARNING: CPU: 2 PID: 6728 at drivers/mtd/mtdcore.c:595 add_mtd_device+0x7f/0x7b0 Fixes: 2431c4f5b46c3("mtd: Implement mtd_{read,write}() around ...") Signed-off-by: Zhihao Cheng --- drivers/mtd/mtdconcat.c | 113 +--------------------------------------- 1 file changed, 1 insertion(+), 112 deletions(-) diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c index ea130eeb54d5..98d1c79cf51d 100644 --- a/drivers/mtd/mtdconcat.c +++ b/drivers/mtd/mtdconcat.c @@ -256,110 +256,6 @@ concat_writev(struct mtd_info *mtd, const struct kvec *vecs, return err; } -static int -concat_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops) -{ - struct mtd_concat *concat = CONCAT(mtd); - struct mtd_oob_ops devops = *ops; - int i, err, ret = 0; - - ops->retlen = ops->oobretlen = 0; - - for (i = 0; i < concat->num_subdev; i++) { - struct mtd_info *subdev = concat->subdev[i]; - - if (from >= subdev->size) { - from -= subdev->size; - continue; - } - - /* partial read ? */ - if (from + devops.len > subdev->size) - devops.len = subdev->size - from; - - err = mtd_read_oob(subdev, from, &devops); - ops->retlen += devops.retlen; - ops->oobretlen += devops.oobretlen; - - /* Save information about bitflips! */ - if (unlikely(err)) { - if (mtd_is_eccerr(err)) { - mtd->ecc_stats.failed++; - ret = err; - } else if (mtd_is_bitflip(err)) { - mtd->ecc_stats.corrected++; - /* Do not overwrite -EBADMSG !! */ - if (!ret) - ret = err; - } else - return err; - } - - if (devops.datbuf) { - devops.len = ops->len - ops->retlen; - if (!devops.len) - return ret; - devops.datbuf += devops.retlen; - } - if (devops.oobbuf) { - devops.ooblen = ops->ooblen - ops->oobretlen; - if (!devops.ooblen) - return ret; - devops.oobbuf += ops->oobretlen; - } - - from = 0; - } - return -EINVAL; -} - -static int -concat_write_oob(struct mtd_info *mtd, loff_t to, struct mtd_oob_ops *ops) -{ - struct mtd_concat *concat = CONCAT(mtd); - struct mtd_oob_ops devops = *ops; - int i, err; - - if (!(mtd->flags & MTD_WRITEABLE)) - return -EROFS; - - ops->retlen = ops->oobretlen = 0; - - for (i = 0; i < concat->num_subdev; i++) { - struct mtd_info *subdev = concat->subdev[i]; - - if (to >= subdev->size) { - to -= subdev->size; - continue; - } - - /* partial write ? */ - if (to + devops.len > subdev->size) - devops.len = subdev->size - to; - - err = mtd_write_oob(subdev, to, &devops); - ops->retlen += devops.retlen; - ops->oobretlen += devops.oobretlen; - if (err) - return err; - - if (devops.datbuf) { - devops.len = ops->len - ops->retlen; - if (!devops.len) - return 0; - devops.datbuf += devops.retlen; - } - if (devops.oobbuf) { - devops.ooblen = ops->ooblen - ops->oobretlen; - if (!devops.ooblen) - return 0; - devops.oobbuf += devops.oobretlen; - } - to = 0; - } - return -EINVAL; -} - static int concat_erase(struct mtd_info *mtd, struct erase_info *instr) { struct mtd_concat *concat = CONCAT(mtd); @@ -684,10 +580,6 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c subdev_master = mtd_get_master(subdev[0]); if (subdev_master->_writev) concat->mtd._writev = concat_writev; - if (subdev_master->_read_oob) - concat->mtd._read_oob = concat_read_oob; - if (subdev_master->_write_oob) - concat->mtd._write_oob = concat_write_oob; if (subdev_master->_block_isbad) concat->mtd._block_isbad = concat_block_isbad; if (subdev_master->_block_markbad) @@ -724,15 +616,12 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c subdev[i]->flags & MTD_WRITEABLE; } - subdev_master = mtd_get_master(subdev[i]); concat->mtd.size += subdev[i]->size; concat->mtd.ecc_stats.badblocks += subdev[i]->ecc_stats.badblocks; if (concat->mtd.writesize != subdev[i]->writesize || concat->mtd.subpage_sft != subdev[i]->subpage_sft || - concat->mtd.oobsize != subdev[i]->oobsize || - !concat->mtd._read_oob != !subdev_master->_read_oob || - !concat->mtd._write_oob != !subdev_master->_write_oob) { + concat->mtd.oobsize != subdev[i]->oobsize) { kfree(concat); printk("Incompatible OOB or ECC data on \"%s\"\n", subdev[i]->name);