Skip to content

Commit f036dfe

Browse files
Boris Brezillonrichardweinberger
Boris Brezillon
authored and
richardweinberger
committed
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>
1 parent 7b6b749 commit f036dfe

File tree

1 file changed

+77
-53
lines changed

1 file changed

+77
-53
lines changed

‎drivers/mtd/ubi/eba.c

Lines changed: 77 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -554,39 +554,37 @@ int ubi_eba_read_leb_sg(struct ubi_device *ubi, struct ubi_volume *vol,
554554
}
555555

556556
/**
557-
* recover_peb - recover from write failure.
558-
* @ubi: UBI device description object
557+
* try_recover_peb - try to recover from write failure.
558+
* @vol: volume description object
559559
* @pnum: the physical eraseblock to recover
560-
* @vol_id: volume ID
561560
* @lnum: logical eraseblock number
562561
* @buf: data which was not written because of the write failure
563562
* @offset: offset of the failed write
564563
* @len: how many bytes should have been written
564+
* @vid: VID header
565+
* @retry: whether the caller should retry in case of failure
565566
*
566567
* This function is called in case of a write failure and moves all good data
567568
* from the potentially bad physical eraseblock to a good physical eraseblock.
568569
* This function also writes the data which was not written due to the failure.
569-
* Returns new physical eraseblock number in case of success, and a negative
570-
* error code in case of failure.
570+
* Returns 0 in case of success, and a negative error code in case of failure.
571+
* In case of failure, the %retry parameter is set to false if this is a fatal
572+
* error (retrying won't help), and true otherwise.
571573
*/
572-
static int recover_peb(struct ubi_device *ubi, int pnum, int vol_id, int lnum,
573-
const void *buf, int offset, int len)
574+
static int try_recover_peb(struct ubi_volume *vol, int pnum, int lnum,
575+
const void *buf, int offset, int len,
576+
struct ubi_vid_hdr *vid_hdr, bool *retry)
574577
{
575-
int err, idx = vol_id2idx(ubi, vol_id), new_pnum, data_size, tries = 0;
576-
struct ubi_volume *vol = ubi->volumes[idx];
577-
struct ubi_vid_hdr *vid_hdr;
578+
struct ubi_device *ubi = vol->ubi;
579+
int new_pnum, err, vol_id = vol->vol_id, data_size;
578580
uint32_t crc;
579581

580-
vid_hdr = ubi_zalloc_vid_hdr(ubi, GFP_NOFS);
581-
if (!vid_hdr)
582-
return -ENOMEM;
582+
*retry = false;
583583

584-
retry:
585584
new_pnum = ubi_wl_get_peb(ubi);
586585
if (new_pnum < 0) {
587-
ubi_free_vid_hdr(ubi, vid_hdr);
588-
up_read(&ubi->fm_eba_sem);
589-
return new_pnum;
586+
err = new_pnum;
587+
goto out_put;
590588
}
591589

592590
ubi_msg(ubi, "recover PEB %d, move data to PEB %d",
@@ -596,7 +594,6 @@ static int recover_peb(struct ubi_device *ubi, int pnum, int vol_id, int lnum,
596594
if (err && err != UBI_IO_BITFLIPS) {
597595
if (err > 0)
598596
err = -EIO;
599-
up_read(&ubi->fm_eba_sem);
600597
goto out_put;
601598
}
602599

@@ -608,12 +605,12 @@ static int recover_peb(struct ubi_device *ubi, int pnum, int vol_id, int lnum,
608605
/* Read everything before the area where the write failure happened */
609606
if (offset > 0) {
610607
err = ubi_io_read_data(ubi, ubi->peb_buf, pnum, 0, offset);
611-
if (err && err != UBI_IO_BITFLIPS) {
612-
up_read(&ubi->fm_eba_sem);
608+
if (err && err != UBI_IO_BITFLIPS)
613609
goto out_unlock;
614-
}
615610
}
616611

612+
*retry = true;
613+
617614
memcpy(ubi->peb_buf + offset, buf, len);
618615

619616
data_size = offset + len;
@@ -623,49 +620,76 @@ static int recover_peb(struct ubi_device *ubi, int pnum, int vol_id, int lnum,
623620
vid_hdr->data_size = cpu_to_be32(data_size);
624621
vid_hdr->data_crc = cpu_to_be32(crc);
625622
err = ubi_io_write_vid_hdr(ubi, new_pnum, vid_hdr);
626-
if (err) {
627-
mutex_unlock(&ubi->buf_mutex);
628-
up_read(&ubi->fm_eba_sem);
629-
goto write_error;
630-
}
623+
if (err)
624+
goto out_unlock;
631625

632626
err = ubi_io_write_data(ubi, ubi->peb_buf, new_pnum, 0, data_size);
633-
if (err) {
634-
mutex_unlock(&ubi->buf_mutex);
635-
up_read(&ubi->fm_eba_sem);
636-
goto write_error;
637-
}
638627

628+
out_unlock:
639629
mutex_unlock(&ubi->buf_mutex);
640-
ubi_free_vid_hdr(ubi, vid_hdr);
641630

642-
vol->eba_tbl[lnum] = new_pnum;
631+
if (!err)
632+
vol->eba_tbl[lnum] = new_pnum;
633+
634+
out_put:
643635
up_read(&ubi->fm_eba_sem);
644-
ubi_wl_put_peb(ubi, vol_id, lnum, pnum, 1);
645636

646-
ubi_msg(ubi, "data was successfully recovered");
647-
return 0;
637+
if (!err) {
638+
ubi_wl_put_peb(ubi, vol_id, lnum, pnum, 1);
639+
ubi_msg(ubi, "data was successfully recovered");
640+
} else if (new_pnum >= 0) {
641+
/*
642+
* Bad luck? This physical eraseblock is bad too? Crud. Let's
643+
* try to get another one.
644+
*/
645+
ubi_wl_put_peb(ubi, vol_id, lnum, new_pnum, 1);
646+
ubi_warn(ubi, "failed to write to PEB %d", new_pnum);
647+
}
648648

649-
out_unlock:
650-
mutex_unlock(&ubi->buf_mutex);
651-
out_put:
652-
ubi_wl_put_peb(ubi, vol_id, lnum, new_pnum, 1);
653-
ubi_free_vid_hdr(ubi, vid_hdr);
654649
return err;
650+
}
655651

656-
write_error:
657-
/*
658-
* Bad luck? This physical eraseblock is bad too? Crud. Let's try to
659-
* get another one.
660-
*/
661-
ubi_warn(ubi, "failed to write to PEB %d", new_pnum);
662-
ubi_wl_put_peb(ubi, vol_id, lnum, new_pnum, 1);
663-
if (++tries > UBI_IO_RETRIES) {
664-
ubi_free_vid_hdr(ubi, vid_hdr);
665-
return err;
652+
/**
653+
* recover_peb - recover from write failure.
654+
* @ubi: UBI device description object
655+
* @pnum: the physical eraseblock to recover
656+
* @vol_id: volume ID
657+
* @lnum: logical eraseblock number
658+
* @buf: data which was not written because of the write failure
659+
* @offset: offset of the failed write
660+
* @len: how many bytes should have been written
661+
*
662+
* This function is called in case of a write failure and moves all good data
663+
* from the potentially bad physical eraseblock to a good physical eraseblock.
664+
* This function also writes the data which was not written due to the failure.
665+
* Returns 0 in case of success, and a negative error code in case of failure.
666+
* This function tries %UBI_IO_RETRIES before giving up.
667+
*/
668+
static int recover_peb(struct ubi_device *ubi, int pnum, int vol_id, int lnum,
669+
const void *buf, int offset, int len)
670+
{
671+
int err, idx = vol_id2idx(ubi, vol_id), tries;
672+
struct ubi_volume *vol = ubi->volumes[idx];
673+
struct ubi_vid_hdr *vid_hdr;
674+
675+
vid_hdr = ubi_zalloc_vid_hdr(ubi, GFP_NOFS);
676+
if (!vid_hdr)
677+
return -ENOMEM;
678+
679+
for (tries = 0; tries <= UBI_IO_RETRIES; tries++) {
680+
bool retry;
681+
682+
err = try_recover_peb(vol, pnum, lnum, buf, offset, len,
683+
vid_hdr, &retry);
684+
if (!err || !retry)
685+
break;
686+
687+
ubi_msg(ubi, "try again");
666688
}
667-
ubi_msg(ubi, "try again");
668-
goto retry;
689+
690+
ubi_free_vid_hdr(ubi, vid_hdr);
691+
692+
return err;
669693
}
670694

671695
/**

0 commit comments

Comments
 (0)