You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "cmcfarlen (via GitHub)" <gi...@apache.org> on 2023/04/21 19:52:57 UTC

[GitHub] [trafficserver] cmcfarlen opened a new pull request, #9630: Make io_uring or thread AIO modes a startup time decision (vs compile time)

cmcfarlen opened a new pull request, #9630:
URL: https://github.com/apache/trafficserver/pull/9630

   merge THREAD and IOURING AIO_MODEs. Add supported_op to io_uring lib.
   move more details into AIOCallbackInternal
   fix io_uring cpp checks
   add config to force io_uring on/off
   fix test_AIO
   add config to force aio to io_uring or threads


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] bryancall commented on a diff in pull request #9630: Make io_uring or thread AIO modes a startup time decision (vs compile time)

Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on code in PR #9630:
URL: https://github.com/apache/trafficserver/pull/9630#discussion_r1187911995


##########
src/traffic_server/traffic_server.cc:
##########
@@ -2007,6 +2036,10 @@ main(int /* argc ATS_UNUSED */, const char **argv)
 
   REC_ReadConfigInteger(thread_max_heartbeat_mseconds, "proxy.config.thread.max_heartbeat_mseconds");
 
+#if TS_USE_LINUX_IO_URING
+  configure_io_uring();
+#endif
+

Review Comment:
   Below on line 2076 it is going to print that it is "Using io_uring for AIO" if the server is running in the thread mode.
   
   ```
   [May  8 14:20:48.641] traffic_server WARNING: <AIO.cc:241 (ink_aio_init)> Using thread for AIO
   [May  8 14:20:48.641] traffic_server NOTE: <traffic_server.cc:2076 (main)> Using io_uring for AIO
   [May  8 14:20:48.641] traffic_server NOTE: <traffic_server.cc:2080 (main)> io_uring: WQ workers - bounded = 128, unbounded = 256574
   ```



##########
iocore/aio/AIO.cc:
##########
@@ -190,35 +182,69 @@ ink_aio_init(ts::ModuleVersion v)
   RecRegisterRawStat(aio_rsb, RECT_PROCESS, "proxy.process.io_uring.completed", RECD_FLOAT, RECP_PERSISTENT,
                      (int)AIO_STAT_IO_URING_COMPLETED, aio_stats_cb);
 #endif
-#if AIO_MODE == AIO_MODE_THREAD
+#if AIO_MODE == AIO_MODE_DEFAULT
   memset(&aio_reqs, 0, MAX_DISKS_POSSIBLE * sizeof(AIO_Reqs *));
   ink_mutex_init(&insert_mutex);
 #endif
   REC_ReadConfigInteger(cache_config_threads_per_disk, "proxy.config.cache.threads_per_disk");
 #if TS_USE_LINUX_NATIVE_AIO
-  Warning("Running with Linux AIO, there are known issues with this feature");
+  Warning(
+    "Running with Linux libaio is deprecated. There are known issues with this feature and it is being replaced with io_uring");
 #endif
 
+#if AIO_MODE == AIO_MODE_DEFAULT
 #if TS_USE_LINUX_IO_URING
