Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/libc/stdio/lib_lowoutstream.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@

#include "lib_internal.h"

#ifdef CONFIG_CRASH_LOG_SAVE
#include <tinyara/crashlog_writer/crashlog_writer.h>>
#endif
/****************************************************************************
* Private Functions
****************************************************************************/
Expand All @@ -76,6 +79,11 @@
static void lowoutstream_putc(FAR struct lib_outstream_s *this, int ch)
{
DEBUGASSERT(this);
#ifdef __KERNEL__
#ifdef CONFIG_CRASH_LOG_SAVE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about below

#if defined(__KERNEL__) && defined(CONFIG_CRASH_LOG_SAVE)
void crash_log_to_buffer(ch);
#else
#define crash_log_to_buffer(ch) (void)

This makes it possible to specify in the api header that it can only be used in kernel.

crash_log_to_buffer(ch);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. This feature covers the storage of all logs output in lldbg as well as crash logs
  2. This feature supports log saving to fs with compress.

How about module naming like ll_log_dump?

#endif
#endif
#if defined(CONFIG_BUILD_FLAT) || (defined(CONFIG_BUILD_PROTECTED) && defined(__KERNEL__))
if (up_putc(ch) != EOF) {
this->nput++;
Expand Down
4 changes: 4 additions & 0 deletions os/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,10 @@ menu "Binary manager"
source kernel/binary_manager/Kconfig
endmenu

menu "Crash log saving"
source kernel/crashlog_writer/Kconfig
endmenu

menu "Task Monitor"
source kernel/task_monitor/Kconfig
endmenu
Expand Down
14 changes: 13 additions & 1 deletion os/arch/arm/src/armv7-a/arm_assert.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@
#include "irq/irq.h"
#include "task/task.h"
#include "up_internal.h"
#ifdef CONFIG_CRASH_LOG_SAVE
#include <tinyara/crashlog_writer/crashlog_writer.h>>
#endif

bool abort_mode = false;

Expand Down Expand Up @@ -580,6 +583,7 @@ static inline void print_assert_detail(const uint8_t *filename, int lineno, stru

void up_assert(const uint8_t *filename, int lineno)
{

/* ARCH_GET_RET_ADDRESS should always be
* called at the start of the function */

Expand Down Expand Up @@ -610,7 +614,14 @@ void up_assert(const uint8_t *filename, int lineno)
} else {
asserted_location = kernel_assert_location;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please revert unnecessary change

#ifdef CONFIG_CRASH_LOG_SAVE
#ifdef CONFIG_BINMGR_RECOVERY
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's check CONFIG_CRASH_LOG_SAVE only here.

you already made dependency BINMGR_RECOVERY

if (IS_FAULT_IN_USER_SPACE(asserted_location)) {
/* start assert log saving */
set_store_to_buffer_flag(1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the fault handler start point might not be here. like arm_dataabort.c
the fault function also have log, the info matters

so in my opinion, we need to set default save on, and we need to stop and start between specific section menually

}
#endif
#endif
#ifdef CONFIG_SMP
/* Pause all other CPUs to avoid mix up of logs while printing assert logs */
up_cpu_pause_all();
Expand Down Expand Up @@ -654,6 +665,7 @@ void up_assert(const uint8_t *filename, int lineno)
if (IS_FAULT_IN_USER_SPACE(asserted_location)) {
/* Recover user fault through binary manager */
binary_manager_recover_userfault();
state = NORMAL_STATE;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert unnecessary change

} else
#endif
{
Expand Down
57 changes: 57 additions & 0 deletions os/include/tinyara/crashlog_writer/crashlog_writer.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/****************************************************************************
*
* Copyright 2022 Samsung Electronics All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2022 -> 2025.
Apply it to other new files as well.

*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
* either express or implied. See the License for the specific
* language governing permissions and limitations under the License.
*
****************************************************************************/

#ifndef __CRASH_LOG_H
#define __CRASH_LOG_H

/****************************************************************************
* Included Files
****************************************************************************/
/****************************************************************************
* Description:
* This is used to store logs that are output via lldbg() during a user crash situation into a buffer.
*
****************************************************************************/
void crash_log_to_buffer(char ch);

/****************************************************************************
* Description:
* If the flag is set to 1, crash logs start being stored in the buffer; if
* it is set to 0, the storage stops.
*
****************************************************************************/
void set_store_to_buffer_flag(int flag);

/****************************************************************************
* Description:
* Saves the crash log to a file.
* If the flag is 1, the log is saved in compressed form; if it is 0, it is saved uncompressed.
*
****************************************************************************/
char* save_crash_log(int flag);

/*************************************************************************************
* Name: read_crash_log
*
* Description:
* Reads a log from a file and stores it in a buffer. If the log is compresed, it is
* decompressed before being stored.
*************************************************************************************/
int read_crash_log(char *filename, char *buf, int buf_size);

#endif
1 change: 1 addition & 0 deletions os/kernel/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ include debug/Make.defs
include preference/Make.defs
include task_monitor/Make.defs
include log_dump/Make.defs
include crashlog_writer/Make.defs
include silent_reboot/Make.defs

AOBJS = $(ASRCS:.S=$(OBJEXT))
Expand Down
15 changes: 15 additions & 0 deletions os/kernel/binary_manager/binary_manager_recovery.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@
#include "semaphore/semaphore.h"
#include "binary_manager_internal.h"

#ifdef CONFIG_CRASH_LOG_SAVE
#include <tinyara/crashlog_writer/crashlog_writer.h>
#endif

/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
Expand Down Expand Up @@ -345,6 +349,17 @@ void binary_manager_recovery(int bin_idx)
bmlldbg("Failed to deactivate binary, bin idx %d\n", bin_idx);
goto reboot_board;
}
#endif
#ifdef CONFIG_CRASH_LOG_SAVE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use defined values, not hard-coded values for better readability.

#define XXXX_ENABLE 1
#define XXXX_DISABLE 0

set_store_to_buffer_flag(0); //stop saving log to buf
#ifdef CONFIG_COMPRESSION_FLAG
save_crash_log(1);
#else
save_crash_log(0);
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. If we select compress or not using CONFIG_COMPRESSION_FLAG, we don't need to have dynamic check and branch compress or not.
    but, if it is for checking memory not enough state, we need to have branch dynamically.

  2. and binary manager don't need to know about commpress or not, Let's move these into save_crash_log

void save_crash_log()
{
#ifdef CONFIG_CRASH_LOG_COMPRESS
    /* check memory largest chunk */
    if (memory enough) {
        /* compress log */
        /* saving compressed log into fs */
    } 
#ifdef CONFIG_CRASH_LOG_SUPPORT_MEMORY_FULL
    else {
        /* just saving log into fs */
    }
#endif
#else
/* just saving log into fs */
#endif
}

#ifdef CONFIG_REBOOT_AFTER_LOG_SAVE
binary_manager_reset_board(REBOOT_SYSTEM_BINARY_RECOVERYFAIL);
Copy link
Contributor

@ewoodev ewoodev Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reboot Reason should not be overwritten here.
in the time, we already writed reason reason about cause of crash.

/* Do not overwrite reboot reason here */ 
boardctl(BOARDIOC_RESET, EXIT_SUCCESS);

#endif
#endif
/* Create loader to reload binary */
ret = binary_manager_execute_loader(LOADCMD_RELOAD, bin_idx);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to make option to system reboot here.

Expand Down
64 changes: 64 additions & 0 deletions os/kernel/crashlog_writer/Kconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
#
# For a description of the syntax of this configuration file,
# see kconfig-language at https://www.kernel.org/doc/Documentation/kbuild/kconfig-language.txt
#


config CRASH_LOG_SAVE
bool "enable crash log saving"
default n
depends on BINMGR_RECOVERY
---help---
This saves the crash log.

if CRASH_LOG_SAVE

config COMPRESSION_FLAG
bool "COMPRESSION_FLAG"
default y
---help---
Controls whether compression is applied.
if the value is 0 compression is not performed.
Copy link
Contributor

@seokhun-eom24 seokhun-eom24 Jul 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be n instead of 0 since it's a boolean value.



config CRASH_LOG_BUF_SIZE
int "CRASH_LOG_BUF_SIZE"
default 8192
---help---
Determines the size of the buffer where crash logs will be stored.
The crash log is stored in the data section.

config REBOOT_AFTER_LOG_SAVE
bool "REBOOT_AFTER_SAVING_LOG"
default n
---help---
Determines whether to reboot after saving the crash log.

config MAX_DIGITS
int "MAX_DIGITS"
default 5
range 1 8
---help---
Specify the number of digits to use in the file name suffix.

config MAX_FILES
int "MAX_FILES"
default 5
---help---
Determine the maximum number of crash log files to be stored.

config CRASH_LOG_SAVE_RETRY_COUNT
int "CRASH_LOG_SAVE_RETRY_COUNT"
default 5
depends on !REBOOT_AFTER_LOG_SAVE
---help---
Limit the number of crash log saves in user mode during consecutive crashes to preserve flash memory lifespan.

config CRASH_LOG_TIME_TEST
bool "CRASH_LOG_TIME_TEST"
default n
---help---
Measure and record the time it takes to save the crash log to a file.

endif # CRASH_LOG_SAVE

26 changes: 26 additions & 0 deletions os/kernel/crashlog_writer/Make.defs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
###########################################################################
#
# Copyright 2022 Samsung Electronics All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
# either express or implied. See the License for the specific
# language governing permissions and limitations under the License.
#
###########################################################################

ifeq ($(CONFIG_CRASH_LOG_SAVE),y)

CSRCS += crashlog_writer.c

DEPPATH += --dep-path crashlog_writer
VPATH += :crashlog_writer

endif
Loading