[Skiboot] [PATCH] Fix race in flash resource preloading
Stewart Smith
stewart at linux.vnet.ibm.com
Tue May 12 14:05:41 AEST 2015
(13:31:46) benh: stewart: flash_load_resources()
(13:31:53) benh: stewart: you hit the unlock at the bottom of the loop
(13:31:59) benh: stewart: at that point the list may be empty
(13:32:09) benh: stewart: and so another concurrent load can restart the thread
(13:32:15) benh: stewart: you end up with duplicate threads
(13:32:26) benh: stewart: in which case you can hit the assert
<patch goes here>
(13:34:27) benh: ie, never drop the lock with the queue empty
(13:34:29) benh: unless you know you will exit the job
(13:34:32) benh: otherwise you can have a duplicate job
(13:34:41) benh: -> kaboom
(13:36:29) benh: yeah the decision to exit the loop must be atomic with
the popping of the last element in the list
(13:36:43) benh: to match the decision to start the thread which is atomic
with the queuing of the first element
Reported-by: Benjamin Herrenschmidt <benh at kernel.crashing.org>
Signed-off-by: Stewart Smith <stewart at linux.vnet.ibm.com>
---
core/flash.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/core/flash.c b/core/flash.c
index 092f8f2..c08bb49 100644
--- a/core/flash.c
+++ b/core/flash.c
@@ -680,15 +680,16 @@ static void flash_load_resources(void *data __unused)
struct flash_load_resource_item *r;
int result;
+ lock(&flash_load_resource_lock);
do {
- lock(&flash_load_resource_lock);
if (list_empty(&flash_load_resource_queue)) {
- unlock(&flash_load_resource_lock);
break;
}
r = list_top(&flash_load_resource_queue,
struct flash_load_resource_item, link);
- assert(r->result == OPAL_EMPTY);
+ if (r->result != OPAL_EMPTY)
+ prerror("flash_load_resources() list_top unexpected "
+ " result %d\n", r->result);
r->result = OPAL_BUSY;
unlock(&flash_load_resource_lock);
@@ -699,8 +700,8 @@ static void flash_load_resources(void *data __unused)
struct flash_load_resource_item, link);
r->result = result;
list_add_tail(&flash_loaded_resources, &r->link);
- unlock(&flash_load_resource_lock);
} while(true);
+ unlock(&flash_load_resource_lock);
}
static void start_flash_load_resource_job(void)
--
1.7.10.4
More information about the Skiboot
mailing list