-  REC_ReadConfigInteger(aio_io_uring_queue_entries, "proxy.config.aio.io_uring.entries");
-  REC_ReadConfigInteger(aio_io_uring_sq_poll_ms, "proxy.config.aio.io_uring.sq_poll_ms");
-  REC_ReadConfigInteger(aio_io_uring_attach_wq, "proxy.config.aio.io_uring.attach_wq");
-  REC_ReadConfigInteger(aio_io_uring_wq_bounded, "proxy.config.aio.io_uring.wq_workers_bounded");
-  REC_ReadConfigInteger(aio_io_uring_wq_unbounded, "proxy.config.aio.io_uring.wq_workers_unbounded");
-#endif
-}
+  // If the caller specified auto backend, check for config to force a backend
+  if (backend == AIOBackend::AIO_BACKEND_AUTO) {
+    RecString aio_io_uring_force_aio = nullptr;
+    REC_ReadConfigStringAlloc(aio_io_uring_force_aio, "proxy.config.aio.force_aio");
+    if (aio_io_uring_force_aio) {
+      if (strcasecmp(aio_io_uring_force_aio, "auto") == 0) {
+        backend = AIOBackend::AIO_BACKEND_AUTO;
+      } else if (strcasecmp(aio_io_uring_force_aio, "thread") == 0) {
+        // force thread mode
+        backend = AIOBackend::AIO_BACKEND_THREAD;
+      } else if (strcasecmp(aio_io_uring_force_aio, "io_uring") == 0) {
+        // force io_uring mode
+        backend = AIOBackend::AIO_BACKEND_IO_URING;
+      } else {
+        Warning("Invalid value '%s' for proxy.config.aio.force_aio.  autodetecting", aio_io_uring_force_aio);
+      }
 
-int
-ink_aio_start()
-{
-#ifdef AIO_STATS
-  data = new AIOTestData();
-  eventProcessor.schedule_in(data, HRTIME_MSECONDS(100), ET_CALL);
+      ats_free(aio_io_uring_force_aio);
+    }
+  }
+
+  switch (backend) {
+  case AIOBackend::AIO_BACKEND_AUTO: {
+    // detect if io_uring is available and can support the required features
+    auto *ctx = IOUringContext::local_context();
+    if (ctx && ctx->supports_op(IORING_OP_WRITE) && ctx->supports_op(IORING_OP_READ)) {
+      use_io_uring = true;
+    } else {
+      Note("AIO using thread backend as required io_uring ops are not supported");
+      use_io_uring = false;
+    }
+    break;
+  }
+  case AIOBackend::AIO_BACKEND_IO_URING:
+    use_io_uring = true;
+    break;
+  case AIOBackend::AIO_BACKEND_THREAD:
+    use_io_uring = false;
+    break;
+  }
+
+  if (use_io_uring) {
+    Warning("Using io_uring for AIO");

Review Comment:
   Should this be a Note instead of Warning? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] bryancall commented on pull request #9630: Make io_uring or thread AIO modes a startup time decision (vs compile time)

Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on PR #9630:
URL: https://github.com/apache/trafficserver/pull/9630#issuecomment-1539085160

   With this PR we are breaking `--enable-experimental-linux-native-aio`.  Is that what we want to do? When will `--enable-experimental-linux-native-aio` option be removed?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] bryancall commented on a diff in pull request #9630: Make io_uring or thread AIO modes a startup time decision (vs compile time)

Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on code in PR #9630:
URL: https://github.com/apache/trafficserver/pull/9630#discussion_r1187910400


##########
doc/admin-guide/files/records.yaml.en.rst:
##########
@@ -5196,3 +5196,46 @@ Sockets
 
 .. _Traffic Shaping:
                  https://cwiki.apache.org/confluence/display/TS/Traffic+Shaping
+
+IO_URING
+========
+
+.. ts:cv:: CONFIG proxy.config.io_uring.entries INT 32
+
+   Specify the number of entries in each io_uring.  There will be on io_uring instance per thread that uses io_uring
+   for IO.  This parameter is passed to io_uring_queue_init.
+
+.. ts:cv:: CONFIG proxy.config.io_uring.sq_poll_ms INT 0
+
+   If this value is >0 then use submit queue polling mode.  The value will be used to specifiy the sq_thread_idle parameter
+   to io_uring setup. More information about submit queue polling mode can be found here: https://unixism.net/loti/tutorial/sq_poll.html
+
+.. ts:cv:: CONFIG proxy.config.io_uring.attach_wq INT 0
+
+   Set this to 1 if you want io_uring to re-use the same worker queue backend for each thread.
+
+.. ts:cv:: CONFIG proxy.config.io_uring.wq_workers_bounded INT 0
+.. ts:cv:: CONFIG proxy.config.io_uring.wq_workers_unbounded INT 0
+
+   These settings configured the number of threads for the io_uring worker queue backend.  See the manpage for
+   io_uring_register_iowq_max_workers for more information.
+
+AIO
+===
+
+.. ts:cv:: CONFIG proxy.config.aio.force_aio STRING NULL

Review Comment:
   I would change "force_aio" to "mode" or something.  Seems kind of redundant to have aio twice.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] bryancall commented on a diff in pull request #9630: Make io_uring or thread AIO modes a startup time decision (vs compile time)

Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on code in PR #9630:
URL: https://github.com/apache/trafficserver/pull/9630#discussion_r1187902180


##########
doc/admin-guide/files/records.yaml.en.rst:
##########
@@ -5196,3 +5196,46 @@ Sockets
 
 .. _Traffic Shaping:
                  https://cwiki.apache.org/confluence/display/TS/Traffic+Shaping
