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

@@ -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
+  rtems_chain_control swapout_free_workers; /**< The work threads for the swapout
-                                          * task. */
+                                             * 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)
+  return sc;
-    sc = rtems_task_start (*id, entry, arg);
+}
+
+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
+   * Create and start swapout task.
-   * threads.
    */
+
+  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.status = RTEMS_RESOURCE_IN_USE;
-    transfer->write_req->bufnum = 0;
+    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];
+        buf = &transfer->write_req.bufs[transfer->write_req.bufnum];
-        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.status = RTEMS_RESOURCE_IN_USE;
-        transfer->write_req->bufnum = 0;
+        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);
+  node = rtems_chain_first (&bdbuf_cache.swapout_free_workers);
-  while (!rtems_chain_is_tail (&bdbuf_cache.swapout_workers, node))
+  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;
+  rtems_bdbuf_swapout_transfer* transfer = (rtems_bdbuf_swapout_transfer *) arg;
-  uint32_t                     period_in_ticks;
+  uint32_t                      period_in_ticks;
-  const uint32_t               period_in_msecs = bdbuf_config.swapout_period;
+  const uint32_t                period_in_msecs = bdbuf_config.swapout_period;
-  uint32_t                     timer_delta;
+  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;
 
   /*
    * 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);
 }

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) {

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