UBI: simplify recover_peb() code

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 <boris.brezillon@free-electrons.com>
Signed-off-by: Richard Weinberger <richard@nod.at>

diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
index ebf517271d2925452d911eb8df9a9d973b05926c..be59cfb81934c8be1004f14c1f7d16d6eb18fb7e 100644 (file)
--- 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;
 }
 
 /**