diff --git a/Documentation/testing/ntfc.rst b/Documentation/testing/ntfc.rst index 76b780cbb21bb..47cb6e169bfc0 100644 --- a/Documentation/testing/ntfc.rst +++ b/Documentation/testing/ntfc.rst @@ -22,6 +22,7 @@ CI runtime test cases use NTFC. Configurations with an NTFC runner include: - :doc:`arm/imx6 `: ``sabre-6quad/citest`` - :doc:`arm64/qemu `: ``qemu-armv8a/citest`` - :doc:`arm64/qemu `: ``qemu-armv8a/citest_smp`` +- :doc:`x86_64/qemu `: ``qemu-intel64/citest_smp`` Running a CI Test Target Locally ================================ diff --git a/arch/x86_64/src/intel64/intel64_hpet.c b/arch/x86_64/src/intel64/intel64_hpet.c index 83ff189b1eb80..36512ac6c7175 100644 --- a/arch/x86_64/src/intel64/intel64_hpet.c +++ b/arch/x86_64/src/intel64/intel64_hpet.c @@ -223,6 +223,7 @@ static void intel64_hpet_cmpset(struct intel64_tim_dev_s *dev, uint8_t timer, uint64_t cmp) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + DEBUGASSERT(timer < hpet->timers); intel64_hpet_putreg(hpet, HPET_TCOMP_OFFSET(timer), cmp); } @@ -239,6 +240,7 @@ static uint64_t intel64_hpet_cmpget(struct intel64_tim_dev_s *dev, uint8_t timer) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + DEBUGASSERT(timer < hpet->timers); return intel64_hpet_getreg(hpet, HPET_TCOMP_OFFSET(timer)); } @@ -255,6 +257,7 @@ static uint64_t intel64_hpet_intget(struct intel64_tim_dev_s *dev, uint8_t timer) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + return (intel64_hpet_getreg(hpet, HPET_GISR_OFFSET) & HPET_GISR_TINT(timer)); } @@ -271,6 +274,7 @@ static void intel64_hpet_intack(struct intel64_tim_dev_s *dev, uint8_t timer) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + intel64_hpet_putreg(hpet, HPET_GISR_OFFSET, HPET_GISR_TINT(timer)); } @@ -285,6 +289,7 @@ static void intel64_hpet_intack(struct intel64_tim_dev_s *dev, static uint64_t intel64_hpet_cntget(struct intel64_tim_dev_s *dev) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + return intel64_hpet_getreg(hpet, HPET_MCNTR_OFFSET); } @@ -300,6 +305,7 @@ static void intel64_hpet_cntset(struct intel64_tim_dev_s *dev, uint64_t cntr) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + return intel64_hpet_putreg(hpet, HPET_MCNTR_OFFSET, cntr); } @@ -314,6 +320,7 @@ static void intel64_hpet_cntset(struct intel64_tim_dev_s *dev, static uint32_t intel64_hpet_perget(struct intel64_tim_dev_s *dev) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + return hpet->clk_per_fs; } @@ -383,9 +390,12 @@ static int intel64_hpet_setisr(struct intel64_tim_dev_s *dev, uint8_t timer, if (handler == NULL) { - /* Disable interrupt */ + /* Disable the interrupt but keep the ISR attached. Detaching it here + * installs irq_unexpected_isr(), and an HPET interrupt that is already + * in flight to another CPU then panics the system. A stray interrupt + * is handled as spurious by the oneshot ISR instead. + */ - irq_attach(irq, handler, arg); up_disable_irq(irq); } else diff --git a/arch/x86_64/src/intel64/intel64_oneshot.c b/arch/x86_64/src/intel64/intel64_oneshot.c index a98b8cea55502..27168a329f88b 100644 --- a/arch/x86_64/src/intel64/intel64_oneshot.c +++ b/arch/x86_64/src/intel64/intel64_oneshot.c @@ -100,18 +100,21 @@ static int intel64_oneshot_handler(int irg_num, void * context, void *arg) INTEL64_TIM_ACKINT(oneshot->tch, oneshot->chan); #endif - /* The timer is no longer running */ + /* The timer is no longer running. Only pick up the handler here; + * it is owned by intel64_oneshot_start()/cancel(), which may be + * re-arming the timer on another CPU right now. Clearing it from + * the ISR could leave a re-armed timer without a handler, and the + * next expiry would then jump through a NULL pointer. + */ oneshot->running = false; - - /* Forward the event, clearing out any vestiges */ - oneshot_handler = (oneshot_handler_t)oneshot->handler; - oneshot->handler = NULL; oneshot_arg = (void *)oneshot->arg; - oneshot->arg = NULL; - oneshot_handler(oneshot_arg); + if (oneshot_handler != NULL) + { + oneshot_handler(oneshot_arg); + } } else { @@ -301,10 +304,21 @@ int intel64_oneshot_start(struct intel64_oneshot_s *oneshot, flags = spin_lock_irqsave(&g_oneshot_spin); if (oneshot->running) { - /* Yes.. then cancel it */ + /* Yes.. then stop it. Do NOT call intel64_oneshot_cancel() here: + * it takes g_oneshot_spin, which we already hold, and spinlocks are + * not recursive, so that deadlocks the CPU. Everything else that + * cancel would do (ISR, comparator, interrupt enable) is + * reprogrammed below anyway. + */ tmrinfo("Already running... cancelling\n"); - intel64_oneshot_cancel(oneshot, NULL); + +#ifndef CONFIG_INTEL64_HPET_FSB + INTEL64_TIM_DISABLEINT(oneshot->tch, oneshot->chan); + INTEL64_TIM_SETISR(oneshot->tch, oneshot->chan, NULL, NULL, false); +#endif + + oneshot->running = false; } /* Save the new handler and its argument */ diff --git a/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/config.yaml b/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/config.yaml new file mode 100644 index 0000000000000..cfce28d938844 --- /dev/null +++ b/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/config.yaml @@ -0,0 +1,14 @@ +config: + cwd: './' + +product: + name: "qemu-intel64" + cores: + core0: + name: 'main' + device: 'qemu' + exec_path: 'qemu-system-x86_64' + exec_args: '-m 2G -smp 4 -cpu max -nographic -serial mon:stdio' + uptime: 60 + conf_path: './.config' + elf_path: './nuttx' diff --git a/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/defconfig b/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/defconfig new file mode 100644 index 0000000000000..32b437b429766 --- /dev/null +++ b/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/defconfig @@ -0,0 +1,161 @@ +# +# This file is autogenerated: PLEASE DO NOT EDIT IT. +# +# You can use "make menuconfig" to make any modifications to the installed .config file. +# You can then do "make savedefconfig" to generate a new defconfig file that includes your +# modifications. +# +# CONFIG_ARCH_INTEL64_HAVE_PCID is not set +# CONFIG_ARCH_X86_64_X2APIC is not set +CONFIG_16550_ADDRWIDTH=16 +CONFIG_16550_PCI_UART1=y +CONFIG_16550_PCI_UART1_CLOCK=1843200 +CONFIG_16550_PCI_UART1_DEVICE=0x0002 +CONFIG_16550_PCI_UART1_VENDOR=0x1b36 +CONFIG_16550_PCI_UART=y +CONFIG_16550_UART0=y +CONFIG_16550_UART0_BASE=0x3f8 +CONFIG_16550_UART0_CLOCK=1843200 +CONFIG_16550_UART0_IRQ=36 +CONFIG_16550_UART0_RXBUFSIZE=16 +CONFIG_16550_UART0_SERIAL_CONSOLE=y +CONFIG_16550_UART0_TXBUFSIZE=16 +CONFIG_ALLOW_MIT_COMPONENTS=y +CONFIG_ARCH="x86_64" +CONFIG_ARCH_BOARD="qemu-intel64" +CONFIG_ARCH_BOARD_INTEL64_QEMU=y +CONFIG_ARCH_CHIP="qemu" +CONFIG_ARCH_CHIP_INTEL64_QEMU=y +CONFIG_ARCH_INTEL64_HPET_ALARM=y +CONFIG_ARCH_INTERRUPTSTACK=8192 +CONFIG_ARCH_SETJMP_H=y +CONFIG_ARCH_SIZET_LONG=y +CONFIG_ARCH_X86_64=y +CONFIG_ARCH_X86_64_SSE3=y +CONFIG_ARCH_X86_64_SSE41=y +CONFIG_ARCH_X86_64_SSE42=y +CONFIG_ARCH_X86_64_SSE4A=y +CONFIG_ARCH_X86_64_SSSE3=y +CONFIG_BOARD_LOOPSPERMSEC=999 +CONFIG_BOOT_RUNFROMEXTSRAM=y +CONFIG_BUILTIN=y +CONFIG_CM_MM_TEST=y +CONFIG_CM_MUTEX_TEST=y +CONFIG_CM_PTHREAD_TEST=y +CONFIG_CM_SCHED_TEST=y +CONFIG_CM_SOCKET_TEST=y +CONFIG_CM_TIME_TEST=y +CONFIG_CONSOLE_SYSLOG=y +CONFIG_DEBUG_FEATURES=y +CONFIG_DEBUG_PCI=y +CONFIG_DEBUG_PCI_ERROR=y +CONFIG_DEBUG_PCI_INFO=y +CONFIG_DEBUG_PCI_WARN=y +CONFIG_DEBUG_SYMBOLS=y +CONFIG_DEFAULT_TASK_STACKSIZE=4194304 +CONFIG_DEV_SIMPLE_ADDRENV=y +CONFIG_EVENT_FD=y +CONFIG_EXAMPLES_HELLO=y +CONFIG_EXAMPLES_HIDKBD=y +CONFIG_EXAMPLES_PIPE=y +CONFIG_EXAMPLES_POPEN=y +CONFIG_FS_AIO=y +CONFIG_FS_FAT=y +CONFIG_FS_NAMED_SEMAPHORES=y +CONFIG_FS_NOTIFY=y +CONFIG_FS_PROCFS=y +CONFIG_FS_SHMFS=y +CONFIG_FS_TMPFS=y +CONFIG_HIDKBD_STACKSIZE=10240 +CONFIG_HIDMOUSE_STACKSIZE=10240 +CONFIG_IDLETHREAD_STACKSIZE=4194304 +CONFIG_INIT_ENTRYPOINT="nsh_main" +CONFIG_IOB_ALIGNMENT=64 +CONFIG_IOB_BUFSIZE=2048 +CONFIG_IOB_NBUFFERS=1024 +CONFIG_IOB_NCHAINS=255 +CONFIG_LIBC_LOCALE=y +CONFIG_LIBC_MEMFD_ERROR=y +CONFIG_LIBM=y +CONFIG_NET=y +CONFIG_NETDB_DNSCLIENT=y +CONFIG_NETDEV_LATEINIT=y +CONFIG_NETINIT_DRIPADDR=0xc0a80801 +CONFIG_NETINIT_IPADDR=0x0a000102 +CONFIG_NETUTILS_CODECS=y +CONFIG_NETUTILS_IPERF=y +CONFIG_NETUTILS_TELNETD=y +CONFIG_NETUTILS_TFTPC=y +CONFIG_NETUTILS_WEBCLIENT=y +CONFIG_NET_BROADCAST=y +CONFIG_NET_E1000=y +CONFIG_NET_ETH_PKTSIZE=1514 +CONFIG_NET_ICMP_SOCKET=y +CONFIG_NET_IGC=y +CONFIG_NET_LL_GUARDSIZE=32 +CONFIG_NET_MAX_LISTENPORTS=8 +CONFIG_NET_PKT=y +CONFIG_NET_STATISTICS=y +CONFIG_NET_TCP=y +CONFIG_NET_UDP=y +CONFIG_NSH_BUILTIN_APPS=y +CONFIG_NSH_READLINE=y +CONFIG_PCI=y +CONFIG_PCI_MSIX=y +CONFIG_PCI_QEMU_EDU=y +CONFIG_PCI_QEMU_TEST=y +CONFIG_PIPES=y +CONFIG_PREALLOC_CHILDSTATUS=16 +CONFIG_PRIORITY_INHERITANCE=y +CONFIG_PSEUDOFS_ATTRIBUTES=y +CONFIG_PSEUDOFS_FILE=y +CONFIG_PTHREAD_MUTEX_TYPES=y +CONFIG_PTHREAD_STACK_MIN=4194304 +CONFIG_RAM_SIZE=268435456 +CONFIG_SCHED_CHILD_STATUS=y +CONFIG_SCHED_HAVE_PARENT=y +CONFIG_SCHED_HPWORK=y +CONFIG_SCHED_IRQMONITOR=y +CONFIG_SCHED_LPNTHREADS=1 +CONFIG_SCHED_LPWORK=y +CONFIG_SCHED_THREAD_LOCAL=y +CONFIG_SCHED_TICKLESS=y +CONFIG_SIGNAL_FD=y +CONFIG_SIG_DEFAULT=y +CONFIG_SIG_EVTHREAD=y +CONFIG_SMP=y +CONFIG_START_DAY=3 +CONFIG_START_MONTH=3 +CONFIG_START_YEAR=2011 +CONFIG_SYSTEM_CLE=y +CONFIG_SYSTEM_NSH=y +CONFIG_SYSTEM_POPEN=y +CONFIG_SYSTEM_SETLOGMASK=y +CONFIG_TESTING_ATOMIC=y +CONFIG_TESTING_CMOCKA=y +CONFIG_TESTING_CRYPTO=y +CONFIG_TESTING_DRIVER_TEST=y +CONFIG_TESTING_FFF=y +CONFIG_TESTING_FSTEST=y +CONFIG_TESTING_GETPRIME=y +CONFIG_TESTING_HEAP=y +CONFIG_TESTING_LTP=y +CONFIG_TESTING_LTP_STACKSIZE=65536 +CONFIG_TESTING_MEMORY_STRESS=y +CONFIG_TESTING_OSTEST=y +CONFIG_TESTING_OSTEST_STACKSIZE=4194304 +CONFIG_TESTING_RAMTEST=y +CONFIG_TESTING_SMP=y +CONFIG_TESTING_TIMERJITTER=y +CONFIG_TESTS_TESTSUITES=y +CONFIG_TESTS_TESTSUITES_MOUNT_DIR="/tmp" +CONFIG_TIMER_FD=y +CONFIG_USBHOST=y +CONFIG_USBHOST_COMPOSITE=y +CONFIG_USBHOST_FT232R=y +CONFIG_USBHOST_HIDKBD=y +CONFIG_USBHOST_HIDMOUSE=y +CONFIG_USBHOST_MSC=y +CONFIG_USBHOST_WAITER=y +CONFIG_USBHOST_XHCI_PCI=y +CONFIG_USEC_PER_TICK=1 diff --git a/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/run.sh b/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/run.sh new file mode 100755 index 0000000000000..b337e5891df3a --- /dev/null +++ b/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/run.sh @@ -0,0 +1,54 @@ +#!/usr/bin/env bash +############################################################################ +# boards/x86_64/qemu/qemu-intel64/configs/citest_smp/run.sh +# +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. The +# ASF licenses this file to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance with the +# License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# +############################################################################ + +set -o xtrace + +# start from nuttx dir +olddir=$(pwd) +nuttdir=${CURRENTCONFDIR}/../../../../../../ +cd ${nuttdir} + +# run NTFC +confpath=${CURRENTCONFDIR}/config.yaml +jsonconf=${CURRENTCONFDIR}/session.json +testpath=${NTFCDIR}/external/nuttx-testing +ntfc test --testpath=${testpath} --confpath=${confpath} --jsonconf=${jsonconf} + +ret="$?" +echo $ret + +# export test results +artifacts=${ARTIFACTCONFDIR}/ntfc +mkdir -p ${artifacts} + +# A retried build already exported the previous attempt; replace it, or +# "mv result" fails and the leftover directory makes the tree unclean. + +rm -rf ${artifacts}/result ${artifacts}/pytest.debug.log +mv pytest.debug.log ${artifacts} +mv result ${artifacts} + +# restore old dir +cd ${olddir} + +exit $ret diff --git a/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/session.json b/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/session.json new file mode 100644 index 0000000000000..2a8c05d05daae --- /dev/null +++ b/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/session.json @@ -0,0 +1,13 @@ +{ + "module": { + "include_module": [], + "exclude_module": + [ + "Nuttx_System_Fs_Fs" + ], + "order": [] + }, + "args": { + "kv": [] + } +} diff --git a/drivers/serial/serial_io.c b/drivers/serial/serial_io.c index 0f8bc6b270869..9eabbab24cb80 100644 --- a/drivers/serial/serial_io.c +++ b/drivers/serial/serial_io.c @@ -57,14 +57,22 @@ void uart_xmitchars(FAR uart_dev_t *dev) { uint16_t nbytes = 0; + sbuf_size_t head; #ifdef CONFIG_SMP irqstate_t flags = enter_critical_section(); #endif - /* Send while we still have data in the TX buffer & room in the fifo */ + /* Send while we still have data in the TX buffer & room in the fifo. + * + * uart_putxmitchar() advances xmit.head from thread context without + * holding the critical section, so on SMP it can move (and wrap) while + * we are in here. Sample it once per iteration: a stale value only + * makes us send less now, whereas reading it twice can turn the batch + * length negative and send from far beyond the buffer. + */ - while (dev->xmit.head != dev->xmit.tail && uart_txready(dev)) + while ((head = dev->xmit.head) != dev->xmit.tail && uart_txready(dev)) { /* Send the next byte */ @@ -72,9 +80,9 @@ void uart_xmitchars(FAR uart_dev_t *dev) { ssize_t sent; - if (dev->xmit.tail < dev->xmit.head) + if (dev->xmit.tail < head) { - sent = dev->xmit.head - dev->xmit.tail; + sent = head - dev->xmit.tail; } else { @@ -157,6 +165,7 @@ void uart_recvchars(FAR uart_dev_t *dev) int signo = 0; #endif uint16_t nbytes = 0; + sbuf_size_t tail; /* Loop putting characters into the receive buffer until there are no * further characters to available. @@ -165,22 +174,32 @@ void uart_recvchars(FAR uart_dev_t *dev) while (uart_rxavailable(dev)) { int nexthead = rxbuf->head + 1 < rxbuf->size ? rxbuf->head + 1 : 0; - bool is_full = (nexthead == rxbuf->tail); + bool is_full; FAR char *pbuf = NULL; char ch; + /* uart_read() advances recv.tail from thread context without holding + * the critical section, so on SMP it can move (and wrap) while we are + * in here. Sample it once per iteration and derive the free space + * from that snapshot: a stale value only makes us store less now, + * whereas reading it twice can turn the batch length negative. + */ + + tail = rxbuf->tail; + is_full = (nexthead == tail); + #ifdef CONFIG_SERIAL_IFLOWCONTROL_WATERMARKS unsigned int nbuffered; /* How many bytes are buffered */ - if (rxbuf->head >= rxbuf->tail) + if (rxbuf->head >= tail) { - nbuffered = rxbuf->head - rxbuf->tail; + nbuffered = rxbuf->head - tail; } else { - nbuffered = rxbuf->size - rxbuf->tail + rxbuf->head; + nbuffered = rxbuf->size - tail + rxbuf->head; } /* Is the level now above the watermark level that we need to report? */ @@ -223,11 +242,11 @@ void uart_recvchars(FAR uart_dev_t *dev) if (!is_full) { - if (rxbuf->tail > rxbuf->head) + if (tail > rxbuf->head) { - nbytes = rxbuf->tail - rxbuf->head - 1; + nbytes = tail - rxbuf->head - 1; } - else if (rxbuf->tail) + else if (tail) { nbytes = rxbuf->size - rxbuf->head; } 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) * ****************************************************************************/