Re: [PATCH 2/5] ubi: Limit amount of reserved eraseblocks for badPEB handling

From: Artem Bityutskiy
Date: Wed Jul 18 2012 - 06:34:14 EST


On Wed, 2012-07-04 at 11:06 +0300, Shmulik Ladkani wrote:
> @@ -1045,20 +1046,14 @@ static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk,
> }
>
> spin_lock(&ubi->volumes_lock);
> - need = ubi->beb_rsvd_level - ubi->beb_rsvd_pebs + 1;
> - if (need > 0) {
> - need = ubi->avail_pebs >= need ? need : ubi->avail_pebs;
> - ubi->avail_pebs -= need;
> - ubi->rsvd_pebs += need;
> - ubi->beb_rsvd_pebs += need;
> - if (need > 0)
> - ubi_msg("reserve more %d PEBs", need);
> - }
> -
> if (ubi->beb_rsvd_pebs == 0) {
> - spin_unlock(&ubi->volumes_lock);
> - ubi_err("no reserved physical eraseblocks");
> - goto out_ro;
> + if (ubi->avail_pebs == 0) {
> + spin_unlock(&ubi->volumes_lock);
> + ubi_err("no reserved/available physical eraseblocks");
> + goto out_ro;
> + }
> + ubi->avail_pebs -= 1;
> + available_consumed = 1;
> }
> spin_unlock(&ubi->volumes_lock);

The whole thing will become simpler if we first mark the PEB as bad
unconditionally (because it _is_ bad), then grab the lock and do all the
re-calculations.

>
> @@ -1068,11 +1063,23 @@ static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk,
> goto out_ro;
>
> spin_lock(&ubi->volumes_lock);
> - ubi->beb_rsvd_pebs -= 1;
> + if (ubi->beb_rsvd_pebs > 0) {
> + if (available_consumed) {
> + /*
> + * Some PEBs were added to the reserved pool since we
> + * last checked. Use a PEB from the reserved pool.
> + */
> + ubi->avail_pebs += 1;
> + available_consumed = 0;
> + }
> + ubi->beb_rsvd_pebs -= 1;
> + }
> ubi->bad_peb_count += 1;
> ubi->good_peb_count -= 1;
> ubi_calculate_reserved(ubi);

We do not need to call this function from here, right?

--
Best Regards,
Artem Bityutskiy

Attachment: signature.asc
Description: This is a digitally signed message part