Page 1 of 1
universal chSysLock?
Posted: Sun Jun 23, 2013 7:05 pm
by 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?
Re: universal chSysLock?
Posted: Sun Jun 23, 2013 8:18 pm
by 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
Re: universal chSysLock?
Posted: Tue Jul 23, 2013 4:44 pm
by 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.
Re: universal chSysLock?
Posted: Tue Jul 23, 2013 5:09 pm
by 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.
Re: universal chSysLock?
Posted: Tue Jul 23, 2013 5:41 pm
by Giovanni
Hi,
A context-aware chSysLock() is being considered for version 3.0.
Giovanni
Re: universal chSysLock?
Posted: Thu Jul 25, 2013 11:05 am
by 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