Skip to content

Commit 3359f53

Browse files
committed
Implement deferred logging for thread-safe printf
This implements an asynchronous I/O pattern for printf/puts, eliminating race conditions in preemptive multitasking. - Ring buffer: 8 entries × 128 bytes (1KB total) - Logger task at IDLE priority: drains queue to UART - Mutex-protected enqueue/dequeue with short critical sections - UART output outside lock: prevents blocking other tasks during writes - Graceful degradation: falls back to direct output on queue full Benefits: - Lower interrupt latency - ISRs remain responsive during logging - No nested critical section issues
1 parent 0ce5a6c commit 3359f53

File tree

9 files changed

+283
-17
lines changed

9 files changed

+283
-17
lines changed

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ include arch/$(ARCH)/build.mk
1717
INC_DIRS += -I $(SRC_DIR)/include \
1818
-I $(SRC_DIR)/include/lib
1919

20-
KERNEL_OBJS := timer.o mqueue.o pipe.o semaphore.o mutex.o error.o syscall.o task.o main.o
20+
KERNEL_OBJS := timer.o mqueue.o pipe.o semaphore.o mutex.o logger.o error.o syscall.o task.o main.o
2121
KERNEL_OBJS := $(addprefix $(BUILD_KERNEL_DIR)/,$(KERNEL_OBJS))
2222
deps += $(KERNEL_OBJS:%.o=%.o.d)
2323

