Any comments on my binary semaphore wrapper?

Kernel
1

I need some additional features of a binary semaphore (for more info, please see Advice on the preferred "architecture" of a specific communication bus? ). What do you think, will this work (on an STM32U5-microcontroller)? Is it legal to take a semaphore when all interrupts are disabled (also the HAL_TICKS)?

#define DISABLE_ALL_INTS()                      uint32_t old_primask = __get_PRIMASK();   \
                                                __disable_irq()

#define ENABLE_ALL_INTS_IF_THEY_WERE_ENABLED()  if (!old_primask) {                       \
                                                    __enable_irq();                       \
                                                }

typedef enum {
    WE_JUST_TOOK_SEMAPHORE,
    WE_DID_NOT_TAKE_SEMAPHORE_BECAUSE_IT_WOULD_BLOCK_TASK,
} tryToTakeSemaphoreStatus_e;

typedef enum {
    DEFAULT_SEMAPHORE_STATE,
    SOMEONE_WANTS_SEMAPHORE,
    SEMAPHORE_TAKEN,
} semaphoreState_e;

static semaphoreState_e semaphoreState = DEFAULT_SEMAPHORE_STATE;
static xSemaphoreHandle semaphoreHandle = xSemaphoreCreateBinary();
static uint32_t numTasksTryingToTakeThisSemaphore_ = 0;

void takeSemaphoreBlockingAllowed() {
    {
        DISABLE_ALL_INTS();     // Critical section (no interrupts and context switches allowed!)
        semaphoreState = SOMEONE_WANTS_SEMAPHORE;
        numTasksTryingToTakeThisSemaphore_++;
        ENABLE_ALL_INTS_IF_THEY_WERE_ENABLED();
    }
    xSemaphoreTake(semaphoreHandle, portMAX_DELAY);
    {
        DISABLE_ALL_INTS();     // Critical section (no interrupts and context switches allowed!)
        semaphoreState = SEMAPHORE_TAKEN;
        numTasksTryingToTakeThisSemaphore_--;
        ENABLE_ALL_INTS_IF_THEY_WERE_ENABLED();
    }
}

tryToTakeSemaphoreStatus_e tryToTakeSemaphoreNoBlockingAllowed() {
    tryToTakeSemaphoreStatus_e tryToTakeSemaphoreStatus;
    DISABLE_ALL_INTS();     // Critical section (no interrupts and context switches allowed!)
    if (semaphoreState != DEFAULT_SEMAPHORE_STATE) {
        tryToTakeSemaphoreStatus = WE_DID_NOT_TAKE_SEMAPHORE_BECAUSE_IT_WOULD_BLOCK_TASK;
    } else {
        xSemaphoreTake(semaphoreHandle, portMAX_DELAY);    // Must not block!!! Am I allowed to take a semaphore with interrupts turned off?
        semaphoreState = SEMAPHORE_TAKEN;
        tryToTakeSemaphoreStatus = WE_JUST_TOOK_SEMAPHORE;
    }
    ENABLE_ALL_INTS_IF_THEY_WERE_ENABLED();
    return tryToTakeSemaphoreStatus;
}

void giveSemaphore() {
    DISABLE_ALL_INTS();     // Critical section (no interrupts and context switches allowed!)
    xSemaphoreGive(semaphoreHandle);
    if (semaphoreState == SEMAPHORE_TAKEN) {  // If a task recently started waiting for semaphore, the state will be SOMEONE_WANTS_SEMAPHORE and we shouldn't change it
        semaphoreState = DEFAULT_SEMAPHORE_STATE;
    }
    ENABLE_ALL_INTS_IF_THEY_WERE_ENABLED();
}

uint32_t numTasksTryingToTakeThisSemaphore() {
    return numTasksTryingToTakeThisSemaphore_;
}
2

First point notice, if you are in a critical section and don’t want to be able to block, call vTaskSemaphoreTake with a delay of 0, not portMAX_DELAY.

Also note, since you aren’t using a “From_ISR” version of the take, you can’t use this inside an ISR.

Also, it can be a bad idea to be changing the interrupt enable status and not use the FreeRTOS critical section API, as the FreeRTOS code will use its Critical Section API which might re-enable the interrupt which you have disabled, not knowing that you have disabled it.

2 Likes
3

The FreeRTOS kernel includes critical sections in task.h as macros taskENTER_CRITICAL and taskEXIT_CRITICAL. These macros are defined by all ports, normally in their respective portmacro.h.

