This is an automated email from the ASF dual-hosted git repository. acassis pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit ad364be8188523591229458f19743a0f9d7b66b7 Author: Xiang Xiao <[email protected]> AuthorDate: Fri Sep 11 02:18:38 2026 +0800 fs/aio: rework lio_listio() with a lock-protected request list Previously, lio_listio() called aio_read()/aio_write() to submit the I/O and only then initialized the per-request notification state (aio_priv based), so a worker thread could complete an operation before that state was set up (thread-unsafe), and the completion notification hijacked the per-request sigevent machinery. Rework the implementation: lio_listio() now links every aiocb of the batch into a list (lio_link) before any I/O is submitted. When an operation completes, aio_signal() removes its node from the list under aio_lock() and delivers the lio_listio completion notification only when the list becomes empty. The unused aio_priv field is replaced by the lio_link/lio_sigevent/lio_sigwork fields in struct aiocb. Co-developed-by: wushenhui <[email protected]> Signed-off-by: wushenhui <[email protected]> Signed-off-by: Xiang Xiao <[email protected]> --- fs/aio/aio_fsync.c | 1 - fs/aio/aio_read.c | 1 - fs/aio/aio_signal.c | 26 ++++++ fs/aio/aio_write.c | 1 - fs/aio/lio_listio.c | 246 ++++++++-------------------------------------------- include/aio.h | 25 +++--- 6 files changed, 78 insertions(+), 222 deletions(-) diff --git a/fs/aio/aio_fsync.c b/fs/aio/aio_fsync.c index cc4ad40dbbb..f81a8a60a4c 100644 --- a/fs/aio/aio_fsync.c +++ b/fs/aio/aio_fsync.c @@ -205,7 +205,6 @@ int aio_fsync(int op, FAR struct aiocb *aiocbp) 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. diff --git a/fs/aio/aio_read.c b/fs/aio/aio_read.c index b3e366215e2..9cdb670d004 100644 --- a/fs/aio/aio_read.c +++ b/fs/aio/aio_read.c @@ -253,7 +253,6 @@ int aio_read(FAR struct aiocb *aiocbp) 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. diff --git a/fs/aio/aio_signal.c b/fs/aio/aio_signal.c index 65ca20484ce..c8c734d12dd 100644 --- a/fs/aio/aio_signal.c +++ b/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) diff --git a/fs/aio/aio_write.c b/fs/aio/aio_write.c index 0ba11dc357e..c21c0ef361b 100644 --- a/fs/aio/aio_write.c +++ b/fs/aio/aio_write.c @@ -290,7 +290,6 @@ int aio_write(FAR struct aiocb *aiocbp) 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. diff --git a/fs/aio/lio_listio.c b/fs/aio/lio_listio.c index 42b6f3cfa9c..c2e8610eb53 100644 --- a/fs/aio/lio_listio.c +++ b/fs/aio/lio_listio.c @@ -1,5 +1,5 @@ /**************************************************************************** - * libs/libc/aio/lio_listio.c + * fs/aio/lio_listio.c * * SPDX-License-Identifier: Apache-2.0 * @@ -36,25 +36,10 @@ #include <nuttx/signal.h> #include <nuttx/sched.h> -#include "libc.h" #include "aio/aio.h" #ifdef CONFIG_FS_AIO -/**************************************************************************** - * Private Types - ****************************************************************************/ - -struct lio_sighand_s -{ - FAR struct aiocb * const *list; /* List of I/O operations */ - FAR struct sigevent sig; /* Describes how to signal the caller */ - int nent; /* Number or elements in list[] */ - pid_t pid; /* ID of client */ - sigset_t oprocmask; /* sigprocmask to restore */ - struct sigaction oact; /* Signal handler to restore */ -}; - /**************************************************************************** * Private Functions ****************************************************************************/ @@ -125,173 +110,6 @@ static int lio_checkio(FAR struct aiocb * const *list, int nent) return ret; } -/**************************************************************************** - * Name: lio_sighandler - * - * Description: - * Handle the SIGPOLL signal. - * - * Input Parameters: - * signo - The number of the signal that we caught (SIGPOLL) - * info - Information accompanying the signal - * context - Not used in NuttX - * - * Returned Value: - * None - * - ****************************************************************************/ - -static void lio_sighandler(int signo, siginfo_t *info, void *ucontext) -{ - FAR struct aiocb *aiocbp; - FAR struct lio_sighand_s *sighand; - int ret; - - DEBUGASSERT(signo == SIGPOLL && info); - - /* The info structure should contain a pointer to the AIO control block */ - - aiocbp = (FAR struct aiocb *)info->si_value.sival_ptr; - DEBUGASSERT(aiocbp && aiocbp->aio_result != -EINPROGRESS); - - /* Recover our private data from the AIO control block */ - - sighand = (FAR struct lio_sighand_s *)aiocbp->aio_priv; - DEBUGASSERT(sighand && sighand->list); - aiocbp->aio_priv = NULL; - - /* Check if all of the pending I/O has completed */ - - ret = lio_checkio(sighand->list, sighand->nent); - if (ret != -EINPROGRESS) - { - /* All pending I/O has completed */ - - /* Restore the signal handler */ - - sigaction(SIGPOLL, &sighand->oact, NULL); - - /* Restore the sigprocmask */ - - sigprocmask(SIG_SETMASK, &sighand->oprocmask, NULL); - - /* Signal the client */ - - DEBUGVERIFY(nxsig_notification(sighand->pid, &sighand->sig, - SI_ASYNCIO, &aiocbp->aio_sigwork)); - - /* And free the container */ - - lib_free(sighand); - } -} - -/**************************************************************************** - * Name: lio_sigsetup - * - * Description: - * Setup a signal handler to detect when until all I/O completes. - * - * Input Parameters: - * list - The list of I/O operations to be performed - * nent - The number of elements in the list - * - * Returned Value: - * Zero (OK) is returned if all I/O completed successfully; Otherwise, a - * negated errno value is returned corresponding to the first error - * detected. - * - * Assumptions: - * The scheduler is locked and no I/O can complete asynchronously with - * the logic in this function. - * - ****************************************************************************/ - -static int lio_sigsetup(FAR struct aiocb * const *list, int nent, - FAR struct sigevent *sig) -{ - FAR struct aiocb *aiocbp; - struct lio_sighand_s sighand; - sigset_t set; - struct sigaction act; - int status; - int i; - - /* Initialize the allocated structure */ - - memset(&sighand, 0, sizeof(struct lio_sighand_s)); - sighand.list = list; - sighand.sig = *sig; - sighand.nent = nent; - sighand.pid = _SCHED_GETPID(); - - /* Make sure that SIGPOLL is not blocked */ - - sigemptyset(&set); - sigaddset(&set, SIGPOLL); - status = sigprocmask(SIG_UNBLOCK, &set, &sighand.oprocmask); - if (status != OK) - { - int errcode = get_errno(); - ferr("ERROR sigprocmask failed: %d\n", errcode); - DEBUGASSERT(errcode > 0); - return -errcode; - } - - /* Attach our signal handler */ - - finfo("Registering signal handler\n"); - - act.sa_sigaction = lio_sighandler; - act.sa_flags = SA_SIGINFO; - - sigfillset(&act.sa_mask); - sigdelset(&act.sa_mask, SIGPOLL); - - status = sigaction(SIGPOLL, &act, &sighand.oact); - if (status != OK) - { - int errcode = get_errno(); - - ferr("ERROR sigaction failed: %d\n", errcode); - - DEBUGASSERT(errcode > 0); - return -errcode; - } - - /* Save this structure as the private data attached to each aiocb */ - - for (i = 0; i < nent; i++) - { - /* Skip over NULL entries in the list */ - - aiocbp = list[i]; - if (aiocbp) - { - FAR void *priv = NULL; - - /* Check if I/O is pending for this entry */ - - if (aiocbp->aio_result == -EINPROGRESS) - { - priv = lib_zalloc(sizeof(struct lio_sighand_s)); - if (!priv) - { - ferr("ERROR: lib_zalloc failed\n"); - return -ENOMEM; - } - - memcpy(priv, (FAR void *)&sighand, - sizeof(struct lio_sighand_s)); - } - - aiocbp->aio_priv = priv; - } - } - - return OK; -} - /**************************************************************************** * Name: lio_waitall * @@ -506,6 +324,7 @@ int lio_listio(int mode, FAR struct aiocb * const list[], int nent, FAR struct sigevent *sig) { FAR struct aiocb *aiocbp = NULL; + struct list_node head; int nqueued; int errcode; int retcode; @@ -524,12 +343,20 @@ int lio_listio(int mode, FAR struct aiocb * const list[], int nent, nqueued = 0; /* No I/O operations yet queued */ ret = OK; /* Assume success */ - /* Lock the scheduler so that no I/O events can complete on the worker - * thread until we set our wait set up. Pre-emption will, of course, be - * re-enabled while we are waiting for the signal. - */ + if (mode == LIO_NOWAIT && sig) + { + list_initialize(&head); - sched_lock(); + for (i = 0; i < nent; i++) + { + aiocbp = list[i]; + if (aiocbp && aiocbp->aio_lio_opcode != LIO_NOP) + { + list_add_head(&head, &(aiocbp->lio_link)); + aiocbp->lio_sigevent = *sig; + } + } + } /* Submit each asynchronous I/O operation in the list, skipping over NULL * entries. @@ -581,6 +408,14 @@ int lio_listio(int mode, FAR struct aiocb * const list[], int nent, aiocbp->aio_result = -errcode; ret = ERROR; } + + if (status < 0 || aiocbp->aio_result == -EBADF || + aiocbp->aio_result == -EINVAL) + { + aio_lock(); + list_delete(&aiocbp->lio_link); + aio_unlock(); + } else { /* Increment the count of successfully queue operations */ @@ -649,35 +484,31 @@ int lio_listio(int mode, FAR struct aiocb * const list[], int nent, else if (sig != NULL) { - if (nqueued > 0) + aio_lock(); + status = list_is_empty(&head); + list_delete(&head); + aio_unlock(); + + if (status) { - /* Setup a signal handler to detect when until all I/O completes. */ + /* head is empty meaning all I/O completed before head was + * removed, so manually signal the client + */ - status = lio_sigsetup(list, nent, sig); + status = nxsig_notification(nxsched_getpid(), + &aiocbp->lio_sigevent, + SI_ASYNCIO, + &aiocbp->lio_sigwork); if (status < 0 && ret == OK) { - /* Something bad happened while setting up the signal and this - * is the first error to be reported. + /* Something bad happened while signal the client and + * this is the first error to be reported. */ retcode = -status; ret = ERROR; } } - else - { - status = nxsig_notification(_SCHED_GETPID(), sig, - SI_ASYNCIO, &aiocbp->aio_sigwork); - if (status < 0 && ret == OK) - { - /* Something bad happened while performing the notification - * and this is the first error to be reported. - */ - - retcode = -status; - ret = ERROR; - } - } } /* Case 3: mode == LIO_NOWAIT and sig == NULL @@ -685,7 +516,6 @@ int lio_listio(int mode, FAR struct aiocb * const list[], int nent, * Just return now. */ - sched_unlock(); if (ret < 0) { set_errno(retcode); diff --git a/include/aio.h b/include/aio.h index 5f508028eec..81dcddd962b 100644 --- a/include/aio.h +++ b/include/aio.h @@ -32,6 +32,7 @@ #include <sys/types.h> #include <time.h> +#include <nuttx/list.h> #include <nuttx/signal.h> #include <nuttx/wqueue.h> @@ -133,7 +134,9 @@ struct aiocb struct sigwork_s aio_sigwork; /* Signal work */ volatile ssize_t aio_result; /* Support for aio_error() and aio_return() */ - FAR void *aio_priv; /* Used by signal handlers */ + struct list_node lio_link; /* Make list of aiocb for lio_listio() */ + struct sigevent lio_sigevent; /* Sigevent for lio_listio() */ + struct sigwork_s lio_sigwork; /* Signal work for lio_listio() */ }; /**************************************************************************** @@ -152,16 +155,16 @@ extern "C" * Public Function Prototypes ****************************************************************************/ -int aio_cancel(int fildes, FAR struct aiocb *aiocbp); -int aio_error(FAR const struct aiocb *aiocbp); -int aio_fsync(int op, FAR struct aiocb *aiocbp); -int aio_read(FAR struct aiocb *aiocbp); -ssize_t aio_return(FAR struct aiocb *aiocbp); -int aio_suspend(FAR const struct aiocb * const list[], int nent, - FAR const struct timespec *timeout); -int aio_write(FAR struct aiocb *aiocbp); -int lio_listio(int mode, FAR struct aiocb * const list[], int nent, - FAR struct sigevent *sig); +int aio_cancel(int, FAR struct aiocb *); +int aio_error(FAR const struct aiocb *); +int aio_fsync(int, FAR struct aiocb *); +int aio_read(FAR struct aiocb *); +ssize_t aio_return(FAR struct aiocb *); +int aio_suspend(FAR const struct aiocb * const[], int, + FAR const struct timespec *); +int aio_write(FAR struct aiocb *); +int lio_listio(int, FAR struct aiocb *restrict const[restrict], int, + FAR struct sigevent *restrict); #undef EXTERN #ifdef __cplusplus
