[BUG] Fix deadlocks on disconnected secondary half (#17423)

This commit is contained in:
Stefan Kerkmann 2022-06-21 00:24:53 +02:00 committed by GitHub
parent 62eaec52e0
commit 2703ecc9e9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 38 additions and 11 deletions

View file

@ -87,10 +87,7 @@ static THD_FUNCTION(Thread1, arg) {
chRegSetThreadName("blinker"); chRegSetThreadName("blinker");
while (true) { while (true) {
palWaitLineTimeout(SOFT_SERIAL_PIN, TIME_INFINITE); palWaitLineTimeout(SOFT_SERIAL_PIN, TIME_INFINITE);
split_shared_memory_lock();
interrupt_handler(NULL); interrupt_handler(NULL);
split_shared_memory_unlock();
} }
} }
@ -155,6 +152,7 @@ static void __attribute__((noinline)) serial_write_byte(uint8_t data) {
// interrupt handle to be used by the slave device // interrupt handle to be used by the slave device
void interrupt_handler(void *arg) { void interrupt_handler(void *arg) {
split_shared_memory_lock_autounlock();
chSysLockFromISR(); chSysLockFromISR();
sync_send(); sync_send();
@ -212,6 +210,8 @@ void interrupt_handler(void *arg) {
static inline bool initiate_transaction(uint8_t sstd_index) { static inline bool initiate_transaction(uint8_t sstd_index) {
if (sstd_index > NUM_TOTAL_TRANSACTIONS) return false; if (sstd_index > NUM_TOTAL_TRANSACTIONS) return false;
split_shared_memory_lock_autounlock();
split_transaction_desc_t *trans = &split_transaction_table[sstd_index]; split_transaction_desc_t *trans = &split_transaction_table[sstd_index];
// TODO: remove extra delay between transactions // TODO: remove extra delay between transactions
@ -292,8 +292,5 @@ static inline bool initiate_transaction(uint8_t sstd_index) {
// //
// this code is very time dependent, so we need to disable interrupts // this code is very time dependent, so we need to disable interrupts
bool soft_serial_transaction(int sstd_index) { bool soft_serial_transaction(int sstd_index) {
split_shared_memory_lock(); return initiate_transaction((uint8_t)sstd_index);
bool result = initiate_transaction((uint8_t)sstd_index);
split_shared_memory_unlock();
return result;
} }

View file

@ -22,13 +22,11 @@ static THD_FUNCTION(SlaveThread, arg) {
chRegSetThreadName("split_protocol_tx_rx"); chRegSetThreadName("split_protocol_tx_rx");
while (true) { while (true) {
split_shared_memory_lock();
if (unlikely(!react_to_transaction())) { if (unlikely(!react_to_transaction())) {
/* Clear the receive queue, to start with a clean slate. /* Clear the receive queue, to start with a clean slate.
* Parts of failed transactions or spurious bytes could still be in it. */ * Parts of failed transactions or spurious bytes could still be in it. */
serial_transport_driver_clear(); serial_transport_driver_clear();
} }
split_shared_memory_unlock();
} }
} }
@ -64,6 +62,8 @@ static inline bool react_to_transaction(void) {
return false; return false;
} }
split_shared_memory_lock_autounlock();
split_transaction_desc_t* transaction = &split_transaction_table[transaction_id]; split_transaction_desc_t* transaction = &split_transaction_table[transaction_id];
/* Send back the handshake which is XORed as a simple checksum, /* Send back the handshake which is XORed as a simple checksum,
@ -102,9 +102,7 @@ static inline bool react_to_transaction(void) {
* @return bool Indicates success of transaction. * @return bool Indicates success of transaction.
*/ */
bool soft_serial_transaction(int index) { bool soft_serial_transaction(int index) {
split_shared_memory_lock();
bool result = initiate_transaction((uint8_t)index); bool result = initiate_transaction((uint8_t)index);
split_shared_memory_unlock();
if (unlikely(!result)) { if (unlikely(!result)) {
/* Clear the receive queue, to start with a clean slate. /* Clear the receive queue, to start with a clean slate.
@ -125,6 +123,8 @@ static inline bool initiate_transaction(uint8_t transaction_id) {
return false; return false;
} }
split_shared_memory_lock_autounlock();
split_transaction_desc_t* transaction = &split_transaction_table[transaction_id]; split_transaction_desc_t* transaction = &split_transaction_table[transaction_id];
/* Send transaction table index to the slave, which doubles as basic handshake token. */ /* Send transaction table index to the slave, which doubles as basic handshake token. */

View file

@ -12,3 +12,33 @@ void split_shared_memory_unlock(void);
inline void split_shared_memory_lock(void){}; inline void split_shared_memory_lock(void){};
inline void split_shared_memory_unlock(void){}; inline void split_shared_memory_unlock(void){};
#endif #endif
/* GCCs cleanup attribute expects a function with one parameter, which is a
* pointer to a type compatible with the variable. As we don't want to expose
* the platforms internal mutex type this workaround with auto generated adapter
* function is defined */
#define QMK_DECLARE_AUTOUNLOCK_HELPERS(prefix) \
inline unsigned prefix##_autounlock_lock_helper(void) { \
prefix##_lock(); \
return 0; \
} \
\
inline void prefix##_autounlock_unlock_helper(unsigned* unused_guard) { \
prefix##_unlock(); \
}
/* Convinience macro the automatically generate the correct RAII-style
* lock_autounlock function macro */
#define QMK_DECLARE_AUTOUNLOCK_CALL(prefix) unsigned prefix##_guard __attribute__((unused, cleanup(prefix##_autounlock_unlock_helper))) = prefix##_autounlock_lock_helper
QMK_DECLARE_AUTOUNLOCK_HELPERS(split_shared_memory)
/**
* @brief Acquire exclusive access to the split keyboard shared memory, by
* calling the platforms `split_shared_memory_lock()` function. The lock is
* automatically released by calling the platforms `split_shared_memory_unlock()`
* function. This happens when the block where
* `split_shared_memory_lock_autounlock()` is called in goes out of scope i.e.
* when the enclosing function returns.
*/
#define split_shared_memory_lock_autounlock QMK_DECLARE_AUTOUNLOCK_CALL(split_shared_memory)