linux-kernel - Re: [PATCH 2/5] ubi: Limit amount of reserved eraseblocks for bad PEB handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1342607317.7530.9.camel@brekeke>
Date:	Wed, 18 Jul 2012 13:28:37 +0300
From:	Artem Bityutskiy <dedekind1@...il.com>
To:	Shmulik Ladkani <shmulik.ladkani@...il.com>
Cc:	linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Richard Weinberger <richard@....at>,
	Richard Genoud <richard.genoud@...il.com>
Subject: Re: [PATCH 2/5] ubi: Limit amount of reserved eraseblocks for bad
 PEB handling

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

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)