xiaoxiang781216 commented on code in PR #20375:
URL: https://github.com/apache/nuttx/pull/20375#discussion_r4122168608
##########
include/nuttx/coredump.h:
##########
@@ -59,6 +78,47 @@ struct coredump_info_s
* Public Function Prototypes
****************************************************************************/
+/****************************************************************************
+ * Name: coredump_get_config
+ *
+ * Description:
+ * Copy the runtime coredump configuration.
+ *
+ ****************************************************************************/
+
+void coredump_get_config(FAR struct coredump_config_s *config);
+
+/****************************************************************************
+ * Name: coredump_set_mode
+ *
+ * Description:
+ * Set the runtime coredump mode.
+ *
+ ****************************************************************************/
+
+int coredump_set_mode(enum coredump_mode_e mode);
+
+/****************************************************************************
+ * Name: coredump_clear_memory_regions
+ *
+ * Description:
+ * Clear all runtime coredump memory regions.
+ *
+ ****************************************************************************/
+
+void coredump_clear_memory_regions(void);
+
+/****************************************************************************
+ * Name: coredump_set_memory_regions
+ *
+ * Description:
+ * Replace all runtime coredump memory regions.
+ *
+ ****************************************************************************/
+
+int coredump_set_memory_regions(
Review Comment:
```suggestion
int coredump_set_memory_regions(FAR const struct memory_region_s *regions,
size_t count);
```
##########
sched/misc/coredump.c:
##########
@@ -118,6 +127,99 @@ static const struct memory_region_s *g_regions;
* Private Functions
****************************************************************************/
+static int coredump_select_dump_scope(enum coredump_mode_e mode,
Review Comment:
```suggestion
static int coredump_select_pid(enum coredump_mode_e mode,
```
##########
sched/misc/coredump.c:
##########
@@ -719,7 +821,8 @@ static void elf_emit_phdr(FAR struct elf_dumpinfo_s *cinfo,
****************************************************************************/
#ifdef CONFIG_BOARD_COREDUMP_SYSLOG
-static void coredump_dump_syslog(pid_t pid)
+static void coredump_dump_syslog(
Review Comment:
```suggestion
static void coredump_dump_syslog(FAR const struct memory_region_s *regions,
pid_t pid)
```
##########
sched/misc/coredump.c:
##########
@@ -775,7 +878,8 @@ static void coredump_dump_syslog(pid_t pid)
****************************************************************************/
#ifdef CONFIG_BOARD_COREDUMP_DEV
-static void coredump_dump_dev(pid_t pid)
+static void coredump_dump_dev(
+ FAR const struct memory_region_s *regions, pid_t pid)
Review Comment:
ditto
##########
include/nuttx/coredump.h:
##########
@@ -42,10 +42,29 @@
#define COREDUMP_MAGIC 0x434f5245
#define COREDUMP_INFONAME_SIZE ALIGN_UP(CONFIG_TASK_NAME_SIZE, 8)
+#ifndef CONFIG_COREDUMP_MEMORY_REGION_MAX
+# define CONFIG_COREDUMP_MEMORY_REGION_MAX 8
+#endif
+
/****************************************************************************
* Public Types
****************************************************************************/
+/* Runtime coredump mode */
+
+enum coredump_mode_e
+{
+ COREDUMP_MODE_DISABLED = 0,
+ COREDUMP_MODE_CURRENT_TASK,
+ COREDUMP_MODE_ALL_TASKS,
Review Comment:
COREDUMP_MODE_OFF
COREDUMP_MODE_CURRENT
COREDUMP_MODE_ALL
##########
sched/misc/coredump.c:
##########
@@ -804,118 +908,119 @@ static void coredump_dump_dev(pid_t pid)
#endif
/****************************************************************************
- * Name: coredump_initialize_memory_region
+ * Name: coredump_get_config
*
* Description:
- * initialize the memory region with board memory range specified in config
+ * Copy the current runtime coredump configuration into the caller buffer.
*
****************************************************************************/
-static int coredump_initialize_memory_region(void)
+void coredump_get_config(FAR struct coredump_config_s *config)
{
-#ifdef CONFIG_BOARD_MEMORY_RANGE
- if (g_regions == NULL)
- {
- g_regions = g_memory_region;
- }
-#endif
+ irqstate_t flags;
- return OK;
+ DEBUGASSERT(config != NULL);
+
+ flags = enter_critical_section();
+ *config = g_coredump_config;
+ leave_critical_section(flags);
}
/****************************************************************************
- * Name: coredump_add_memory_region
- *
- * Description:
- * Use coredump to dump the memory of the specified area.
- *
+ * Name: coredump_set_mode
****************************************************************************/
-int coredump_add_memory_region(FAR const void *ptr, size_t size,
- uint32_t flags)
+int coredump_set_mode(enum coredump_mode_e mode)
{
- FAR struct memory_region_s *region;
- size_t count = 1; /* 1 for end flag */
- int ret;
-
- ret = coredump_initialize_memory_region();
- if (ret < 0)
+ switch (mode)
{
- return ret;
- }
-
- if (g_regions != NULL)
- {
- region = (FAR struct memory_region_s *)g_regions;
-
- while (region->start < region->end)
+ case COREDUMP_MODE_DISABLED:
+ case COREDUMP_MODE_CURRENT_TASK:
+ case COREDUMP_MODE_ALL_TASKS:
{
- if ((uintptr_t)ptr >= region->start &&
- (uintptr_t)ptr + size < region->end)
- {
- /* Already watched */
+ irqstate_t flags = enter_critical_section();
- return 0;
- }
- else if ((uintptr_t)ptr < region->start &&
- (uintptr_t)ptr + size >= region->end)
- {
- /* start out of region, end out of region */
+ g_coredump_config.mode = mode;
+ leave_critical_section(flags);
+ return OK;
+ }
- region->start = (uintptr_t)ptr;
- region->end = (uintptr_t)ptr + size;
- return 0;
- }
- else if ((uintptr_t)ptr < region->start &&
- (uintptr_t)ptr + size >= region->start)
- {
- /* start out of region, end in region */
+ default:
+ return -EINVAL;
+ }
+}
- region->start = (uintptr_t)ptr;
- return 0;
- }
- else if ((uintptr_t)ptr < region->end &&
- (uintptr_t)ptr + size >= region->end)
- {
- /* start in region, end out of region */
+/****************************************************************************
+ * Name: coredump_clear_memory_regions
+ ****************************************************************************/
- region->end = (uintptr_t)ptr + size;
- return 0;
- }
+void coredump_clear_memory_regions(void)
+{
+ coredump_set_memory_regions(NULL, 0);
+}
- count++;
- region++;
- }
+/****************************************************************************
+ * Name: coredump_set_memory_regions
+ ****************************************************************************/
- /* Need a new region */
+int coredump_set_memory_regions(FAR const struct memory_region_s *regions,
+ size_t count)
+{
+ irqstate_t flags;
+ size_t i;
+
+ if (count > CONFIG_COREDUMP_MEMORY_REGION_MAX ||
+ (count > 0 && regions == NULL))
+ {
+ return -EINVAL;
}
- region = lib_malloc(sizeof(struct memory_region_s) * (count + 1));
- if (region == NULL)
+ for (i = 0; i < count; i++)
{
- return -ENOMEM;
+ if (regions[i].start >= regions[i].end)
+ {
+ return -EINVAL;
+ }
}
- memcpy(region, g_regions, sizeof(struct memory_region_s) * count);
+ flags = enter_critical_section();
+ memset(g_coredump_config.regions, 0, sizeof(g_coredump_config.regions));
- if (g_regions != NULL
-#ifdef CONFIG_BOARD_MEMORY_RANGE
- && g_regions != g_memory_region
-#endif
- )
+ if (count > 0)
{
- lib_free((FAR void *)g_regions);
+ memcpy(g_coredump_config.regions, regions,
+ count * sizeof(struct memory_region_s));
}
- region[count - 1].start = (uintptr_t)ptr;
- region[count - 1].end = (uintptr_t)ptr + size;
- region[count - 1].flags = flags;
- region[count].start = 0;
- region[count].end = 0;
- region[count].flags = 0;
+ leave_critical_section(flags);
+ return OK;
+}
+
+/****************************************************************************
+ * Name: coredump_add_memory_region
+ *
+ * Description:
+ * Use coredump to dump the memory of the specified area.
+ *
+ ****************************************************************************/
+
+int coredump_add_memory_region(FAR const void *ptr, size_t size,
+ uint32_t flags)
+{
+ uintptr_t start = (uintptr_t)ptr;
+ uintptr_t end = start + size;
+ irqstate_t irqflags;
+ int ret;
- g_regions = region;
- return 0;
+ if (ptr == NULL || size == 0 || end <= start)
+ {
+ return -EINVAL;
+ }
+
+ irqflags = enter_critical_section();
Review Comment:
change to spinlock
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
