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);
>>
>>
>
>
>

Reply via email to