app/mutex.c

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,13 @@ static int currently_in_critical_section = 0;
1818
/* Enhanced Task A */
1919
void task_a(void)
2020
{
21-
/* WORKAROUND: Printf not thread-safe in preemptive mode - minimize usage */
22-
2321
for (int i = 0; i < MAX_ITERATIONS; i++) {
2422
mo_sem_wait(binary_mutex);
2523

2624
/* === CRITICAL SECTION START === */
2725
if (currently_in_critical_section != 0) {
2826
critical_section_violations++;
27+
printf("Task A: VIOLATION detected at iteration %d\n", i);
2928
}
3029
currently_in_critical_section = mo_task_id();
3130

@@ -37,9 +36,11 @@ void task_a(void)
3736

3837
shared_counter = old_counter + 1;
3938
task_a_count++;
39+
printf("Task A: iteration %d, counter=%d\n", i, shared_counter);
4040

4141
if (currently_in_critical_section != mo_task_id()) {
4242
critical_section_violations++;
43+
printf("Task A: VIOLATION on exit at iteration %d\n", i);
4344
}
4445
currently_in_critical_section = 0;
4546
/* === CRITICAL SECTION END === */
@@ -51,6 +52,8 @@ void task_a(void)
5152
mo_task_yield();
5253
}
5354

55+
printf("Task A completed all iterations\n");
56+
5457
/* Keep running to prevent panic */
5558
while (1) {
5659
for (int i = 0; i < 10; i++)
@@ -61,18 +64,18 @@ void task_a(void)
6164
/* Enhanced Task B */
6265
void task_b(void)
6366
{
64-
/* WORKAROUND: Printf not thread-safe in preemptive mode - minimize usage */
65-
6667
for (int i = 0; i < MAX_ITERATIONS; i++) {
6768
/* Try non-blocking first */
6869
int32_t trylock_result = mo_sem_trywait(binary_mutex);
6970
if (trylock_result != ERR_OK) {
71+
printf("Task B: trylock failed, blocking at iteration %d\n", i);
7072
mo_sem_wait(binary_mutex);
7173
}
7274

7375
/* === CRITICAL SECTION START === */
7476
if (currently_in_critical_section != 0) {
7577
critical_section_violations++;
78+
printf("Task B: VIOLATION detected at iteration %d\n", i);
7679
}
7780
currently_in_critical_section = mo_task_id();
7881

@@ -84,9 +87,11 @@ void task_b(void)
8487

8588
shared_counter = old_counter + 10;
8689
task_b_count++;
90+
printf("Task B: iteration %d, counter=%d\n", i, shared_counter);
8791

8892
if (currently_in_critical_section != mo_task_id()) {
8993
critical_section_violations++;
94+
printf("Task B: VIOLATION on exit at iteration %d\n", i);
9095
}
9196
currently_in_critical_section = 0;
9297
/* === CRITICAL SECTION END === */
@@ -98,6 +103,8 @@ void task_b(void)
98103
mo_task_yield();
99104
}
100105

106+
printf("Task B completed all iterations\n");
107+
101108
/* Keep running to prevent panic */
102109
while (1) {
103110
for (int i = 0; i < 10; i++)
@@ -108,15 +115,16 @@ void task_b(void)
108115
/* Simple monitor task */
109116
void monitor_task(void)
110117
{
111-
/* WORKAROUND: Printf not thread-safe - only print at end when tasks idle */
112-
113118
int cycles = 0;
114119

120+
printf("Monitor: Starting test monitoring\n");
121+
115122
while (cycles < 50) { /* Monitor for reasonable time */
116123
cycles++;
117124

118125
/* Check if both tasks completed */
119126
if (task_a_count >= MAX_ITERATIONS && task_b_count >= MAX_ITERATIONS) {
127+
printf("Monitor: Both tasks completed, finalizing test\n");
120128
break;
121129
}
122130

@@ -125,11 +133,11 @@ void monitor_task(void)
125133
mo_task_yield();
126134
}
127135

128-
/* Wait a bit for tasks to fully idle */
136+
/* Wait a bit for tasks to fully complete */
129137
for (int i = 0; i < 50; i++)
130138
mo_task_yield();
131139

132-
/* Final report - safe to print when other tasks are idle */
140+
/* Final report */
133141
printf("\n=== FINAL RESULTS ===\n");
134142
printf("Task A iterations: %d\n", task_a_count);
135143
printf("Task B iterations: %d\n", task_b_count);
@@ -193,7 +201,9 @@ int32_t app_main(void)
193201
return false;
194202
}
195203

196-
/* CRITICAL FIX: Printf hangs after task_spawn - remove all printf calls */
197-
/* Tasks created: A=%d, B=%d, Monitor=%d, Idle=%d */
204+
printf("Tasks created: A=%d, B=%d, Monitor=%d, Idle=%d\n", task_a_id,
205+
task_b_id, monitor_id, idle_id);
206+
printf("Enabling preemptive scheduling mode\n");
207+
198208
return true; /* Enable preemptive scheduling */
199209
}

include/lib/libc.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ int random_r(struct random_data *buf, int32_t *result);
141141

142142
/* Character and string output */
143143
int32_t puts(const char *str);
144+
int _putchar(int c); /* Low-level character output (used by logger) */
144145

145146
/* Character and string input */
146147
int getchar(void);

include/linmo.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <lib/malloc.h>
77

88
#include <sys/errno.h>
9+
#include <sys/logger.h>
910
#include <sys/mqueue.h>
1011
#include <sys/mutex.h>
1112
#include <sys/pipe.h>

include/private/error.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ enum {
2626
ERR_NOT_OWNER, /* Operation requires ownership */
2727

2828
/* Memory Protection Errors */
29-
ERR_STACK_CHECK, /* Stack overflow or corruption detected */
29+
ERR_STACK_CHECK, /* Stack overflow or corruption detected */
3030
ERR_HEAP_CORRUPT, /* Heap corruption or invalid free detected */
3131

3232
/* IPC and Synchronization Errors */

include/sys/logger.h

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
#pragma once
2+
3+
/* Deferred logging system for thread-safe printf in preemptive mode.
4+
*
5+
* Architecture:
6+
* - printf/puts format into a buffer and enqueue the complete message
7+
* - Logger task dequeues messages and outputs to UART
8+
* - Minimal critical sections (only during enqueue/dequeue operations)
9+
* - No long interrupt disable periods during UART output
10+
*
11+
* Benefits:
12+
* - Low interrupt latency
13+
* - ISRs remain responsive during logging
14+
* - No nested critical section issues
15+
* - Proper separation between formatting and output
16+
*/
17+
18+
#include <types.h>
19+
20+
/* Logger Configuration - Optimized for memory efficiency
21+
* These values balance memory usage with logging capacity:
22+
* - 8 entries handles typical burst logging scenarios
23+
* - 128 bytes accommodates most debug messages
24+
* Total buffer overhead: 8 × 128 = 1KB (down from 4KB)
25+
*/
26+
#define LOG_QSIZE 8 /* Number of log entries in ring buffer */
27+
#define LOG_ENTRY_SZ 128 /* Maximum length of single log message */
28+
29+
/* Logger Control */
30+
31+
/* Initialize the logger subsystem.
32+
* Creates the log queue and spawns the logger task.
33+
* Must be called during kernel initialization, after heap and task system init.
34+
*
35+
* Returns ERR_OK on success, ERR_FAIL on failure
36+
*/
37+
int32_t mo_logger_init(void);
38+
39+
/* Enqueue a log message for deferred output.
40+
* Non-blocking: if queue is full, message is dropped.
41+
* Thread-safe: protected by short critical section.
42+
* @msg : Null-terminated message string
43+
* @length : Length of message (excluding null terminator)
44+
*
45+
* Returns ERR_OK if enqueued, ERR_BUSY if queue full
46+
*/
47+
int32_t mo_logger_enqueue(const char *msg, uint16_t length);
48+
49+
/* Get the number of messages currently in the queue.
50+
* Useful for monitoring queue depth and detecting overruns.
51+
*
52+
* Returns number of queued messages
53+
*/
54+
uint32_t mo_logger_queue_depth(void);
55+
56+
/* Get the total number of dropped messages due to queue overflow.
57+
*
58+
* Returns total dropped message count since logger init
59+
*/
60+
uint32_t mo_logger_dropped_count(void);

kernel/logger.c

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
/* Deferred logging: async I/O pattern for thread-safe printf.
2+
*
3+
* Design rationale:
4+
* - Ring buffer + mutex
5+
* - Logger task at IDLE priority: drains queue without blocking tasks
6+
* - UART output outside lock: other tasks enqueue while we output
7+
* - Graceful degradation: fallback to direct output on queue full
8+
*/
9+
10+
#include <lib/libc.h>
11+
#include <sys/logger.h>
12+
#include <sys/mutex.h>
13+
#include <sys/task.h>
14+
15+
#include "private/error.h"
16+
17+
/* Ring buffer entry: fixed-size for O(1) enqueue/dequeue */
18+
typedef struct {
19+
uint16_t length;
20+
char data[LOG_ENTRY_SZ];
21+
} log_entry_t;
22+
23+
/* Logger state: single global instance, no dynamic allocation */
24+
typedef struct {
25+
log_entry_t queue[LOG_QSIZE];
26+
uint32_t head, tail, count;
27+
uint32_t dropped; /* Diagnostic: tracks queue overflow events */
28+
mutex_t lock; /* Protects queue manipulation, not UART output */
29+
int32_t task_id;
30+
bool initialized;
31+
} logger_state_t;
32+
33+
static logger_state_t logger;
34+
35+
/* Logger task: IDLE priority ensures application tasks run first */
36+
static void logger_task(void)
37+
{
38+
log_entry_t entry;
39+
40+
while (1) {
41+
bool have_message = false;
42+
43+
/* Critical section: only queue manipulation, not UART I/O */
44+
mo_mutex_lock(&logger.lock);
45+
if (logger.count > 0) {
46+
memcpy(&entry, &logger.queue[logger.tail], sizeof(log_entry_t));
47+
logger.tail = (logger.tail + 1) % LOG_QSIZE;
48+
logger.count--;
49+
have_message = true;
50+
}
51+
mo_mutex_unlock(&logger.lock);
52+
53+
if (have_message) {
54+
/* Key design: UART output outside lock prevents blocking enqueuers.
55+
* shorter UART write does not hold mutex - other tasks enqueue in
56+
* parallel.
57+
*/
58+
for (uint16_t i = 0; i < entry.length; i++)
59+
_putchar(entry.data[i]);
60+
} else {
61+
/* Block when idle: sleep 1 tick, scheduler wakes us next period */
62+
mo_task_delay(1);
63+
}
64+
}
65+
}
66+
67+
/* Call after heap + task system init, before enabling preemption */
68+
int32_t mo_logger_init(void)
69+
{
70+
if (logger.initialized)
71+
return ERR_OK;
72+
73+
memset(&logger, 0, sizeof(logger_state_t));
74+
75+
if (mo_mutex_init(&logger.lock) != ERR_OK)
76+
return ERR_FAIL;
77+
78+
/* 512B stack: simple operations only (no printf/recursion/ISR use) */
79+
logger.task_id = mo_task_spawn(logger_task, 512);
80+
if (logger.task_id < 0) {
81+
mo_mutex_destroy(&logger.lock);
82+
return ERR_FAIL;
83+
}
84+
85+
/* IDLE priority: runs only when no application tasks are ready */
86+
mo_task_priority(logger.task_id, TASK_PRIO_IDLE);
87+
88+
logger.initialized = true;
89+
return ERR_OK;
90+
}
91+
92+
/* Non-blocking enqueue: returns ERR_TASK_BUSY on overflow, never waits */
93+
int32_t mo_logger_enqueue(const char *msg, uint16_t length)
94+
{
95+
if (!logger.initialized || !msg || length == 0)
96+
return ERR_FAIL;
97+
98+
/* Defensive check: stdio.c pre-filters, but validate anyway */
99+
if (length > LOG_ENTRY_SZ - 1)
100+
length = LOG_ENTRY_SZ - 1;
101+
102+
mo_mutex_lock(&logger.lock);
103+
104+
/* Drop message on full queue: non-blocking design, caller falls back to
105+
* direct I/O
106+
*/
107+
if (logger.count >= LOG_QSIZE) {
108+
logger.dropped++;
109+
mo_mutex_unlock(&logger.lock);
110+
return ERR_TASK_BUSY;
111+
}
112+
113+
log_entry_t *entry = &logger.queue[logger.head];
114+
entry->length = length;
115+
memcpy(entry->data, msg, length);
116+
/* Safety: enables direct string ops on data[] */
117+
entry->data[length] = '\0';
118+
119+
logger.head = (logger.head + 1) % LOG_QSIZE;
120+
logger.count++;
121+
122+
mo_mutex_unlock(&logger.lock);
123+
124+
return ERR_OK;
125+
}
126+
127+
/* Diagnostic: monitor queue depth to detect sustained overflow conditions */
128+
uint32_t mo_logger_queue_depth(void)
129+
{
130+
if (!logger.initialized)
131+
return 0;
132+
133+
mo_mutex_lock(&logger.lock);
134+
uint32_t depth = logger.count;
135+
mo_mutex_unlock(&logger.lock);
136+
137+
return depth;
138+
}
139+
140+
/* Diagnostic: total messages lost since init (non-resettable counter) */
141+
uint32_t mo_logger_dropped_count(void)
142+
{
143+
if (!logger.initialized)
144+
return 0;
145+
146+
mo_mutex_lock(&logger.lock);
147+
uint32_t dropped = logger.dropped;
148+
mo_mutex_unlock(&logger.lock);
149+
150+
return dropped;
151+
}

kernel/main.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include <hal.h>
22
#include <lib/libc.h>
3+
#include <sys/logger.h>
34
#include <sys/task.h>
45

56
#include "private/error.h"
@@ -23,6 +24,17 @@ int32_t main(void)
2324
printf("Heap initialized, %u bytes available\n",
2425
(unsigned int) (size_t) &_heap_size);
2526

27+
/* Initialize deferred logging system.
28+
* Must be done after heap init but before app_main() to ensure
29+
* application tasks can use thread-safe printf.
30+
* Note: Early printf calls (above) use direct output fallback.
31+
*/
32+
if (mo_logger_init() != 0) {
33+
printf("Warning: Logger initialization failed, using direct output\n");
34+
} else {
35+
printf("Logger initialized\n");
36+
}
37+
2638
/* Call the application's main entry point to create initial tasks. */
2739
kcb->preemptive = (bool) app_main();
2840
printf("Scheduler mode: %s\n",

0 commit comments

Comments
 (0)