Both the blocking and non-blocking versions of this wrapper do not check if xSemaphoreTake returned pdTRUE, indicating the semaphore was actually taken. If the semaphore count is 0 even after blocking, the semaphore is not taken.

The non-blocking version also does not set the max wait time to ( TickType_t ) 0. So, it doesn’t prevent xSemaphoreTake from blocking.

1 Like
4

I had to introduce the concept of semaphore “ownership”. If a task takes a semaphore and later doesn’t need it anymore but it knows some other task wants it, it can give up ownership of the semaphore without actually giving the semaphore. This is what my code looks like now:

typedef enum {
    WE_TOOK_SEMAPHORE_AND_SEMAPHORE_OWNERSHIP_OUTRIGHT,
    WE_ONLY_TOOK_SEMAPHORE_OWNERSHIP,
    WE_TOOK_NEITHER_SEMAPHORE_NOR_SEMAPHORE_OWNERSHIP_BECAUSE_IT_WOULD_BLOCK_TASK,
} tryToTakeSemaphoreStatus_e;

typedef enum {
    DEFAULT_SEMAPHORE_STATE,
    SOMEONE_WANTS_SEMAPHORE,
    SEMAPHORE_TAKEN,
} semaphoreState_e;

static semaphoreState_e semaphoreState = DEFAULT_SEMAPHORE_STATE;
static xSemaphoreHandle semaphoreHandle = xSemaphoreCreateBinary();
static bool semaphoreHasOwner = false;
static uint32_t numTasksTryingToTakeThisSemaphore_ = 0;

void takeSemaphoreBlockingAllowed() {
    taskENTER_CRITICAL();
    semaphoreState = SOMEONE_WANTS_SEMAPHORE;
    numTasksTryingToTakeThisSemaphore_++;
    taskEXIT_CRITICAL();
    xSemaphoreTake(semaphoreHandle, portMAX_DELAY);  // Calling task could possibly wait here for a "long" time to take the semaphore
    taskENTER_CRITICAL();
    semaphoreHasOwner = true;
    semaphoreState = SEMAPHORE_TAKEN;
    numTasksTryingToTakeThisSemaphore_--;
    taskEXIT_CRITICAL();
}

tryToTakeSemaphoreStatus_e tryToTakeSemaphoreNoBlockingAllowed() {
    tryToTakeSemaphoreStatus_e tryToTakeSemaphoreStatus;
    taskENTER_CRITICAL();
    if (semaphoreState != DEFAULT_SEMAPHORE_STATE) {
        if (semaphoreHasOwner) {
            tryToTakeSemaphoreStatus = WE_TOOK_NEITHER_SEMAPHORE_NOR_SEMAPHORE_OWNERSHIP_BECAUSE_IT_WOULD_BLOCK_TASK;
        } else {
            semaphoreHasOwner = true;
            tryToTakeSemaphoreStatus = WE_ONLY_TOOK_SEMAPHORE_OWNERSHIP;
        }
    } else {
        xSemaphoreTake(semaphoreHandle, 0);    // Must not block!!!
        semaphoreState = SEMAPHORE_TAKEN;
        semaphoreHasOwner = true;
        tryToTakeSemaphoreStatus = WE_TOOK_SEMAPHORE_AND_SEMAPHORE_OWNERSHIP_OUTRIGHT;
    }
    taskEXIT_CRITICAL();
    return tryToTakeSemaphoreStatus;
}

void giveUpSemaphoreOwnership() {
    taskENTER_CRITICAL();
    semaphoreHasOwner = false;
    taskEXIT_CRITICAL();
}

void giveSemaphore() {
    taskENTER_CRITICAL();
    xSemaphoreGive(semaphoreHandle);
    semaphoreHasOwner = false;
    if (semaphoreState == SEMAPHORE_TAKEN) {  // If a task is waiting for semaphore the state will be SOMEONE_WANTS_SEMAPHORE and we shouldn't change it
        semaphoreState = DEFAULT_SEMAPHORE_STATE;
    }
    taskEXIT_CRITICAL();
}

uint32_t numTasksTryingToTakeThisSemaphore() {
    return numTasksTryingToTakeThisSemaphore_;
}
5

Technically doing something like a Mutex in the Posix world, it has its uses!
Good work!
You can also use the task handle as the owner of the semaphore