Skip to content

Commit 825dea6

Browse files
bjarki-andreasenfabiobaltieri
authored andcommitted
shell: exchange k_mutex for k_sem
The mutex is being used as a simple binary semaphore. It is not recursed so we don't need to track thread ownership nor lock count. Exchange the mutex for a binary semaphore to save resources and speed up shell. Signed-off-by: Bjarki Arge Andreasen <[email protected]>
1 parent a90a47b commit 825dea6

File tree

4 files changed

+34
-19
lines changed

4 files changed

+34
-19
lines changed

include/zephyr/shell/shell.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -963,7 +963,7 @@ struct shell_ctx {
963963

964964
struct k_event signal_event;
965965

966-
struct k_mutex wr_mtx;
966+
struct k_sem lock_sem;
967967
k_tid_t tid;
968968
int ret_val;
969969
};

subsys/shell/shell.c

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -562,11 +562,11 @@ static int exec_cmd(const struct shell *sh, size_t argc, const char **argv,
562562
/* Unlock thread mutex in case command would like to borrow
563563
* shell context to other thread to avoid mutex deadlock.
564564
*/
565-
k_mutex_unlock(&sh->ctx->wr_mtx);
565+
z_shell_unlock(sh);
566566
ret_val = sh->ctx->active_cmd.handler(sh, argc,
567567
(char **)argv);
568568
/* Bring back mutex to shell thread. */
569-
k_mutex_lock(&sh->ctx->wr_mtx, K_FOREVER);
569+
z_shell_lock(sh);
570570
z_flag_cmd_ctx_set(sh, false);
571571
}
572572

@@ -1217,7 +1217,7 @@ static int instance_init(const struct shell *sh,
12171217
history_init(sh);
12181218

12191219
k_event_init(&sh->ctx->signal_event);
1220-
k_mutex_init(&sh->ctx->wr_mtx);
1220+
k_sem_init(&sh->ctx->lock_sem, 1, 1);
12211221

12221222
if (IS_ENABLED(CONFIG_SHELL_STATS)) {
12231223
sh->stats->log_lost_cnt = 0;
@@ -1342,7 +1342,7 @@ void shell_thread(void *shell_handle, void *arg_log_backend,
13421342
false,
13431343
K_FOREVER);
13441344

1345-
k_mutex_lock(&sh->ctx->wr_mtx, K_FOREVER);
1345+
z_shell_lock(sh);
13461346

13471347
shell_signal_handle(sh, SHELL_SIGNAL_KILL, kill_handler);
13481348
shell_signal_handle(sh, SHELL_SIGNAL_RXRDY, shell_process);
@@ -1355,7 +1355,7 @@ void shell_thread(void *shell_handle, void *arg_log_backend,
13551355
sh->iface->api->update(sh->iface);
13561356
}
13571357

1358-
k_mutex_unlock(&sh->ctx->wr_mtx);
1358+
z_shell_unlock(sh);
13591359
}
13601360
}
13611361

@@ -1420,7 +1420,7 @@ int shell_start(const struct shell *sh)
14201420
z_shell_log_backend_enable(sh->log_backend, (void *)sh, sh->ctx->log_level);
14211421
}
14221422

1423-
if (k_mutex_lock(&sh->ctx->wr_mtx, SHELL_TX_MTX_TIMEOUT) != 0) {
1423+
if (!z_shell_trylock(sh, SHELL_TX_MTX_TIMEOUT)) {
14241424
return -EBUSY;
14251425
}
14261426

@@ -1442,7 +1442,7 @@ int shell_start(const struct shell *sh)
14421442
*/
14431443
z_shell_backend_rx_buffer_flush(sh);
14441444

1445-
k_mutex_unlock(&sh->ctx->wr_mtx);
1445+
z_shell_unlock(sh);
14461446

14471447
return 0;
14481448
}
@@ -1524,7 +1524,7 @@ void shell_vfprintf(const struct shell *sh, enum shell_vt100_color color,
15241524
return;
15251525
}
15261526

1527-
if (k_mutex_lock(&sh->ctx->wr_mtx, SHELL_TX_MTX_TIMEOUT) != 0) {
1527+
if (!z_shell_trylock(sh, SHELL_TX_MTX_TIMEOUT)) {
15281528
return;
15291529
}
15301530

@@ -1537,7 +1537,7 @@ void shell_vfprintf(const struct shell *sh, enum shell_vt100_color color,
15371537
}
15381538
z_transport_buffer_flush(sh);
15391539

1540-
k_mutex_unlock(&sh->ctx->wr_mtx);
1540+
z_shell_unlock(sh);
15411541
}
15421542

