ubi: suspicious calculation in 'ubi_wl_get_peb'

[Shmulik Ladkani]

Hi,

Sorry for a long post - this issue requires some detailed background.

It looks like a calculation within 'ubi_wl_get_peb' involved in
picking a candidate PEB is incorrect.

The function 'find_wl_entry(rb_root *root, int max)' finds a
wear-leveling entry closest to a certain erase counter.

The parameter 'max' is documented as "highest possible erase counter",
however it is actually "diff from first entry" - since initially, the
funtion adds first entry's EC to the given 'max':

	e = rb_entry(rb_first(root), struct ubi_wl_entry, u.rb);
	max += e->ec;

Almost all calls to 'find_wl_entry' follow the (undocumented)
convention, providing "diff from first" argument (where the absolute
'max' value seeked is calculated *internally* by 'find_wl_entry').
E.g. 4 of 5 calls are of the pattern:

	e = find_wl_entry(&ubi->free, WL_FREE_MAX_DIFF);

However, there is one call where the caller seems to provide an actual
absolute "max" value, instead of a "diff from first".
The call is within 'ubi_wl_get_peb', quote:

	case UBI_UNKNOWN:
		/*
		 * For unknown data we pick a physical eraseblock with medium
		 * erase counter. But we by no means can pick a physical
		 * eraseblock with erase counter greater or equivalent than the
		 * lowest erase counter plus %WL_FREE_MAX_DIFF.
		 */
		first = rb_entry(rb_first(&ubi->free), struct ubi_wl_entry,
					u.rb);
		last = rb_entry(rb_last(&ubi->free), struct ubi_wl_entry, u.rb);

		if (last->ec - first->ec < WL_FREE_MAX_DIFF)
			e = rb_entry(ubi->free.rb_node,
					struct ubi_wl_entry, u.rb);
		else {
			medium_ec = (first->ec + WL_FREE_MAX_DIFF)/2;
			e = find_wl_entry(&ubi->free, medium_ec);
		}
		break;

Note calculation of 'medium_ec' - it does no seem as a "diff" value.
The *actual* resulting seeked EC is approximately 1.5*First + Diff/2.
Is it the intended behavior? it does not seem to fit the remark.

Deducting from the "last->ec - first->ec < WL_FREE_MAX_DIFF" condition,
it looks like it should have been:

-			e = find_wl_entry(&ubi->free, medium_ec);
+			e = find_wl_entry(&ubi->free, WL_FREE_MAX_DIFF/2)

Did I get something wrong?

Looking forward for any comments.

Regards,
Shmulik