diff --git a/fs/aio/aio_cancel.c b/fs/aio/aio_cancel.c index ceb31dd4a4106..d715033afae48 100644 --- a/fs/aio/aio_cancel.c +++ b/fs/aio/aio_cancel.c @@ -180,6 +180,13 @@ int aio_cancel(int fildes, FAR struct aiocb *aiocbp) if (aioc) { + /* Advance to the next container now: work_cancel() may report + * that this one is already executing (-ENOENT), and we must + * not examine the same container again in that case. + */ + + next = (FAR struct aio_container_s *)aioc->aioc_link.flink; + /* Yes... attempt to cancel the I/O. There are two * possibilities:* (1) the work has already been started and * is no longer queued, or (2) the work has not been started @@ -195,8 +202,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); diff --git a/fs/aio/aio_fsync.c b/fs/aio/aio_fsync.c index cc4ad40dbbbe7..5484dfa9ed5cf 100644 --- a/fs/aio/aio_fsync.c +++ b/fs/aio/aio_fsync.c @@ -79,7 +79,12 @@ 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 I/O while the container and its file reference are + * still valid. aioc_decant() drops the reference and frees the + * container, so it must be the last use of 'aioc'. + */ /* Perform the fsync using aioc_filep */ @@ -96,6 +101,7 @@ static void aio_fsync_worker(FAR void *arg) /* Signal the client */ + aioc_decant(aioc); aio_signal(pid, aiocbp); #ifdef CONFIG_PRIORITY_INHERITANCE diff --git a/fs/aio/aio_read.c b/fs/aio/aio_read.c index b3e366215e24b..8c2dca953af14 100644 --- a/fs/aio/aio_read.c +++ b/fs/aio/aio_read.c @@ -79,7 +79,12 @@ 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 I/O while the container and its file reference are + * still valid. aioc_decant() drops the reference and frees the + * container, so it must be the last use of 'aioc'. + */ /* Perform the file read using: * @@ -105,6 +110,7 @@ static void aio_read_worker(FAR void *arg) /* Signal the client */ + aioc_decant(aioc); aio_signal(pid, aiocbp); #ifdef CONFIG_PRIORITY_INHERITANCE diff --git a/fs/aio/aio_write.c b/fs/aio/aio_write.c index 0ba11dc357ea9..cefd3bc9abe2d 100644 --- a/fs/aio/aio_write.c +++ b/fs/aio/aio_write.c @@ -82,7 +82,12 @@ static void aio_write_worker(FAR void *arg) #ifdef CONFIG_PRIORITY_INHERITANCE prio = aioc->aioc_prio; #endif - aiocbp = aioc_decant(aioc); + aiocbp = aioc->aioc_aiocbp; + + /* Perform the I/O while the container and its file reference are + * still valid. aioc_decant() drops the reference and frees the + * container, so it must be the last use of 'aioc'. + */ /* Call fcntl(F_GETFL) to get the file open mode. */ @@ -133,6 +138,7 @@ static void aio_write_worker(FAR void *arg) /* Signal the client */ + aioc_decant(aioc); aio_signal(pid, aiocbp); #ifdef CONFIG_PRIORITY_INHERITANCE diff --git a/libs/libc/aio/lio_listio.c b/libs/libc/aio/lio_listio.c index 42b6f3cfa9c03..f4612aec819be 100644 --- a/libs/libc/aio/lio_listio.c +++ b/libs/libc/aio/lio_listio.c @@ -157,7 +157,18 @@ static void lio_sighandler(int signo, siginfo_t *info, void *ucontext) /* Recover our private data from the AIO control block */ sighand = (FAR struct lio_sighand_s *)aiocbp->aio_priv; - DEBUGASSERT(sighand && sighand->list); + if (sighand == NULL) + { + /* This I/O completed before lio_sigsetup() attached our private data + * to it (the completion raced with lio_listio() on another CPU). + * Nothing to do: an entry that still carries the private data will + * finish the job, or lio_sigsetup() already notified the caller. + */ + + return; + } + + DEBUGASSERT(sighand->list); aiocbp->aio_priv = NULL; /* Check if all of the pending I/O has completed */ @@ -165,6 +176,26 @@ static void lio_sighandler(int signo, siginfo_t *info, void *ucontext) ret = lio_checkio(sighand->list, sighand->nent); if (ret != -EINPROGRESS) { + FAR struct aiocb *other; + int i; + + /* Detach the private data of every other entry. A SIGPOLL that was + * dispatched for one of them before we got here still carries this + * handler and will be delivered later, possibly after the caller has + * been notified and its list is gone; with aio_priv cleared such a + * late invocation finds nothing to do. + */ + + for (i = 0; i < sighand->nent; i++) + { + other = sighand->list[i]; + if (other != NULL && other->aio_priv != NULL) + { + lib_free(other->aio_priv); + other->aio_priv = NULL; + } + } + /* All pending I/O has completed */ /* Restore the signal handler */ @@ -211,10 +242,12 @@ static int lio_sigsetup(FAR struct aiocb * const *list, int nent, FAR struct sigevent *sig) { FAR struct aiocb *aiocbp; + FAR struct aiocb *first = NULL; struct lio_sighand_s sighand; sigset_t set; struct sigaction act; int status; + int nprivs = 0; int i; /* Initialize the allocated structure */ @@ -225,14 +258,19 @@ static int lio_sigsetup(FAR struct aiocb * const *list, int nent, sighand.nent = nent; sighand.pid = _SCHED_GETPID(); - /* Make sure that SIGPOLL is not blocked */ + /* Block SIGPOLL until every entry carries its private data. The I/O is + * already in flight and may complete on another CPU at any time; a + * completion signal delivered before the loop below has attached the + * private data would be lost to lio_sighandler(). + */ sigemptyset(&set); sigaddset(&set, SIGPOLL); - status = sigprocmask(SIG_UNBLOCK, &set, &sighand.oprocmask); + status = sigprocmask(SIG_BLOCK, &set, &sighand.oprocmask); if (status != OK) { int errcode = get_errno(); + ferr("ERROR sigprocmask failed: %d\n", errcode); DEBUGASSERT(errcode > 0); return -errcode; @@ -270,6 +308,11 @@ static int lio_sigsetup(FAR struct aiocb * const *list, int nent, { FAR void *priv = NULL; + if (first == NULL) + { + first = aiocbp; + } + /* Check if I/O is pending for this entry */ if (aiocbp->aio_result == -EINPROGRESS) @@ -283,12 +326,40 @@ static int lio_sigsetup(FAR struct aiocb * const *list, int nent, memcpy(priv, (FAR void *)&sighand, sizeof(struct lio_sighand_s)); + nprivs++; } aiocbp->aio_priv = priv; } } + if (nprivs == 0 && first != NULL) + { + /* Every I/O completed while the signal handler was being set up, so + * no lio_sighandler() invocation will see our private data. Finish + * the job here: restore the signal state and notify the caller. + */ + + sigaction(SIGPOLL, &sighand.oact, NULL); + sigprocmask(SIG_SETMASK, &sighand.oprocmask, NULL); + return nxsig_notification(sighand.pid, &sighand.sig, SI_ASYNCIO, + &first->aio_sigwork); + } + + /* Let the completion signals through. Any that arrived meanwhile are + * delivered right here. + */ + + status = sigprocmask(SIG_UNBLOCK, &set, NULL); + if (status != OK) + { + int errcode = get_errno(); + + ferr("ERROR sigprocmask failed: %d\n", errcode); + DEBUGASSERT(errcode > 0); + return -errcode; + } + return OK; } @@ -547,59 +618,61 @@ int lio_listio(int mode, FAR struct aiocb * const list[], int nent, status = OK; switch (aiocbp->aio_lio_opcode) { - case LIO_NOP: - { - /* Mark the do-nothing operation complete */ - - aiocbp->aio_result = OK; - } - break; - - case LIO_READ: - case LIO_WRITE: - { - if (aiocbp->aio_lio_opcode == LIO_READ) - { - /* Submit the asynchronous read operation */ - - status = aio_read(aiocbp); - } - else - { - /* Submit the asynchronous write operation */ - - status = aio_write(aiocbp); - } - - if (status < 0) - { - /* Failed to queue the I/O. Set up the error return. */ - - errcode = get_errno(); - ferr("ERROR: aio_read/write failed: %d\n", errcode); - DEBUGASSERT(errcode > 0); - aiocbp->aio_result = -errcode; - ret = ERROR; - } - else - { - /* Increment the count of successfully queue operations */ - - nqueued++; - } - } - break; - - default: - { - /* Make the invalid operation complete with an error */ - - ferr("ERROR: Unrecognized opcode: %d\n", - aiocbp->aio_lio_opcode); - aiocbp->aio_result = -EINVAL; - ret = ERROR; - } - break; + case LIO_NOP: + { + /* Mark the do-nothing operation complete */ + + aiocbp->aio_result = OK; + } + break; + + case LIO_READ: + case LIO_WRITE: + { + if (aiocbp->aio_lio_opcode == LIO_READ) + { + /* Submit the asynchronous read operation */ + + status = aio_read(aiocbp); + } + else + { + /* Submit the asynchronous write operation */ + + status = aio_write(aiocbp); + } + + if (status < 0) + { + /* Failed to queue the I/O. Set up the error return. */ + + errcode = get_errno(); + ferr("ERROR: aio_read/write failed: %d\n", errcode); + DEBUGASSERT(errcode > 0); + aiocbp->aio_result = -errcode; + ret = ERROR; + } + else + { + /* Increment the count of successfully queued + * operations + */ + + nqueued++; + } + } + break; + + default: + { + /* Make the invalid operation complete with an error */ + + ferr("ERROR: Unrecognized opcode: %d\n", + aiocbp->aio_lio_opcode); + aiocbp->aio_result = -EINVAL; + ret = ERROR; + } + break; } } } @@ -674,8 +747,8 @@ int lio_listio(int mode, FAR struct aiocb * const list[], int nent, * and this is the first error to be reported. */ - retcode = -status; - ret = ERROR; + retcode = -status; + ret = ERROR; } } } diff --git a/sched/wqueue/kwork_cancel.c b/sched/wqueue/kwork_cancel.c index 2329abaf70824..99c7774f48549 100644 --- a/sched/wqueue/kwork_cancel.c +++ b/sched/wqueue/kwork_cancel.c @@ -48,6 +48,7 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue, bool sync, { irqstate_t flags; pid_t self = sync ? nxsched_gettid() : INVALID_PROCESS_ID; + int ret = -ENOENT; if (wqueue == NULL || work == NULL) { @@ -83,8 +84,18 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue, bool sync, { work_timer_reset(wqueue); } + + ret = OK; } + /* Otherwise the work is not queued: either it was never queued or a + * worker has already dequeued it and may be executing its callback + * right now. Only the scan below can tell. Report -ENOENT if the + * work is neither queued nor running, so that callers (e.g. + * aio_cancel()) do not free resources that the callback is still + * using. + */ + if (sync) { for (wndx = 0; wndx < wqueue->nthreads; wndx++) @@ -93,6 +104,7 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue, bool sync, { worker[wndx].wait_count++; sync_wait = &worker[wndx].wait; + ret = OK; break; } } @@ -102,7 +114,7 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue, bool sync, if (sync_wait == NULL) { - return OK; + return ret; } nxsem_wait_uninterruptible(sync_wait); @@ -130,6 +142,8 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue, bool sync, * Zero on success, a negated errno on failure * * -EINVAL - An invalid work queue was specified + * -ENOENT - The work is not queued (and, for the sync variant, not + * running either) * ****************************************************************************/