Code review feedback.

src/llfs/ioring_log_config.hpp

@@ -68,9 +68,11 @@ struct IoRingLogConfig {
 
   /** \brief Constructs and returns a config based on a given logical size.
    *
-   * The block size is assumed to be IoRingLogConfig::kDefaultBlockSize unless otherwise specified.
-   * The physical offset is assumed to be 0 unless otherwise specified.  In all cases physical size
-   * is derived from logical size and block size.
+   * The block size defaults to IoRingLogConfig::kDefaultBlockSize.
+   * The physical offset defaults to 0.
+   * In all cases physical size is derived from logical size and block size.
+   *
+   * \return the new IoRingLogConfig
    */
   static IoRingLogConfig from_logical_size(u64 logical_size, Optional<usize> opt_block_size = None,
                                            Optional<i64> opt_physical_offset = None);

src/llfs/ioring_log_device.hpp

@@ -69,7 +69,14 @@ class IoRingLogDeviceFactory : public LogDeviceFactory
 
     BATT_ASSIGN_OK_RESULT(DefaultIoRingLogDeviceStorage storage,
                           DefaultIoRingLogDeviceStorage::make_new(
-                              MaxQueueDepth{calculate.queue_depth() * 2}, this->fd_));
+                              MaxQueueDepth{
+                                  calculate.queue_depth() * 2
+                                  //
+                                  // Double the number of flush ops, to give us some margin so the
+                                  // rings don't ever run out of space (TODO [tastolfi 2024-05-14]
+                                  // this seems excessive; investigate lowering this)
+                              },
+                              this->fd_));
 
     this->fd_ = -1;
 

src/llfs/ioring_log_driver.hpp

@@ -108,19 +108,7 @@ class BasicIoRingLogDriver
     return this->flush_pos_.get_value();
   }
 
-  StatusOr<slot_offset_type> await_flush_pos(slot_offset_type flush_pos)
-  {
-    StatusOr<slot_offset_type> status_or = await_slot_offset(flush_pos, this->flush_pos_);
-
-    // If the flush_pos Watch has been closed, it may indicate there was an I/O error; check the
-    // LogStorageDriverContext and report any error status we find.
-    //
-    if (status_or.status() == batt::StatusCode::kClosed) {
-      BATT_REQUIRE_OK(this->context_.get_error_status());
-    }
-
-    return status_or;
-  }
+  StatusOr<slot_offset_type> await_flush_pos(slot_offset_type flush_pos);
 
   //----
 
@@ -276,7 +264,7 @@ class BasicIoRingLogDriver
    */
   void storage_work_started();
 
-  /** \brief Signals that storage I/O work is now the in process of shutting down.  The first time
+  /** \brief Signals that storage I/O work is now in the process of shutting down.  The first time
    * this is called (after this->storage_work_started()), calls this->storage_.on_work_finished() to
    * decrement the work count, allowing the storage I/O event loop to exit once all activity has
    * completed.

src/llfs/ioring_log_driver.ipp

@@ -193,6 +193,24 @@ inline Status BasicIoRingLogDriver<FlushOpImpl, StorageT>::set_trim_pos(slot_off
   return OkStatus();
 }
 
+//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- -  -  -   -
+//
+template <template <typename> class FlushOpImpl, typename StorageT>
+inline StatusOr<slot_offset_type> BasicIoRingLogDriver<FlushOpImpl, StorageT>::await_flush_pos(
+    slot_offset_type flush_pos)
+{
+  StatusOr<slot_offset_type> status_or = await_slot_offset(flush_pos, this->flush_pos_);
+
+  // If the flush_pos Watch has been closed, it may indicate there was an I/O error; check the
+  // LogStorageDriverContext and report any error status we find.
+  //
+  if (status_or.status() == batt::StatusCode::kClosed) {
+    BATT_REQUIRE_OK(this->context_.get_error_status());
+  }
+
+  return status_or;
+}
+
 //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- -  -  -   -
 //
 template <template <typename> class FlushOpImpl, typename StorageT>