Xiang Xiao, is that still true for the latest code in master branch? And by "system will crash if the priority inheritance enabled semaphore is waited or posted from different threads" do you mean at the point of sem_post/sem_wait or some system instability in general?
Best regards, Petro On Fri, Mar 31, 2023, 10:39 PM Xiang Xiao <xiaoxiang781...@gmail.com> wrote: > BTW, https://github.com/apache/nuttx/pull/5070 report that the system will > crash if the priority inheritance enabled semaphore is waited or posted > from different threads. > > On Sat, Apr 1, 2023 at 3:20 AM Xiang Xiao <xiaoxiang781...@gmail.com> > wrote: > > > > > > > On Sat, Apr 1, 2023 at 12:34 AM Gregory Nutt <spudan...@gmail.com> > wrote: > > > >> On 3/31/2023 8:56 AM, Gregory Nutt wrote: > >> > > >> >> Even more. In my previous example if semaphore is posted from the > >> >> interrupt > >> >> we do not know which of TaskA or TaskB is no longer a "holder l" of a > >> >> semaphore. > >> >> > >> > You are right. In this usage case, the counting semaphore is not > >> > being used for locking; it is being used for signalling an event per > >> > > >> > https://cwiki.apache.org/confluence/display/NUTTX/Signaling+Semaphores+and+Priority+Inheritance > >> > > >> > In that case, priority inheritance must be turned off. > >> > > >> You example is really confusing because you are mixing two different > >> concepts, just subtly enough to be really confusing. If a counting > >> semaphore is used for locking a set of multiple resources, the posting > >> the semaphore does not release the resource. That is not the way that > >> it is done. Here is a more believable example of how this would work: > >> > >> 1. TaskA with priority 10 allocates DMA0 channel by calling a DMA > >> channel allocation function. If a DMA channel is available, it is > >> allocated and the allocation function takes the semaphore. TaskA > >> then starts DMA activity. > >> 2. TaskA waits on another signaling semaphore for the DMA to complete. > >> 3. TaskB with priority 20 does the same. > >> 4. TaskC with priority 30 wants to allocate a DMA channel. It calls > >> the channel allocation function which waits on the sempahore for a > >> count to be available. This boost the priority of TaskA and TaskB > >> to 30. > >> 5. When the DMA started by TaskA completes, it signals TaskA which > >> calls the resource deallocation function which increments the > >> counting semaphore, giving the count to TaskC and storing the base > >> priorities. > >> > >> > > > > Normally, the resource(dma channel here) is allocated from one > > thread/task, but may be freed in another thread/task. Please consider how > > we malloc and free memory. > > > > > >> The confusion arises because you are mixing the signaling logic with the > >> resource deallocation logic. > >> > >> The mm/iob logic works basically this way. The logic more complex then > >> you would think from above. IOBs is an example of a critical system > >> resource that has multiple copies and utilizes a counting semaphore with > >> priority inheritance to achieve good real time performance. IOB > >> handling is key logic for the correct real time operation of the overall > >> system. Nothing we do must risk this. > >> > >> > > IOB is a very good example to demonstrate why it's a bad and dangerous > > idea to enable priority inheritance for the counting semaphore. IOB is > > normally allocated in the send thread but free in the work thread. If we > > want the priority inheritance to work as expected instead of crashing the > > system, sem_wait/sem_post must come from the same thread, which is a kind > > of lock. > > > > > >> Other places where this logic is (probably) used: > >> > >> arch/arm/src/rp2040/rp2040_i2s.c: nxsem_init(&priv->bufsem, 0, > >> CONFIG_RP2040_I2S_MAXINFLIGHT); > >> arch/arm/src/rtl8720c/amebaz_depend.c: if (sem_init(_sema, 0, > >> init_val)) > >> arch/arm/src/sama5/sam_ssc.c: nxsem_init(&priv->bufsem, 0, > >> CONFIG_SAMA5_SSC_MAXINFLIGHT); > >> arch/arm/src/samv7/sam_ssc.c: nxsem_init(&priv->bufsem, 0, > >> CONFIG_SAMV7_SSC_MAXINFLIGHT); > >> arch/arm/src/stm32/stm32_i2s.c: nxsem_init(&priv->bufsem, 0, > >> CONFIG_STM32_I2S_MAXINFLIGHT); > >> drivers/can/mcp2515.c: nxsem_init(&priv->txfsem, 0, > >> MCP2515_NUM_TX_BUFFERS); > >> drivers/video/vnc/vnc_server.c: nxsem_init(&session->freesem, 0, > >> CONFIG_VNCSERVER_NUPDATES); > >> sched/pthread/pthread_completejoin.c: nxsem_init(&pjoin->data_sem, > >> 0, (ntasks_waiting + 1)); > >> wireless/bluetooth/bt_hcicore.c: nxsem_init(&g_btdev.le_pkts_sem, 0, > >> g_btdev.le_pkts); > >> wireless/bluetooth/bt_hcicore.c: nxsem_init(&g_btdev.ncmd_sem, 0, > 1); > >> wireless/ieee802154/mac802154.c: nxsem_init(&priv->txdesc_sem, 0, > >> CONFIG_MAC802154_NTXDESC); > >> wireless/ieee802154/mac802154.c: nxsem_init(&mac->opsem, 0, 1); > >> > >> Maybe: > >> > >> arch/risc-v/src/bl602/bl602_os_hal.c: ret = nxsem_init(sem, 0, > init); > >> arch/risc-v/src/esp32c3/esp32c3_ble_adapter.c: ret = > >> sem_init(&bt_sem->sem, 0, init); > >> arch/risc-v/src/esp32c3/esp32c3_wifi_adapter.c: ret = > >> nxsem_init(sem, 0, init); > >> arch/xtensa/src/esp32/esp32_ble_adapter.c: ret = sem_init(sem, 0, > >> init); > >> arch/xtensa/src/esp32/esp32_wifi_adapter.c: ret = nxsem_init(sem, 0, > >> init); > >> arch/xtensa/src/esp32s3/esp32s3_wifi_adapter.c: ret = > >> nxsem_init(sem, 0, init); > >> > >> > > > > > > >
