From patchwork Fri Sep 16 14:59:20 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boris Brezillon X-Patchwork-Id: 670936 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2001:1868:205::9]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3sbJT35Lyxz9sdm for ; Sat, 17 Sep 2016 01:03:51 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1bkuer-0001Pg-Px; Fri, 16 Sep 2016 15:02:29 +0000 Received: from down.free-electrons.com ([37.187.137.238] helo=mail.free-electrons.com) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1bkuce-0006xN-2y for linux-mtd@lists.infradead.org; Fri, 16 Sep 2016 15:00:23 +0000 Received: by mail.free-electrons.com (Postfix, from userid 110) id 0EA642EB9; Fri, 16 Sep 2016 16:59:30 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on mail.free-electrons.com X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT shortcircuit=ham autolearn=disabled version=3.4.0 Received: from bbrezillon.home (LStLambert-657-1-97-87.w90-63.abo.wanadoo.fr [90.63.216.87]) by mail.free-electrons.com (Postfix) with ESMTPSA id BCED42EAC; Fri, 16 Sep 2016 16:59:29 +0200 (CEST) From: Boris Brezillon To: Artem Bityutskiy , Richard Weinberger Subject: [PATCH v3 12/18] UBI: simplify recover_peb() code Date: Fri, 16 Sep 2016 16:59:20 +0200 Message-Id: <1474037966-5524-13-git-send-email-boris.brezillon@free-electrons.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1474037966-5524-1-git-send-email-boris.brezillon@free-electrons.com> References: <1474037966-5524-1-git-send-email-boris.brezillon@free-electrons.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160916_080012_785707_6969B190 X-CRM114-Status: GOOD ( 18.40 ) X-Spam-Score: -2.3 (--) X-Spam-Report: SpamAssassin version 3.4.0 on bombadil.infradead.org summary: Content analysis details: (-2.3 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, no trust [37.187.137.238 listed in list.dnswl.org] 1.0 FSL_HELO_HOME No description available. -0.0 SPF_PASS SPF: sender matches SPF record -1.4 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] X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-mtd@lists.infradead.org, Brian Norris , David Woodhouse , linux-kernel@vger.kernel.org, Boris Brezillon MIME-Version: 1.0 Sender: "linux-mtd" Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org recover_peb() is using a convoluted retry/exit path. Add try_recover_peb() to simplify the retry logic and make sure we have a single exit path instead of manually releasing the resource in each error path. Signed-off-by: Boris Brezillon --- drivers/mtd/ubi/eba.c | 130 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 77 insertions(+), 53 deletions(-) diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c index ebf517271d29..be59cfb81934 100644 --- a/drivers/mtd/ubi/eba.c +++ b/drivers/mtd/ubi/eba.c @@ -554,39 +554,37 @@ int ubi_eba_read_leb_sg(struct ubi_device *ubi, struct ubi_volume *vol, } /** - * recover_peb - recover from write failure. - * @ubi: UBI device description object + * try_recover_peb - try to recover from write failure. + * @vol: volume description object * @pnum: the physical eraseblock to recover - * @vol_id: volume ID * @lnum: logical eraseblock number * @buf: data which was not written because of the write failure * @offset: offset of the failed write * @len: how many bytes should have been written + * @vid: VID header + * @retry: whether the caller should retry in case of failure * * This function is called in case of a write failure and moves all good data * from the potentially bad physical eraseblock to a good physical eraseblock. * This function also writes the data which was not written due to the failure. - * Returns new physical eraseblock number in case of success, and a negative - * error code in case of failure. + * Returns 0 in case of success, and a negative error code in case of failure. + * In case of failure, the %retry parameter is set to false if this is a fatal + * error (retrying won't help), and true otherwise. */ -static int recover_peb(struct ubi_device *ubi, int pnum, int vol_id, int lnum, - const void *buf, int offset, int len) +static int try_recover_peb(struct ubi_volume *vol, int pnum, int lnum, + const void *buf, int offset, int len, + struct ubi_vid_hdr *vid_hdr, bool *retry) { - int err, idx = vol_id2idx(ubi, vol_id), new_pnum, data_size, tries = 0; - struct ubi_volume *vol = ubi->volumes[idx]; - struct ubi_vid_hdr *vid_hdr; + struct ubi_device *ubi = vol->ubi; + int new_pnum, err, vol_id = vol->vol_id, data_size; uint32_t crc; - vid_hdr = ubi_zalloc_vid_hdr(ubi, GFP_NOFS); - if (!vid_hdr) - return -ENOMEM; + *retry = false; -retry: new_pnum = ubi_wl_get_peb(ubi); if (new_pnum < 0) { - ubi_free_vid_hdr(ubi, vid_hdr); - up_read(&ubi->fm_eba_sem); - return new_pnum; + err = new_pnum; + goto out_put; } ubi_msg(ubi, "recover PEB %d, move data to PEB %d", @@ -596,7 +594,6 @@ retry: if (err && err != UBI_IO_BITFLIPS) { if (err > 0) err = -EIO; - up_read(&ubi->fm_eba_sem); goto out_put; } @@ -608,12 +605,12 @@ retry: /* Read everything before the area where the write failure happened */ if (offset > 0) { err = ubi_io_read_data(ubi, ubi->peb_buf, pnum, 0, offset); - if (err && err != UBI_IO_BITFLIPS) { - up_read(&ubi->fm_eba_sem); + if (err && err != UBI_IO_BITFLIPS) goto out_unlock; - } } + *retry = true; + memcpy(ubi->peb_buf + offset, buf, len); data_size = offset + len; @@ -623,49 +620,76 @@ retry: vid_hdr->data_size = cpu_to_be32(data_size); vid_hdr->data_crc = cpu_to_be32(crc); err = ubi_io_write_vid_hdr(ubi, new_pnum, vid_hdr); - if (err) { - mutex_unlock(&ubi->buf_mutex); - up_read(&ubi->fm_eba_sem); - goto write_error; - } + if (err) + goto out_unlock; err = ubi_io_write_data(ubi, ubi->peb_buf, new_pnum, 0, data_size); - if (err) { - mutex_unlock(&ubi->buf_mutex); - up_read(&ubi->fm_eba_sem); - goto write_error; - } +out_unlock: mutex_unlock(&ubi->buf_mutex); - ubi_free_vid_hdr(ubi, vid_hdr); - vol->eba_tbl[lnum] = new_pnum; + if (!err) + vol->eba_tbl[lnum] = new_pnum; + +out_put: up_read(&ubi->fm_eba_sem); - ubi_wl_put_peb(ubi, vol_id, lnum, pnum, 1); - ubi_msg(ubi, "data was successfully recovered"); - return 0; + if (!err) { + ubi_wl_put_peb(ubi, vol_id, lnum, pnum, 1); + ubi_msg(ubi, "data was successfully recovered"); + } else if (new_pnum >= 0) { + /* + * Bad luck? This physical eraseblock is bad too? Crud. Let's + * try to get another one. + */ + ubi_wl_put_peb(ubi, vol_id, lnum, new_pnum, 1); + ubi_warn(ubi, "failed to write to PEB %d", new_pnum); + } -out_unlock: - mutex_unlock(&ubi->buf_mutex); -out_put: - ubi_wl_put_peb(ubi, vol_id, lnum, new_pnum, 1); - ubi_free_vid_hdr(ubi, vid_hdr); return err; +} -write_error: - /* - * Bad luck? This physical eraseblock is bad too? Crud. Let's try to - * get another one. - */ - ubi_warn(ubi, "failed to write to PEB %d", new_pnum); - ubi_wl_put_peb(ubi, vol_id, lnum, new_pnum, 1); - if (++tries > UBI_IO_RETRIES) { - ubi_free_vid_hdr(ubi, vid_hdr); - return err; +/** + * recover_peb - recover from write failure. + * @ubi: UBI device description object + * @pnum: the physical eraseblock to recover + * @vol_id: volume ID + * @lnum: logical eraseblock number + * @buf: data which was not written because of the write failure + * @offset: offset of the failed write + * @len: how many bytes should have been written + * + * This function is called in case of a write failure and moves all good data + * from the potentially bad physical eraseblock to a good physical eraseblock. + * This function also writes the data which was not written due to the failure. + * Returns 0 in case of success, and a negative error code in case of failure. + * This function tries %UBI_IO_RETRIES before giving up. + */ +static int recover_peb(struct ubi_device *ubi, int pnum, int vol_id, int lnum, + const void *buf, int offset, int len) +{ + int err, idx = vol_id2idx(ubi, vol_id), tries; + struct ubi_volume *vol = ubi->volumes[idx]; + struct ubi_vid_hdr *vid_hdr; + + vid_hdr = ubi_zalloc_vid_hdr(ubi, GFP_NOFS); + if (!vid_hdr) + return -ENOMEM; + + for (tries = 0; tries <= UBI_IO_RETRIES; tries++) { + bool retry; + + err = try_recover_peb(vol, pnum, lnum, buf, offset, len, + vid_hdr, &retry); + if (!err || !retry) + break; + + ubi_msg(ubi, "try again"); } - ubi_msg(ubi, "try again"); - goto retry; + + ubi_free_vid_hdr(ubi, vid_hdr); + + return err; } /**