fs/aio: rework lio_listio() and fix AIO crashes and POSIX conformance

fs/aio/CMakeLists.txt

@@ -31,6 +31,7 @@ if(CONFIG_FS_AIO)
             aio_queue.c
             aio_read.c
             aio_signal.c
-            aio_write.c)
+            aio_write.c
+            lio_listio.c)
 
 endif()

fs/aio/Kconfig

@@ -11,6 +11,18 @@ config FS_AIO
 		Enable support for asynchronous I/O.  This selection enables the
 		interfaces declared in include/aio.h.
 
+config FS_AIO_LISTIO_MAX
+	int "Maximum number of AIO operations in listio"
+	default 256
+	---help---
+		This option sets the maximum number of asynchronous I/O (AIO) operations
+		that can be submitted in a single call to lio_listio().
+
+		This value defines the upper limit for the 'nent' parameter in
+		lio_listio(mode, aiocb_list, nent, sevp). Increasing this value allows
+		more operations to be queued.
+		Default: 10
+
 if FS_AIO
 
 config FS_NAIOC

fs/aio/Make.defs

@@ -25,7 +25,7 @@ ifeq ($(CONFIG_FS_AIO),y)
 # Add the asynchronous I/O C files to the build
 
 CSRCS += aio_cancel.c aioc_contain.c aio_fsync.c aio_initialize.c
-CSRCS += aio_queue.c aio_read.c aio_signal.c aio_write.c
+CSRCS += aio_queue.c aio_read.c aio_signal.c aio_write.c lio_listio.c
 
 # Add the asynchronous I/O directory to the build
 

fs/aio/aio.h

@@ -239,6 +239,9 @@ int aio_queue(FAR struct aio_container_s *aioc, worker_t worker);
 
 int aio_signal(pid_t pid, FAR struct aiocb *aiocbp);
 
+int aio_read_internal(FAR struct aiocb *aiocbp);
+int aio_write_internal(FAR struct aiocb *aiocbp);
+
 #undef EXTERN
 #if defined(__cplusplus)
 }

fs/aio/aio_cancel.c

@@ -32,6 +32,7 @@
 #include <errno.h>
 
 #include <nuttx/wqueue.h>
+#include <nuttx/fs/fs.h>
 
 #include "aio/aio.h"
 
@@ -83,18 +84,23 @@
 
 int aio_cancel(int fildes, FAR struct aiocb *aiocbp)
 {
-  if (fildes < 0)
-    {
-      set_errno(EBADF);
-      return ERROR;
-    }
-
   FAR struct aio_container_s *aioc;
   FAR struct aio_container_s *next;
+  FAR struct file            *filep;
+
   pid_t pid;
   int status;
   int ret;
 
+  ret = file_get(fildes, &filep);
+  if (ret < 0)
+    {
+      set_errno(-ret);
+      return ERROR;
+    }
+
+  file_put(filep);
+
   /* Check if a non-NULL aiocbp was provided */
 
   /* Lock the scheduler so that no I/O events can complete on the worker
@@ -165,14 +171,16 @@ int aio_cancel(int fildes, FAR struct aiocb *aiocbp)
     {
       /* No aiocbp.. cancel all outstanding I/O for the fildes */
 
-      next = (FAR struct aio_container_s *)g_aio_pending.head;
-      do
+      for (aioc = (FAR struct aio_container_s *)g_aio_pending.head;
+           aioc;
+           aioc = next)
         {
-          /* Find the next container with this AIO control block */
+          next = (FAR struct aio_container_s *)aioc->aioc_link.flink;
 
-          for (aioc = next;
-               aioc && aioc->aioc_aiocbp->aio_fildes != fildes;
-               aioc = (FAR struct aio_container_s *)aioc->aioc_link.flink);
+          if (aioc->aioc_aiocbp->aio_fildes != fildes)
+            {
+              continue;
+            }
 
           /* Did we find the container?  We should; the aio_result says
            * that the transfer is pending.  If not we return AIO_ALLDONE.
@@ -195,8 +203,6 @@ int aio_cancel(int fildes, FAR struct aiocb *aiocbp)
                    * transfers
                    */
 
-                  next   =
-                    (FAR struct aio_container_s *)aioc->aioc_link.flink;
                   pid    = aioc->aioc_pid;
                   aiocbp = aioc_decant(aioc);
                   DEBUGASSERT(aiocbp);
@@ -217,7 +223,6 @@ int aio_cancel(int fildes, FAR struct aiocb *aiocbp)
                 }
             }
         }
-      while (aioc);
     }
 
   aio_unlock();

fs/aio/aio_fsync.c

@@ -79,7 +79,7 @@ static void aio_fsync_worker(FAR void *arg)
 #ifdef CONFIG_PRIORITY_INHERITANCE
   prio   = aioc->aioc_prio;
 #endif
-  aiocbp = aioc_decant(aioc);
+  aiocbp = aioc->aioc_aiocbp;
 
   /* Perform the fsync using aioc_filep */
 
@@ -97,6 +97,7 @@ static void aio_fsync_worker(FAR void *arg)
   /* Signal the client */
 
   aio_signal(pid, aiocbp);
+  aioc_decant(aioc);
 
 #ifdef CONFIG_PRIORITY_INHERITANCE
   /* Restore the low priority worker thread default priority */
@@ -193,19 +194,29 @@ int aio_fsync(int op, FAR struct aiocb *aiocbp)
   FAR struct aio_container_s *aioc;
   int ret;
 
