Re: UBIFS Panic

[Richard Weinberger <richard@nod.at>]

[-- Attachment #1: Type: text/plain, Size: 1121 bytes --]

Akshay,

Am 16.07.2014 19:31, schrieb Akshay Bhat:
> 
>>>
>>> Hi Richard, wanted to check if you got a chance to dig into this? Thanks.
> 
>> Can you please rerun with the attached patch applied?
>> Maybe it can give use a hint. :)
> 
> I ran the tests with the patch, below is the dmesg log (note: the first kernel panic resulted in the log running over since I wasn't around, so I had to reboot and re-run the test to capture a new panic).

Thanks a lot for the log!

> # dmesg
> [ 1571.338966] UBI error: refill_wl_pool: didn't get all pebs I wanted!
> [ 1571.367128] UBI error: refill_wl_pool: didn't get all pebs I wanted!
> [ 1571.367159] UBI error: ubi_wl_get_peb: User WL pool is empty!

This is interesting.
Looks like a race in ubi_wl_get_peb(), two threads enter the function and the pool needs refilling.
While T1 triggers a ubi_refill_pools(), T2 goes immediately to " if (!pool->size) {" this triggers
as T1 resets pool->size to 0 while refilling.

Please test the updated debug patch.

Can you also please share your test script with me?
So far none of my boards run into that issue. :(

Thanks,
//richard

[-- Attachment #2: debug.diff --]
[-- Type: text/x-patch, Size: 1788 bytes --]

diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
index 0f3425d..d02b5f9 100644
--- a/drivers/mtd/ubi/wl.c
+++ b/drivers/mtd/ubi/wl.c
@@ -575,8 +575,10 @@ static void refill_wl_pool(struct ubi_device *ubi)
 
 	for (pool->size = 0; pool->size < pool->max_size; pool->size++) {
 		if (!ubi->free.rb_node ||
-		   (ubi->free_count - ubi->beb_rsvd_pebs < 5))
+		   (ubi->free_count - ubi->beb_rsvd_pebs < 5)) {
+			ubi_err("didn't get all pebs I wanted!");
 			break;
+		}
 
 		e = find_wl_entry(ubi, &ubi->free, WL_FREE_MAX_DIFF);
 		self_check_in_wl_tree(ubi, e, &ubi->free);
@@ -600,8 +602,10 @@ static void refill_wl_user_pool(struct ubi_device *ubi)
 
 	for (pool->size = 0; pool->size < pool->max_size; pool->size++) {
 		pool->pebs[pool->size] = __wl_get_peb(ubi);
-		if (pool->pebs[pool->size] < 0)
+		if (pool->pebs[pool->size] < 0) {
+			ubi_err("didn't get all pebs I wanted!");
 			break;
+		}
 	}
 	pool->used = 0;
 }
@@ -631,15 +635,16 @@ int ubi_wl_get_peb(struct ubi_device *ubi)
 	    wl_pool->used == wl_pool->size)
 		ubi_update_fastmap(ubi);
 
+	spin_lock(&ubi->wl_lock);
 	/* we got not a single free PEB */
-	if (!pool->size)
+	if (!pool->size) {
+		ubi_err("User WL pool is empty!");
 		ret = -ENOSPC;
-	else {
-		spin_lock(&ubi->wl_lock);
+	} else {
 		ret = pool->pebs[pool->used++];
 		prot_queue_add(ubi, ubi->lookuptbl[ret]);
-		spin_unlock(&ubi->wl_lock);
 	}
+	spin_unlock(&ubi->wl_lock);
 
 	return ret;
 }
@@ -654,6 +659,7 @@ static struct ubi_wl_entry *get_peb_for_wl(struct ubi_device *ubi)
 	int pnum;
 
 	if (pool->used == pool->size || !pool->size) {
+		ubi_err("WL pool is empty!");
 		/* We cannot update the fastmap here because this
 		 * function is called in atomic context.
 		 * Let's fail here and refill/update it as soon as possible. */