Re: UBI fastmap updates

Shmulik Ladkani
2012-07-09T07:38:57Z

Hi Richard,

On Sun, 08 Jul 2012 14:07:41 +0200 Richard Weinberger <richard@nod.at> wrote:
> > + /* TODO: in the new locking scheme, produce_free_peb is
> > + * called under wl_lock taken.
> > + * so when returning, should reacquire the lock
> > + */
>
> Which new locking scheme?

I am diffing linux-ubi fastmap HEAD against its fork point (vanilla
ubi), that's 6b16351..d41a140 on linux-ubi.

Which gives the following diff in produce_free_pebs:

@@ -261,7 +266,6 @@ static int produce_free_peb(struct ubi_device *ubi)
{
int err;

- spin_lock(&ubi->wl_lock);
while (!ubi->free.rb_node) {
spin_unlock(&ubi->wl_lock);

@@ -272,7 +276,6 @@ static int produce_free_peb(struct ubi_device *ubi)

spin_lock(&ubi->wl_lock);
}
- spin_unlock(&ubi->wl_lock);

return 0;
}

Which is probably okay, since you obtain the lock in the new
'ubi_refill_pools', which calls produce_free_peb:

+void ubi_refill_pools(struct ubi_device *ubi)
+{
+ spin_lock(&ubi->wl_lock);
+ refill_wl_pool(ubi);
+ refill_wl_user_pool(ubi);
+ spin_unlock(&ubi->wl_lock);
+}

However if 'do_work' fails within 'produce_free_peb', you return the
error but leave wl_lock unlocked - where it is expected to be locked
(otherwise, ubi_refill_pools will unlock it again):

static int produce_free_peb(struct ubi_device *ubi)
{
int err;

while (!ubi->free.rb_node) {
spin_unlock(&ubi->wl_lock);

dbg_wl("do one work synchronously");
err = do_work(ubi);
if (err)
return err;

spin_lock(&ubi->wl_lock);
}

return 0;
}

Regards,
Shmulik