universal chSysLock?

[russian]

What is the best way to use chSysLock* from any context, i.e. both from and not from interruption handlers?

My logging helper method needs to put data into the output buffer (actual logging i-o works on a different thread), so I want my universal method not to care if it is invoked from a usual thread or from an interruption handler.

I was able to implement this as

Code: Select all

void lockForLogging() {
   if (dbg_isr_cnt > 0) {
      chSysLockFromIsr();
   } else {
      chSysLock();
   }
}

but I do not really like to access the extern variable which is only available in case of CH_DBG_SYSTEM_STATE_CHECK.

What is the best way to implement 'isInIrqHandler' check? Do I need to add my own hook into CH_IRQ_PROLOGUE?

Does such method not exist because I am not supposed to use such a method because it is wrong or against the philosophy of Chibios?

[Giovanni]

Hi,

Good question, basically it is against the philosophy of ChibiOS. There are two kind of locks in order to not have to perform such checks or save status data, this in the name of performance.

The problem is that chSysLock() and chSysLockFromIsr() on some systems do exactly the same thing, on other systems the behavior is different. The difference exists in order to hide the architecture-dependent details to the application.
If IRQs are preemptable (Cortex) then the "fromIsr" variant does the same thing of the normal function: modifies the mask of interrupts. On systems where IRQs are not preemptable the "fromIsr" variant is just a stub, there is no need to lock anything while in an ISR, the state checker does just a consistency check there.

Your problem is resolved by making your function an "I-Class", basically call you function: logMyMethodI() or something like that and let the caller do the lock for you and resolve ambiguity. Note that most ChibiOS functions have an I-class variant.

Just another note, I-class functions must never reschedule internally, the ISR code does that for them. This means that if you have to call an I-Class from thread context and the I-Class requires rescheduling you need to do:

Code: Select all

chSysLock();
logMyMethodI();
chSchRescheduleS(); // Rescheduling is done here *if* required.
chSysUnlock();

Giovanni

[ahaczewski]

When using Cortex you can use ICSR to know when you're in interrupt.

Code: Select all

  uint32_t icsr_vectactive = SCB->ICSR & 0x1fU;                 /* Get value of VECTACTIVE bits in ... */
                                                                /* ... Interrupt Control and State Register. */
  if (icsr_vectactive == 0) {                                   /* If no interrupt is active ... */
    chMtxLock(&mtx);                                            /* ... lock the mutex. */
  } else {
    chSysLockFromIsr();                                         /* When ISR is active, lock whole system. */
  }

Despite the ChibiOS philosophy it is necessary to implement Crossworks multithreading support functions (__printf_lock(), __debug_io_lock(), etc.) for example.

[russian]

Thank you for the icsr_vectactive implementation!

Regarding the other part of your implementation - is it in fact safe to lock mutex from one context and system from another? I mean if you acquire the mutex first and then the interrupt happens while you are still holding the mutex, you would get unguarded access to the resource would not you? Is not it required to lock whole system even if not within interrupt? Hopefully the interrupts would wait a bit and get processed right after you release the system lock.

[Giovanni]

Hi,

A context-aware chSysLock() is being considered for version 3.0.

Giovanni

[Giovanni]

More info.

Version 3.0 will include two new functions:

Code: Select all

/**
 * @brief   Returns the execution context and enters the kernel lock mode.
 * @details This functions enters into a critical zone and can be called
 *          from any context. Because its flexibility it is less efficient
 *          than @p chSysLock() which is preferable when the calling context
 *          is known.
 *
 * @return              The previous system status, the encoding of this
 *                      status word is architecture-dependent and opaque.
 *
 * @special
 */
syssts_t chSysGetAndLockX(void)  {

  syssts_t sts = port_get_irq_status();
  if (port_irq_enabled(sts)) {
    if (port_is_isr_context())
      chSysLockFromISR();
    else
      chSysLock();
  }
  return sts;
}

/**
 * @brief   Restores the specified execution status.
 *
 * @param[in] sts       the system status to be restored.
 *
 * @special
 */
void chSysRestoreLockX(syssts_t sts) {

  if (port_irq_enabled(sts)) {
    if (port_is_isr_context())
      chSysUnlockFromISR();
    else
      chSysUnlock();
  }
}

The new functions will be both context-aware and reentrant.

Of course the current functions will be retained because still much more efficient.

Giovanni