15431543
/* These functions mustn't be used from shell context to avoid deadlock:
@@ -1664,20 +1664,20 @@ int shell_prompt_change(const struct shell *sh, const char *prompt)
16641664

16651665
size_t prompt_length = z_shell_strlen(prompt);
16661666

1667-
if (k_mutex_lock(&sh->ctx->wr_mtx, SHELL_TX_MTX_TIMEOUT) != 0) {
1667+
if (!z_shell_trylock(sh, SHELL_TX_MTX_TIMEOUT)) {
16681668
return -EBUSY;
16691669
}
16701670

16711671
if ((prompt_length + 1 > CONFIG_SHELL_PROMPT_BUFF_SIZE) || (prompt_length == 0)) {
1672-
k_mutex_unlock(&sh->ctx->wr_mtx);
1672+
z_shell_unlock(sh);
16731673
return -EINVAL;
16741674
}
16751675

16761676
strcpy(sh->ctx->prompt, prompt);
16771677

16781678
sh->ctx->vt100_ctx.cons.name_len = prompt_length;
16791679

1680-
k_mutex_unlock(&sh->ctx->wr_mtx);
1680+
z_shell_unlock(sh);
16811681

16821682
return 0;
16831683
#else
@@ -1687,11 +1687,11 @@ int shell_prompt_change(const struct shell *sh, const char *prompt)
16871687

16881688
void shell_help(const struct shell *sh)
16891689
{
1690-
if (k_mutex_lock(&sh->ctx->wr_mtx, SHELL_TX_MTX_TIMEOUT) != 0) {
1690+
if (!z_shell_trylock(sh, SHELL_TX_MTX_TIMEOUT)) {
16911691
return;
16921692
}
16931693
shell_internal_help_print(sh);
1694-
k_mutex_unlock(&sh->ctx->wr_mtx);
1694+
z_shell_unlock(sh);
16951695
}
16961696

16971697
int shell_execute_cmd(const struct shell *sh, const char *cmd)
@@ -1722,11 +1722,11 @@ int shell_execute_cmd(const struct shell *sh, const char *cmd)
17221722
sh->ctx->cmd_buff_len = cmd_len;
17231723
sh->ctx->cmd_buff_pos = cmd_len;
17241724

1725-
if (k_mutex_lock(&sh->ctx->wr_mtx, SHELL_TX_MTX_TIMEOUT) != 0) {
1725+
if (!z_shell_trylock(sh, SHELL_TX_MTX_TIMEOUT)) {
17261726
return -ENOEXEC;
17271727
}
17281728
ret_val = execute(sh);
1729-
k_mutex_unlock(&sh->ctx->wr_mtx);
1729+
z_shell_unlock(sh);
17301730

17311731
cmd_buffer_clear(sh);
17321732

subsys/shell/shell_log_backend.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ static void process_log_msg(const struct shell *sh,
181181
if (k_is_in_isr()) {
182182
key = irq_lock();
183183
} else {
184-
k_mutex_lock(&sh->ctx->wr_mtx, K_FOREVER);
184+
z_shell_lock(sh);
185185
}
186186
if (!z_flag_cmd_ctx_get(sh)) {
187187
z_shell_cmd_line_erase(sh);
@@ -197,7 +197,7 @@ static void process_log_msg(const struct shell *sh,
197197
if (k_is_in_isr()) {
198198
irq_unlock(key);
199199
} else {
200-
k_mutex_unlock(&sh->ctx->wr_mtx);
200+
z_shell_unlock(sh);
201201
}
202202
}
203203
}

subsys/shell/shell_ops.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,21 @@ void z_shell_vfprintf(const struct shell *sh, enum shell_vt100_color color,
381381
*/
382382
void z_shell_backend_rx_buffer_flush(const struct shell *sh);
383383

384+
static inline bool z_shell_trylock(const struct shell *sh, k_timeout_t timeout)
385+
{
386+
return k_sem_take(&sh->ctx->lock_sem, timeout) == 0;
387+
}
388+
389+
static inline void z_shell_lock(const struct shell *sh)
390+
{
391+
(void)k_sem_take(&sh->ctx->lock_sem, K_FOREVER);
392+
}
393+
394+
static inline void z_shell_unlock(const struct shell *sh)
395+
{
396+
k_sem_give(&sh->ctx->lock_sem);
397+
}
398+
384399
#ifdef __cplusplus
385400
}
386401
#endif

0 commit comments

Comments
 (0)