-  if (op != O_SYNC)
+  /* SUSv2 / POSIX Issue 5 specified that a NULL aiocbp produces no
+   * status through aiocbp and no completion signal. POSIX Issue 6 removed
+   * that special case, so reject NULL defensively.
+   */
+
+  if (op != O_SYNC || aiocbp == NULL)
     {
       set_errno(EINVAL);
       return ERROR;
     }
 
-  DEBUGASSERT(aiocbp);
-
   /* The result -EINPROGRESS means that the transfer has not yet completed */
 
   sigwork_init(&aiocbp->aio_sigwork);
   aiocbp->aio_result = -EINPROGRESS;
-  aiocbp->aio_priv   = NULL;
+
+  /* Clear lio_link so list_in_list() returns false and aio_signal() skips
+   * the lio_listio path; list_initialize() would leave prev non-NULL, so
+   * list_in_list() wrongly returns true and aio_signal() notifies through
+   * the uninitialized lio_sigevent/lio_sigwork.
+   */
+
+  list_clear_node(&aiocbp->lio_link);
 
   /* Create a container for the AIO control block.  This may cause us to
    * block if there are insufficient resources to satisfy the request.

fs/aio/aio_read.c

@@ -79,7 +79,7 @@ static void aio_read_worker(FAR void *arg)
 #ifdef CONFIG_PRIORITY_INHERITANCE
   prio   = aioc->aioc_prio;
 #endif
-  aiocbp = aioc_decant(aioc);
+  aiocbp = aioc->aioc_aiocbp;
 
   /* Perform the file read using:
    *
@@ -106,6 +106,7 @@ static void aio_read_worker(FAR void *arg)
   /* Signal the client */
 
   aio_signal(pid, aiocbp);
+  aioc_decant(aioc);
 
 #ifdef CONFIG_PRIORITY_INHERITANCE
   /* Restore the low priority worker thread default priority */
@@ -214,46 +215,30 @@ static void aio_read_worker(FAR void *arg)
  *
  ****************************************************************************/
 
-int aio_read(FAR struct aiocb *aiocbp)
+int aio_read_internal(FAR struct aiocb *aiocbp)
 {
   FAR struct aio_container_s *aioc;
   int ret;
 
   DEBUGASSERT(aiocbp);
 
-  if (aiocbp->aio_reqprio < 0)
-    {
-      set_errno(EINVAL);
-      return ERROR;
-    }
-
-  if (aiocbp->aio_fildes < 0)
-    {
-      /* the EBADF should be collected by aio_error(), we need return OK at
-       * here
-       */
-
-      aiocbp->aio_result = -EBADF;
-      return OK;
-    }
-
   /* for aio_read, the aio_offset should be large or equal than 0 */
 
-  if (aiocbp->aio_offset < 0)
+  if (aiocbp->aio_offset < 0 || aiocbp->aio_reqprio < 0)
     {
-      /* the EINVAL should be collected by aio_error(), we need to return OK
-       * here
+      /* the EINVAL should be collected by aio_error(), we need to return
+       * ERROR here
        */
 
       aiocbp->aio_result = -EINVAL;
-      return OK;
+      set_errno(EINVAL);
+      return ERROR;
     }
 
   /* The result -EINPROGRESS means that the transfer has not yet completed */
 
   sigwork_init(&aiocbp->aio_sigwork);
   aiocbp->aio_result = -EINPROGRESS;
-  aiocbp->aio_priv   = NULL;
 
   /* Create a container for the AIO control block.  This may cause us to
    * block if there are insufficient resources to satisfy the request.
@@ -265,7 +250,7 @@ int aio_read(FAR struct aiocb *aiocbp)
       /* The errno has already been set (probably EBADF) */
 
       aiocbp->aio_result = -get_errno();
-      return ERROR;
+      return OK;
     }
 
   /* Defer the work to the worker thread */
@@ -282,4 +267,20 @@ int aio_read(FAR struct aiocb *aiocbp)
   return OK;
 }
 
+int aio_read(FAR struct aiocb *aiocbp)
+{
+  if (aiocbp == NULL)
+    {
+      set_errno(EINVAL);
+      return ERROR;
+    }
+
+  /* Clear lio_link so aio_signal() skips the lio_listio path (see
+   * aio_fsync.c); list_initialize() would wrongly leave prev non-NULL.
+   */
+
+  list_clear_node(&aiocbp->lio_link);
+  return aio_read_internal(aiocbp);
+}
+
 #endif /* CONFIG_FS_AIO */

fs/aio/aio_signal.c

@@ -98,6 +98,32 @@ int aio_signal(pid_t pid, FAR struct aiocb *aiocbp)
         }
     }
 
+  if (list_in_list(&aiocbp->lio_link))
+    {
+      /* This I/O is queued by lio_listio, remove this I/O from the list,
+       * signal the client when all I/O is completed
+       */
+
+      aio_lock();
+      status = list_is_empty(&aiocbp->lio_link);
+      list_delete(&aiocbp->lio_link);
+      aio_unlock();
+
+      if (status)
+        {
+          status = nxsig_notification(pid, &aiocbp->lio_sigevent, SI_ASYNCIO,
+                                      &aiocbp->lio_sigwork);
+          if (status < 0)
+            {
+              ferr("ERROR: nxsig_notification failed: %d\n", status);
+              if (ret >= OK)
+                {
+                  ret = status;
+                }
+            }
+        }
+    }
+
   /* Make sure that errno is set correctly on return */
 
   if (ret < 0)