diff options
author | Stewart Smith <stewart@linux.vnet.ibm.com> | 2015-05-12 14:05:41 +1000 |
---|---|---|
committer | Stewart Smith <stewart@linux.vnet.ibm.com> | 2015-05-12 14:56:13 +1000 |
commit | 101d094f22da42b178b325d0710c140079e00af3 (patch) | |
tree | bcdcbdfd35f84de5db2c2da69538055e37e17188 /core/flash.c | |
parent | 8d0186492691501a4ad2270438eb6fc756a13664 (diff) | |
download | skiboot-101d094f22da42b178b325d0710c140079e00af3.zip skiboot-101d094f22da42b178b325d0710c140079e00af3.tar.gz skiboot-101d094f22da42b178b325d0710c140079e00af3.tar.bz2 |
Fix race in flash resource preloading
(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@kernel.crashing.org>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
Diffstat (limited to 'core/flash.c')
-rw-r--r-- | core/flash.c | 9 |
1 files 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) |