xiaoxiang781216 commented on code in PR #13599:
URL: https://github.com/apache/nuttx/pull/13599#discussion_r1779827667
##########
drivers/syslog/ramlog.c:
##########
@@ -234,6 +243,82 @@ static void ramlog_pollnotify(FAR struct ramlog_dev_s
*priv)
}
}
+/****************************************************************************
+ * Name: ramlog_flush_internal
+ ****************************************************************************/
+
+#ifdef CONFIG_RAMLOG_FLUSH
+static void ramlog_flush_internal(FAR struct ramlog_dev_s *priv, bool force)
+{
+ FAR struct ramlog_header_s *header = priv->rl_header;
+ FAR const char *start;
+ FAR const char *pos;
+ irqstate_t flags;
+ uint32_t begin;
+ uint32_t end;
+
+ flags = enter_critical_section();
+
+ do
+ {
+ if (header->rl_head == priv->rl_tail)
+ {
+ break;
+ }
+
+ if (header->rl_head - priv->rl_tail > priv->rl_bufsize)
+ {
+ priv->rl_tail = header->rl_head - priv->rl_bufsize;
+ }
+
+ begin = priv->rl_tail % priv->rl_bufsize;
Review Comment:
change begin/end to tail/head
##########
drivers/syslog/ramlog.c:
##########
@@ -665,6 +768,26 @@ static int ramlog_file_close(FAR struct file *filep)
* Public Functions
****************************************************************************/
+/****************************************************************************
+ * Name: ramlog_flush
+ *
+ * Description:
+ * This is called by system crash-handling logic. It must flush any
+ * buffered data to the SYSLOG device.
+ *
+ * Interrupts are disabled at the time of the crash and this logic must
+ * perform the flush using low-level, non-interrupt driven logic.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_RAMLOG_FLUSH
+int ramlog_flush(FAR syslog_channel_t *channel)
+{
+ ramlog_flush_internal(&g_sysdev, true);
Review Comment:
why not merge ramlog_flush_internal into ramlog_flush?
##########
drivers/syslog/ramlog.c:
##########
@@ -234,6 +243,82 @@ static void ramlog_pollnotify(FAR struct ramlog_dev_s
*priv)
}
}
+/****************************************************************************
+ * Name: ramlog_flush_internal
+ ****************************************************************************/
+
+#ifdef CONFIG_RAMLOG_FLUSH
+static void ramlog_flush_internal(FAR struct ramlog_dev_s *priv, bool force)
+{
+ FAR struct ramlog_header_s *header = priv->rl_header;
+ FAR const char *start;
+ FAR const char *pos;
+ irqstate_t flags;
+ uint32_t begin;
+ uint32_t end;
+
+ flags = enter_critical_section();
+
+ do
+ {
+ if (header->rl_head == priv->rl_tail)
+ {
+ break;
+ }
+
+ if (header->rl_head - priv->rl_tail > priv->rl_bufsize)
+ {
+ priv->rl_tail = header->rl_head - priv->rl_bufsize;
+ }
+
+ begin = priv->rl_tail % priv->rl_bufsize;
+ end = header->rl_head % priv->rl_bufsize;
+
+ if (end <= begin)
+ {
+ end = priv->rl_bufsize;
+ }
+
+ start = header->rl_buffer + begin;
+ pos = start;
+
+ while (pos != (header->rl_buffer + end) && *pos++ != '\n');
Review Comment:
why need find `\n`? let's up_nputs handle multiple `\n` to `\r\n` byself.
##########
drivers/syslog/ramlog.c:
##########
@@ -337,28 +422,46 @@ static ssize_t ramlog_addbuf(FAR struct ramlog_dev_s
*priv,
if (len > 0)
{
- /* Lock the scheduler do NOT switch out */
-
- if (!up_interrupt_context())
+ if (!list_is_empty(&priv->rl_list))
{
+ /* Lock the scheduler do NOT switch out */
+
sched_lock();
- }
#ifndef CONFIG_RAMLOG_NONBLOCKING
- /* Are there threads waiting for read data? */
+ /* Are there threads waiting for read data? */
- ramlog_readnotify(priv);
+ ramlog_readnotify(priv);
#endif
- /* Notify all poll/select waiters that they can read from the FIFO */
+ /* Notify all poll/select waiters that they can
+ * read from the FIFO
+ */
- ramlog_pollnotify(priv);
+ ramlog_pollnotify(priv);
- /* Unlock the scheduler */
+ /* Unlock the scheduler */
- if (!up_interrupt_context())
- {
sched_unlock();
}
+#ifdef CONFIG_RAMLOG_FLUSH_WORKER
+ else if (priv == &g_sysdev)
+ {
+ if (header->rl_head - priv->rl_tail >= CONFIG_RAMLOG_POLLTHRESHOLD)
+ {
+ work_queue(LPWORK, &priv->rl_work,
+ ramlog_flush_worker, (FAR void *)priv, 0);
+ }
+ else
+ {
+ if (work_available(&priv->rl_work))
Review Comment:
merge with line 454
##########
drivers/syslog/ramlog.c:
##########
@@ -234,6 +243,82 @@ static void ramlog_pollnotify(FAR struct ramlog_dev_s
*priv)
}
}
+/****************************************************************************
+ * Name: ramlog_flush_internal
+ ****************************************************************************/
+
+#ifdef CONFIG_RAMLOG_FLUSH
+static void ramlog_flush_internal(FAR struct ramlog_dev_s *priv, bool force)
+{
+ FAR struct ramlog_header_s *header = priv->rl_header;
+ FAR const char *start;
+ FAR const char *pos;
+ irqstate_t flags;
+ uint32_t begin;
+ uint32_t end;
+
+ flags = enter_critical_section();
+
+ do
+ {
+ if (header->rl_head == priv->rl_tail)
+ {
+ break;
+ }
+
+ if (header->rl_head - priv->rl_tail > priv->rl_bufsize)
+ {
+ priv->rl_tail = header->rl_head - priv->rl_bufsize;
+ }
+
+ begin = priv->rl_tail % priv->rl_bufsize;
+ end = header->rl_head % priv->rl_bufsize;
+
+ if (end <= begin)
+ {
+ end = priv->rl_bufsize;
+ }
+
+ start = header->rl_buffer + begin;
+ pos = start;
+
+ while (pos != (header->rl_buffer + end) && *pos++ != '\n');
+
+ up_nputs(start, pos - start);
+ priv->rl_tail += pos - start;
+ }
+ while ((header->rl_head - priv->rl_tail > CONFIG_RAMLOG_POLLTHRESHOLD)
+ || force);
+
+ leave_critical_section(flags);
+}
+#endif
+
+/****************************************************************************
+ * Name: ramlog_flush_worker
+ ****************************************************************************/
+
+#ifdef CONFIG_RAMLOG_FLUSH_WORKER
+static void ramlog_flush_worker(FAR void *arg)
+{
+ FAR struct ramlog_dev_s *priv = arg;
+ FAR struct ramlog_header_s *header = priv->rl_header;
+ irqstate_t flags;
+
+ ramlog_flush_internal(arg, false);
+
+ flags = enter_critical_section();
+
+ if (header->rl_head != priv->rl_tail)
Review Comment:
why not flush all buffer in once
##########
drivers/syslog/Kconfig:
##########
@@ -54,7 +54,36 @@ config RAMLOG_POLLTHRESHOLD
---help---
When the length of circular buffer exceeds the threshold value,
the poll() will
return POLLIN to all poll waiters.
-endif
+
+config RAMLOG_FLUSH
+ bool "RAMLOG lower flush support"
+ default n
+ depends on ARCH_LOWPUTC
+ ---help---
+ RAMLOG lower flush support, this option will flush any buffered
data to the SYSLOG device.
+
+if RAMLOG_FLUSH
+
+config RAMLOG_FLUSH_WORKER
+ bool "RAMLOG flush worker"
+ default n
+ depends on SCHED_WORKQUEUE
+ ---help---
+ ROMLOG lower flush worker to flush the ramlog to lower half
driver.
+
+if RAMLOG_FLUSH_WORKER
Review Comment:
change RAMLOG_FLUSH_WORKER_TIMEOUT_MS to depend on RAMLOG_FLUSH_WORKER
--
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: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
