From 71cf3e9d2f67f8c73abf8677ed92cbcc4008928d Mon Sep 17 00:00:00 2001 From: Sebastian Huber Date: Wed, 20 Feb 2013 13:30:43 +0100 Subject: libblock: Do resource allocation in one place All resource allocations take place in rtems_bdbuf_init() now. After rtems_bdbuf_init() no fatal errors can happen due to configuration errors or resource limits. This makes it easier to detect configuration errors for users. --- cpukit/libblock/src/bdbuf.c | 268 +++++++++++++++------------ testsuites/fstests/fsbdpart01/init.c | 16 -- testsuites/fstests/support/ramdisk_support.c | 17 -- 3 files changed, 153 insertions(+), 148 deletions(-) diff --git a/cpukit/libblock/src/bdbuf.c b/cpukit/libblock/src/bdbuf.c index 57cd1684d3..d2b4238aef 100644 --- a/cpukit/libblock/src/bdbuf.c +++ b/cpukit/libblock/src/bdbuf.c @@ -58,7 +58,7 @@ typedef struct rtems_bdbuf_swapout_transfer rtems_chain_control bds; /**< The transfer list of BDs. */ rtems_disk_device *dd; /**< The device the transfer is for. */ bool syncing; /**< The data is a sync'ing. */ - rtems_blkdev_request* write_req; /**< The write request array. */ + rtems_blkdev_request write_req; /**< The write request. */ } rtems_bdbuf_swapout_transfer; /** @@ -93,8 +93,8 @@ typedef struct rtems_bdbuf_cache bool swapout_enabled; /**< Swapout is only running if * enabled. Set to false to kill the * swap out task. It deletes itself. */ - rtems_chain_control swapout_workers; /**< The work threads for the swapout - * task. */ + rtems_chain_control swapout_free_workers; /**< The work threads for the swapout + * task. */ rtems_bdbuf_buffer* bds; /**< Pointer to table of buffer * descriptors. */ @@ -129,6 +129,9 @@ typedef struct rtems_bdbuf_cache rtems_bdbuf_waiters buffer_waiters; /**< Wait for a buffer and no one is * available. */ + rtems_bdbuf_swapout_transfer *swapout_transfer; + rtems_bdbuf_swapout_worker *swapout_workers; + size_t group_count; /**< The number of groups. */ rtems_bdbuf_group* groups; /**< The groups. */ rtems_id read_ahead_task; /**< Read-ahead task */ @@ -148,11 +151,8 @@ typedef enum { RTEMS_BDBUF_FATAL_PREEMPT_RST, RTEMS_BDBUF_FATAL_RA_WAKE_UP, RTEMS_BDBUF_FATAL_RECYCLE, - RTEMS_BDBUF_FATAL_SO_REQ_NOMEM, - RTEMS_BDBUF_FATAL_SO_WK_NOMEM, RTEMS_BDBUF_FATAL_SO_WAKE_1, RTEMS_BDBUF_FATAL_SO_WAKE_2, - RTEMS_BDBUF_FATAL_SO_WK_CREATE, RTEMS_BDBUF_FATAL_STATE_0, RTEMS_BDBUF_FATAL_STATE_2, RTEMS_BDBUF_FATAL_STATE_4, @@ -1280,8 +1280,6 @@ rtems_bdbuf_create_task( rtems_name name, rtems_task_priority priority, rtems_task_priority default_priority, - rtems_task_entry entry, - rtems_task_argument arg, rtems_id *id ) { @@ -1298,8 +1296,85 @@ rtems_bdbuf_create_task( RTEMS_LOCAL | RTEMS_NO_FLOATING_POINT, id); - if (sc == RTEMS_SUCCESSFUL) - sc = rtems_task_start (*id, entry, arg); + return sc; +} + +static rtems_bdbuf_swapout_transfer* +rtems_bdbuf_swapout_transfer_alloc (void) +{ + /* + * @note chrisj The rtems_blkdev_request and the array at the end is a hack. + * I am disappointment at finding code like this in RTEMS. The request should + * have been a rtems_chain_control. Simple, fast and less storage as the node + * is already part of the buffer structure. + */ + size_t transfer_size = sizeof (rtems_bdbuf_swapout_transfer) + + (bdbuf_config.max_write_blocks * sizeof (rtems_blkdev_sg_buffer)); + return calloc (1, transfer_size); +} + +static void +rtems_bdbuf_transfer_done (rtems_blkdev_request* req, rtems_status_code status); + +static void +rtems_bdbuf_swapout_transfer_init (rtems_bdbuf_swapout_transfer* transfer, + rtems_id id) +{ + rtems_chain_initialize_empty (&transfer->bds); + transfer->dd = BDBUF_INVALID_DEV; + transfer->syncing = false; + transfer->write_req.req = RTEMS_BLKDEV_REQ_WRITE; + transfer->write_req.done = rtems_bdbuf_transfer_done; + transfer->write_req.io_task = id; +} + +static size_t +rtems_bdbuf_swapout_worker_size (void) +{ + return sizeof (rtems_bdbuf_swapout_worker) + + (bdbuf_config.max_write_blocks * sizeof (rtems_blkdev_sg_buffer)); +} + +static rtems_task +rtems_bdbuf_swapout_worker_task (rtems_task_argument arg); + +static rtems_status_code +rtems_bdbuf_swapout_workers_create (void) +{ + rtems_status_code sc; + size_t w; + size_t worker_size; + char *worker_current; + + worker_size = rtems_bdbuf_swapout_worker_size (); + worker_current = calloc (1, bdbuf_config.swapout_workers * worker_size); + if (!worker_current) + sc = RTEMS_NO_MEMORY; + + bdbuf_cache.swapout_workers = (rtems_bdbuf_swapout_worker *) worker_current; + + for (w = 0; + sc == RTEMS_SUCCESSFUL && w < bdbuf_config.swapout_workers; + w++, worker_current += worker_size) + { + rtems_bdbuf_swapout_worker *worker = (rtems_bdbuf_swapout_worker *) worker_current; + + sc = rtems_bdbuf_create_task (rtems_build_name('B', 'D', 'o', 'a' + w), + bdbuf_config.swapout_worker_priority, + RTEMS_BDBUF_SWAPOUT_WORKER_TASK_PRIORITY_DEFAULT, + &worker->id); + if (sc == RTEMS_SUCCESSFUL) + { + rtems_bdbuf_swapout_transfer_init (&worker->transfer, worker->id); + + rtems_chain_append_unprotected (&bdbuf_cache.swapout_free_workers, &worker->link); + worker->enabled = true; + + sc = rtems_task_start (worker->id, + rtems_bdbuf_swapout_worker_task, + (rtems_task_argument) worker); + } + } return sc; } @@ -1357,7 +1432,7 @@ rtems_bdbuf_init (void) bdbuf_cache.sync_device = BDBUF_INVALID_DEV; - rtems_chain_initialize_empty (&bdbuf_cache.swapout_workers); + rtems_chain_initialize_empty (&bdbuf_cache.swapout_free_workers); rtems_chain_initialize_empty (&bdbuf_cache.lru); rtems_chain_initialize_empty (&bdbuf_cache.modified); rtems_chain_initialize_empty (&bdbuf_cache.sync); @@ -1469,31 +1544,52 @@ rtems_bdbuf_init (void) } /* - * Create and start swapout task. This task will create and manage the worker - * threads. + * Create and start swapout task. */ + + bdbuf_cache.swapout_transfer = rtems_bdbuf_swapout_transfer_alloc (); + if (!bdbuf_cache.swapout_transfer) + goto error; + bdbuf_cache.swapout_enabled = true; sc = rtems_bdbuf_create_task (rtems_build_name('B', 'S', 'W', 'P'), bdbuf_config.swapout_priority, RTEMS_BDBUF_SWAPOUT_TASK_PRIORITY_DEFAULT, - rtems_bdbuf_swapout_task, - 0, &bdbuf_cache.swapout); if (sc != RTEMS_SUCCESSFUL) goto error; + rtems_bdbuf_swapout_transfer_init (bdbuf_cache.swapout_transfer, bdbuf_cache.swapout); + + sc = rtems_task_start (bdbuf_cache.swapout, + rtems_bdbuf_swapout_task, + (rtems_task_argument) bdbuf_cache.swapout_transfer); + if (sc != RTEMS_SUCCESSFUL) + goto error; + + if (bdbuf_config.swapout_workers > 0) + { + sc = rtems_bdbuf_swapout_workers_create (); + if (sc != RTEMS_SUCCESSFUL) + goto error; + } + if (bdbuf_config.max_read_ahead_blocks > 0) { bdbuf_cache.read_ahead_enabled = true; sc = rtems_bdbuf_create_task (rtems_build_name('B', 'R', 'D', 'A'), bdbuf_config.read_ahead_priority, RTEMS_BDBUF_READ_AHEAD_TASK_PRIORITY_DEFAULT, - rtems_bdbuf_read_ahead_task, - 0, &bdbuf_cache.read_ahead_task); if (sc != RTEMS_SUCCESSFUL) goto error; + + sc = rtems_task_start (bdbuf_cache.read_ahead_task, + rtems_bdbuf_read_ahead_task, + 0); + if (sc != RTEMS_SUCCESSFUL) + goto error; } rtems_bdbuf_unlock_cache (); @@ -1508,9 +1604,29 @@ error: if (bdbuf_cache.swapout != 0) rtems_task_delete (bdbuf_cache.swapout); + if (bdbuf_cache.swapout_workers) + { + char *worker_current = (char *) bdbuf_cache.swapout_workers; + size_t worker_size = rtems_bdbuf_swapout_worker_size (); + size_t w; + + for (w = 0; + w < bdbuf_config.swapout_workers; + w++, worker_current += worker_size) + { + rtems_bdbuf_swapout_worker *worker = (rtems_bdbuf_swapout_worker *) worker_current; + + if (worker->id != 0) { + rtems_task_delete (worker->id); + } + } + } + free (bdbuf_cache.buffers); free (bdbuf_cache.groups); free (bdbuf_cache.bds); + free (bdbuf_cache.swapout_transfer); + free (bdbuf_cache.swapout_workers); rtems_semaphore_delete (bdbuf_cache.buffer_waiters.sema); rtems_semaphore_delete (bdbuf_cache.access_waiters.sema); @@ -2311,8 +2427,8 @@ rtems_bdbuf_swapout_write (rtems_bdbuf_swapout_transfer* transfer) * removed. Merging members of a struct into the first member is * trouble waiting to happen. */ - transfer->write_req->status = RTEMS_RESOURCE_IN_USE; - transfer->write_req->bufnum = 0; + transfer->write_req.status = RTEMS_RESOURCE_IN_USE; + transfer->write_req.bufnum = 0; while ((node = rtems_chain_get_unprotected(&transfer->bds)) != NULL) { @@ -2328,10 +2444,10 @@ rtems_bdbuf_swapout_write (rtems_bdbuf_swapout_transfer* transfer) if (rtems_bdbuf_tracer) printf ("bdbuf:swapout write: bd:%" PRIu32 ", bufnum:%" PRIu32 " mode:%s\n", - bd->block, transfer->write_req->bufnum, + bd->block, transfer->write_req.bufnum, need_continuous_blocks ? "MULTI" : "SCAT"); - if (need_continuous_blocks && transfer->write_req->bufnum && + if (need_continuous_blocks && transfer->write_req.bufnum && bd->block != last_block + media_blocks_per_block) { rtems_chain_prepend_unprotected (&transfer->bds, &bd->link); @@ -2340,8 +2456,8 @@ rtems_bdbuf_swapout_write (rtems_bdbuf_swapout_transfer* transfer) else { rtems_blkdev_sg_buffer* buf; - buf = &transfer->write_req->bufs[transfer->write_req->bufnum]; - transfer->write_req->bufnum++; + buf = &transfer->write_req.bufs[transfer->write_req.bufnum]; + transfer->write_req.bufnum++; buf->user = bd; buf->block = bd->block; buf->length = dd->block_size; @@ -2355,15 +2471,15 @@ rtems_bdbuf_swapout_write (rtems_bdbuf_swapout_transfer* transfer) */ if (rtems_chain_is_empty (&transfer->bds) || - (transfer->write_req->bufnum >= bdbuf_config.max_write_blocks)) + (transfer->write_req.bufnum >= bdbuf_config.max_write_blocks)) write = true; if (write) { - rtems_bdbuf_execute_transfer_request (dd, transfer->write_req, false); + rtems_bdbuf_execute_transfer_request (dd, &transfer->write_req, false); - transfer->write_req->status = RTEMS_RESOURCE_IN_USE; - transfer->write_req->bufnum = 0; + transfer->write_req.status = RTEMS_RESOURCE_IN_USE; + transfer->write_req.bufnum = 0; } } @@ -2541,7 +2657,7 @@ rtems_bdbuf_swapout_processing (unsigned long timer_delta, else { worker = (rtems_bdbuf_swapout_worker*) - rtems_chain_get_unprotected (&bdbuf_cache.swapout_workers); + rtems_chain_get_unprotected (&bdbuf_cache.swapout_free_workers); if (worker) transfer = &worker->transfer; } @@ -2620,34 +2736,6 @@ rtems_bdbuf_swapout_processing (unsigned long timer_delta, return transfered_buffers; } -/** - * Allocate the write request and initialise it for good measure. - * - * @return rtems_blkdev_request* The write reference memory. - */ -static rtems_blkdev_request* -rtems_bdbuf_swapout_writereq_alloc (void) -{ - /* - * @note chrisj The rtems_blkdev_request and the array at the end is a hack. - * I am disappointment at finding code like this in RTEMS. The request should - * have been a rtems_chain_control. Simple, fast and less storage as the node - * is already part of the buffer structure. - */ - rtems_blkdev_request* write_req = - malloc (sizeof (rtems_blkdev_request) + - (bdbuf_config.max_write_blocks * sizeof (rtems_blkdev_sg_buffer))); - - if (!write_req) - rtems_bdbuf_fatal (RTEMS_BDBUF_FATAL_SO_REQ_NOMEM); - - write_req->req = RTEMS_BLKDEV_REQ_WRITE; - write_req->done = rtems_bdbuf_transfer_done; - write_req->io_task = rtems_task_self (); - - return write_req; -} - /** * The swapout worker thread body. * @@ -2670,56 +2758,16 @@ rtems_bdbuf_swapout_worker_task (rtems_task_argument arg) rtems_chain_initialize_empty (&worker->transfer.bds); worker->transfer.dd = BDBUF_INVALID_DEV; - rtems_chain_append_unprotected (&bdbuf_cache.swapout_workers, &worker->link); + rtems_chain_append_unprotected (&bdbuf_cache.swapout_free_workers, &worker->link); rtems_bdbuf_unlock_cache (); } - free (worker->transfer.write_req); free (worker); rtems_task_delete (RTEMS_SELF); } -/** - * Open the swapout worker threads. - */ -static void -rtems_bdbuf_swapout_workers_open (void) -{ - rtems_status_code sc; - size_t w; - - rtems_bdbuf_lock_cache (); - - for (w = 0; w < bdbuf_config.swapout_workers; w++) - { - rtems_bdbuf_swapout_worker* worker; - - worker = malloc (sizeof (rtems_bdbuf_swapout_worker)); - if (!worker) - rtems_bdbuf_fatal (RTEMS_BDBUF_FATAL_SO_WK_NOMEM); - - rtems_chain_append_unprotected (&bdbuf_cache.swapout_workers, &worker->link); - worker->enabled = true; - worker->transfer.write_req = rtems_bdbuf_swapout_writereq_alloc (); - - rtems_chain_initialize_empty (&worker->transfer.bds); - worker->transfer.dd = BDBUF_INVALID_DEV; - - sc = rtems_bdbuf_create_task (rtems_build_name('B', 'D', 'o', 'a' + w), - bdbuf_config.swapout_worker_priority, - RTEMS_BDBUF_SWAPOUT_WORKER_TASK_PRIORITY_DEFAULT, - rtems_bdbuf_swapout_worker_task, - (rtems_task_argument) worker, - &worker->id); - if (sc != RTEMS_SUCCESSFUL) - rtems_bdbuf_fatal (RTEMS_BDBUF_FATAL_SO_WK_CREATE); - } - - rtems_bdbuf_unlock_cache (); -} - /** * Close the swapout worker threads. */ @@ -2730,8 +2778,8 @@ rtems_bdbuf_swapout_workers_close (void) rtems_bdbuf_lock_cache (); - node = rtems_chain_first (&bdbuf_cache.swapout_workers); - while (!rtems_chain_is_tail (&bdbuf_cache.swapout_workers, node)) + node = rtems_chain_first (&bdbuf_cache.swapout_free_workers); + while (!rtems_chain_is_tail (&bdbuf_cache.swapout_free_workers, node)) { rtems_bdbuf_swapout_worker* worker = (rtems_bdbuf_swapout_worker*) node; worker->enabled = false; @@ -2752,15 +2800,10 @@ rtems_bdbuf_swapout_workers_close (void) static rtems_task rtems_bdbuf_swapout_task (rtems_task_argument arg) { - rtems_bdbuf_swapout_transfer transfer; - uint32_t period_in_ticks; - const uint32_t period_in_msecs = bdbuf_config.swapout_period; - uint32_t timer_delta; - - transfer.write_req = rtems_bdbuf_swapout_writereq_alloc (); - rtems_chain_initialize_empty (&transfer.bds); - transfer.dd = BDBUF_INVALID_DEV; - transfer.syncing = false; + rtems_bdbuf_swapout_transfer* transfer = (rtems_bdbuf_swapout_transfer *) arg; + uint32_t period_in_ticks; + const uint32_t period_in_msecs = bdbuf_config.swapout_period; + uint32_t timer_delta; /* * Localise the period. @@ -2772,11 +2815,6 @@ rtems_bdbuf_swapout_task (rtems_task_argument arg) */ timer_delta = period_in_msecs; - /* - * Create the worker threads. - */ - rtems_bdbuf_swapout_workers_open (); - while (bdbuf_cache.swapout_enabled) { rtems_event_set out; @@ -2805,7 +2843,7 @@ rtems_bdbuf_swapout_task (rtems_task_argument arg) */ if (rtems_bdbuf_swapout_processing (timer_delta, update_timers, - &transfer)) + transfer)) { transfered_buffers = true; } @@ -2828,7 +2866,7 @@ rtems_bdbuf_swapout_task (rtems_task_argument arg) rtems_bdbuf_swapout_workers_close (); - free (transfer.write_req); + free (transfer); rtems_task_delete (RTEMS_SELF); } diff --git a/testsuites/fstests/fsbdpart01/init.c b/testsuites/fstests/fsbdpart01/init.c index aea0da08d1..bf6719cd20 100644 --- a/testsuites/fstests/fsbdpart01/init.c +++ b/testsuites/fstests/fsbdpart01/init.c @@ -100,20 +100,6 @@ static void test_logical_disks(const char *const *rdax, bool exists) } } -static void initialize_swapout_task(void) -{ - int fd = open(rda, O_RDONLY); - int rv = 0; - - rtems_test_assert(fd >= 0); - - rv = rtems_disk_fd_sync(fd); - rtems_test_assert(rv == 0); - - rv = close(fd); - rtems_test_assert(rv == 0); -} - static void test_bdpart(void) { rtems_status_code sc = RTEMS_SUCCESSFUL; @@ -128,8 +114,6 @@ static void test_bdpart(void) memset(&actual_format, 0, sizeof(actual_format)); memset(&actual_partitions [0], 0, sizeof(actual_partitions)); - initialize_swapout_task(); - rtems_resource_snapshot_take(&before); for (i = 0; i < PARTITION_COUNT; ++i) { diff --git a/testsuites/fstests/support/ramdisk_support.c b/testsuites/fstests/support/ramdisk_support.c index 17dbb67820..0b6b50d386 100644 --- a/testsuites/fstests/support/ramdisk_support.c +++ b/testsuites/fstests/support/ramdisk_support.c @@ -28,20 +28,6 @@ dev_t dev = 0; -static void initialize_swapout_task(void) -{ - int fd = open(RAMDISK_PATH, O_RDONLY); - int rv = 0; - - rtems_test_assert(fd >= 0); - - rv = rtems_disk_fd_sync(fd); - rtems_test_assert(rv == 0); - - rv = close(fd); - rtems_test_assert(rv == 0); -} - void init_ramdisk (void) { @@ -52,9 +38,6 @@ init_ramdisk (void) rc = ramdisk_register (RAMDISK_BLOCK_SIZE, RAMDISK_BLOCK_COUNT, false, RAMDISK_PATH, &dev); rtems_test_assert (rc == 0); - - - initialize_swapout_task(); } void -- cgit v1.2.3