summaryrefslogtreecommitdiffstats
path: root/cpukit/libblock
diff options
context:
space:
mode:
authorSebastian Huber <sebastian.huber@embedded-brains.de>2014-11-27 14:41:17 +0100
committerSebastian Huber <sebastian.huber@embedded-brains.de>2014-11-28 10:56:46 +0100
commit3b4ca3ab0f99d15794a3eee60b5735f834fd898c (patch)
treef1e064023ef85d9179082fe7bb0b9186a5acf366 /cpukit/libblock
parentsync.c: Add asserts to document and check assumptions (diff)
downloadrtems-3b4ca3ab0f99d15794a3eee60b5735f834fd898c.tar.bz2
bdbuf: Fix race condition with sync active flag
Bug report by Oleg Kravtsov: In rtems_bdbuf_swapout_processing() function there is the following lines: if (bdbuf_cache.sync_active && !transfered_buffers) { rtems_id sync_requester; rtems_bdbuf_lock_cache (); ... } Here access to bdbuf_cache.sync_active is not protected with anything. Imagine the following test case: 1. Task1 releases buffer(s) with bdbuf_release_modified() calls; 2. After a while swapout task starts and flushes all buffers; 3. In the end of that swapout flush we are before that part of code, and assume there is task switching (just before "if (bdbuf_cache.sync_active && !transfered_buffers)"); 4. Some other task (with higher priority) does bdbuf_release_modified and rtems_bdbuf_syncdev(). This task successfully gets both locks sync and pool (in rtems_bdbuf_syncdev() function), sets sync_active to true and starts waiting for RTEMS_BDBUF_TRANSFER_SYNC event with only sync lock got. 5. Task switching happens again and we are again before "if (bdbuf_cache.sync_active && !transfered_buffers)". As the result we check sync_active and we come inside that "if" statement. 6. The result is that we send RTEMS_BDBUF_TRANSFER_SYNC event! Though ALL modified messages of that task are not flushed yet! close #1485
Diffstat (limited to 'cpukit/libblock')
-rw-r--r--cpukit/libblock/src/bdbuf.c16
1 files changed, 11 insertions, 5 deletions
diff --git a/cpukit/libblock/src/bdbuf.c b/cpukit/libblock/src/bdbuf.c
index bae57e2adf..8ea8cbada5 100644
--- a/cpukit/libblock/src/bdbuf.c
+++ b/cpukit/libblock/src/bdbuf.c
@@ -2749,10 +2749,16 @@ rtems_bdbuf_swapout_processing (unsigned long timer_delta,
{
rtems_bdbuf_swapout_worker* worker;
bool transfered_buffers = false;
+ bool sync_active;
rtems_bdbuf_lock_cache ();
/*
+ * To set this to true you need the cache and the sync lock.
+ */
+ sync_active = bdbuf_cache.sync_active;
+
+ /*
* If a sync is active do not use a worker because the current code does not
* cleaning up after. We need to know the buffers have been written when
* syncing to release sync lock and currently worker threads do not return to
@@ -2761,7 +2767,7 @@ rtems_bdbuf_swapout_processing (unsigned long timer_delta,
* lock. The simplest solution is to get the main swap out task perform all
* sync operations.
*/
- if (bdbuf_cache.sync_active)
+ if (sync_active)
worker = NULL;
else
{
@@ -2773,14 +2779,14 @@ rtems_bdbuf_swapout_processing (unsigned long timer_delta,
rtems_chain_initialize_empty (&transfer->bds);
transfer->dd = BDBUF_INVALID_DEV;
- transfer->syncing = bdbuf_cache.sync_active;
+ transfer->syncing = sync_active;
/*
* When the sync is for a device limit the sync to that device. If the sync
* is for a buffer handle process the devices in the order on the sync
* list. This means the dev is BDBUF_INVALID_DEV.
*/
- if (bdbuf_cache.sync_active)
+ if (sync_active)
transfer->dd = bdbuf_cache.sync_device;
/*
@@ -2799,7 +2805,7 @@ rtems_bdbuf_swapout_processing (unsigned long timer_delta,
rtems_bdbuf_swapout_modified_processing (&transfer->dd,
&bdbuf_cache.modified,
&transfer->bds,
- bdbuf_cache.sync_active,
+ sync_active,
update_timers,
timer_delta);
@@ -2830,7 +2836,7 @@ rtems_bdbuf_swapout_processing (unsigned long timer_delta,
transfered_buffers = true;
}
- if (bdbuf_cache.sync_active && !transfered_buffers)
+ if (sync_active && !transfered_buffers)
{
rtems_id sync_requester;
rtems_bdbuf_lock_cache ();