+
+IO_URING
+========
+
+.. ts:cv:: CONFIG proxy.config.io_uring.entries INT 32
+
+   Specify the number of entries in each io_uring.  There will be on io_uring instance per thread that uses io_uring
+   for IO.  This parameter is passed to io_uring_queue_init.
+
+.. ts:cv:: CONFIG proxy.config.io_uring.sq_poll_ms INT 0
+
+   If this value is >0 then use submit queue polling mode.  The value will be used to specifiy the sq_thread_idle parameter
+   to io_uring setup. More information about submit queue polling mode can be found here: https://unixism.net/loti/tutorial/sq_poll.html
+
+.. ts:cv:: CONFIG proxy.config.io_uring.attach_wq INT 0
+
+   Set this to 1 if you want io_uring to re-use the same worker queue backend for each thread.
+
+.. ts:cv:: CONFIG proxy.config.io_uring.wq_workers_bounded INT 0
+.. ts:cv:: CONFIG proxy.config.io_uring.wq_workers_unbounded INT 0
+
+   These settings configured the number of threads for the io_uring worker queue backend.  See the manpage for
+   io_uring_register_iowq_max_workers for more information.
+
+AIO
+===
+
+.. ts:cv:: CONFIG proxy.config.aio.force_aio STRING NULL
+
+   (Only if io_uring is enabled in the build)
+   Normally, ATS will detect if io_uring can be used for async disk IO.  Using this config item, the AIO mode
+   can instead be specified.  The value can be one of:
+
+   ============ ======================================================================
+   Value        Description
+   ============ ======================================================================
+   ``NULL``     Use the default detection logic

Review Comment:
   Why have a NULL setting in the documentation?  Is someone supposed to set it to NULL to get auto detection?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] cmcfarlen commented on pull request #9630: Make io_uring or thread AIO modes a startup time decision (vs compile time)

Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on PR #9630:
URL: https://github.com/apache/trafficserver/pull/9630#issuecomment-1539102709

   > With this PR we are breaking `--enable-experimental-linux-native-aio`. Is that what we want to do? When will `--enable-experimental-linux-native-aio` option be removed?
   
   I can try to fix it if we want to keep it
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] cmcfarlen merged pull request #9630: Make io_uring or thread AIO modes a startup time decision (vs compile time)

Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen merged PR #9630:
URL: https://github.com/apache/trafficserver/pull/9630


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] cmcfarlen commented on a diff in pull request #9630: Make io_uring or thread AIO modes a startup time decision (vs compile time)

Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on code in PR #9630:
URL: https://github.com/apache/trafficserver/pull/9630#discussion_r1187927387


##########
doc/admin-guide/files/records.yaml.en.rst:
##########
@@ -5196,3 +5196,46 @@ Sockets
 
 .. _Traffic Shaping:
                  https://cwiki.apache.org/confluence/display/TS/Traffic+Shaping
+
+IO_URING
+========
+
+.. ts:cv:: CONFIG proxy.config.io_uring.entries INT 32
+
+   Specify the number of entries in each io_uring.  There will be on io_uring instance per thread that uses io_uring
+   for IO.  This parameter is passed to io_uring_queue_init.
+
+.. ts:cv:: CONFIG proxy.config.io_uring.sq_poll_ms INT 0
+
+   If this value is >0 then use submit queue polling mode.  The value will be used to specifiy the sq_thread_idle parameter
+   to io_uring setup. More information about submit queue polling mode can be found here: https://unixism.net/loti/tutorial/sq_poll.html
+
+.. ts:cv:: CONFIG proxy.config.io_uring.attach_wq INT 0
+
+   Set this to 1 if you want io_uring to re-use the same worker queue backend for each thread.
+
+.. ts:cv:: CONFIG proxy.config.io_uring.wq_workers_bounded INT 0
+.. ts:cv:: CONFIG proxy.config.io_uring.wq_workers_unbounded INT 0
+
+   These settings configured the number of threads for the io_uring worker queue backend.  See the manpage for
+   io_uring_register_iowq_max_workers for more information.
+
+AIO
+===
+
+.. ts:cv:: CONFIG proxy.config.aio.force_aio STRING NULL
+
+   (Only if io_uring is enabled in the build)
+   Normally, ATS will detect if io_uring can be used for async disk IO.  Using this config item, the AIO mode
+   can instead be specified.  The value can be one of:
+
+   ============ ======================================================================
+   Value        Description
+   ============ ======================================================================
+   ``NULL``     Use the default detection logic

Review Comment:
   I'll just default it to auto and leave this out



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] cmcfarlen commented on pull request #9630: Make io_uring or thread AIO modes a startup time decision (vs compile time)

Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on PR #9630:
URL: https://github.com/apache/trafficserver/pull/9630#issuecomment-1520487153

   [approve ci autest]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org