syslog/inbuffer: refactor intbuffer to circbuf

Replace character copy with string copy to improve performance

Signed-off-by: chao an <anchao@lixiang.com>

drivers/syslog/syslog.h

@@ -196,8 +196,7 @@ void syslog_register(void);
  *   ch - The character to add to the interrupt buffer (must be positive).
  *
  * Returned Value:
- *   Zero success, the character is echoed back to the caller.  A negated
+ *   None
- *   errno value is returned on any failure.
  *
  * Assumptions:
  *   Called only from interrupt handling logic; Interrupts will be disabled.
@@ -205,7 +204,7 @@ void syslog_register(void);
  ****************************************************************************/
 
 #ifdef CONFIG_SYSLOG_INTBUFFER
-int syslog_add_intbuffer(int ch);
+void syslog_add_intbuffer(FAR const char *buffer, size_t buflen);
 #endif
 
 /****************************************************************************
@@ -219,8 +218,7 @@ int syslog_add_intbuffer(int ch);
  *   force   - Use the force() method of the channel vs. the putc() method.
  *
  * Returned Value:
- *   On success, the character is echoed back to the caller.  A negated
+ *   None
- *   errno value is returned on any failure.
  *
  * Assumptions:
  *   Interrupts may or may not be disabled.
@@ -228,7 +226,7 @@ int syslog_add_intbuffer(int ch);
  ****************************************************************************/
 
 #ifdef CONFIG_SYSLOG_INTBUFFER
-int syslog_flush_intbuffer(bool force);
+void syslog_flush_intbuffer(bool force);
 #endif
 
 /****************************************************************************

drivers/syslog/syslog_intbuffer.c

@@ -32,7 +32,8 @@
 #include <errno.h>
 
 #include <nuttx/syslog/syslog.h>
-#include <nuttx/irq.h>
+#include <nuttx/spinlock.h>
+#include <nuttx/circbuf.h>
 
 #include "syslog.h"
 
@@ -53,79 +54,71 @@
 
 /* This structure encapsulates the interrupt buffer state */
 
-struct g_syslog_intbuffer_s
+struct syslog_intbuffer_s
 {
-  volatile uint16_t si_inndx;
+  struct circbuf_s circ;
-  volatile uint16_t si_outndx;
+  spinlock_t       splock;
-  uint8_t si_buffer[CONFIG_SYSLOG_INTBUFSIZE];
+  uint8_t          buffer[CONFIG_SYSLOG_INTBUFSIZE];
 };
 
 /****************************************************************************
  * Private Data
  ****************************************************************************/
 
-static struct g_syslog_intbuffer_s g_syslog_intbuffer;
+static struct syslog_intbuffer_s g_si_buffer =
+{
+  CIRCBUF_INITIALIZER(g_si_buffer.buffer, sizeof(g_si_buffer.buffer)),
+  SP_UNLOCKED,
+};
 
 /****************************************************************************
- * Private Data
+ * Private Functions
  ****************************************************************************/
 
 /****************************************************************************
- * Name: syslog_remove_intbuffer
+ * Name: syslog_flush_internal
  *
  * Description:
- *   Extract any characters that may have been added to the interrupt buffer
+ *   Flush any characters that may have been added to the interrupt buffer
  *   to the SYSLOG device.
  *
  * Input Parameters:
- *   None
+ *   force   - Use the force() method of the channel vs. the putc() method.
  *
  * Returned Value:
- *   On success, the extracted character is returned.  EOF is returned if
+ *   None
- *   the interrupt buffer is empty.
  *
  * Assumptions:
  *   Interrupts may or may not be disabled.
  *
  ****************************************************************************/
 
-static int syslog_remove_intbuffer(void)
+void syslog_flush_internal(bool force, size_t buflen)
 {
   irqstate_t flags;
-  uint32_t outndx;
+  char *buffer;
-  int ret = EOF;
+  size_t size;
 
-  /* Extraction of the character and adjustment of the circular buffer
+  /* This logic is performed with the scheduler disabled to protect from
-   * indices must be performed in a critical section to protect from
+   * concurrent modification by other tasks.
-   * concurrent modification from interrupt handlers.
    */
 
-  flags = enter_critical_section();
+  flags = spin_lock_irqsave_wo_note(&g_si_buffer.splock);
 
-  /* Check if the interrupt buffer is empty */
+  do
-
-  outndx = (uint32_t)g_syslog_intbuffer.si_outndx;
-  if (outndx != (uint32_t)g_syslog_intbuffer.si_inndx)
     {
-      /* Not empty.. Take the next character from the interrupt buffer */
+      buffer = circbuf_get_readptr(&g_si_buffer.circ, &size);
-
+      if (size > 0)
-      ret = g_syslog_intbuffer.si_buffer[outndx];
-
-      /* Increment the OUT index, handling wrap-around */
-
-      if (++outndx >= CONFIG_SYSLOG_INTBUFSIZE)
         {
-          outndx -= CONFIG_SYSLOG_INTBUFSIZE;
+          size = (size >= buflen) ? buflen : size;
+          syslog_write_foreach(buffer, size, force);
+          circbuf_readcommit(&g_si_buffer.circ, size);
+          buflen -= size;
         }
-
-      g_syslog_intbuffer.si_outndx = (uint16_t)outndx;
     }
+  while (size > 0 && buflen > 0);
 
-  leave_critical_section(flags);
+  spin_unlock_irqrestore_wo_note(&g_si_buffer.splock, flags);
-
-  /* Now we can send the extracted character to the SYSLOG device */
-
-  return ret;
 }
 
 /****************************************************************************
@@ -144,8 +137,7 @@ static int syslog_remove_intbuffer(void)
  *   ch - The character to add to the interrupt buffer (must be positive).
  *
  * Returned Value:
- *   Zero success, the character is echoed back to the caller.  A negated
+ *   None
- *   errno value is returned on any failure.
  *
  * Assumptions:
  *   - Called either from (1) interrupt handling logic with interrupts
@@ -155,57 +147,35 @@ static int syslog_remove_intbuffer(void)
  *
  ****************************************************************************/
 
-int syslog_add_intbuffer(int ch)
+void syslog_add_intbuffer(FAR const char *buffer, size_t buflen)
 {
   irqstate_t flags;
-  uint32_t inndx;
+  size_t space;
-  uint32_t outndx;
-  uint32_t endndx;
-  unsigned int inuse;
-  int ret = OK;
 
   /* Disable concurrent modification from interrupt handling logic */
 
-  flags = enter_critical_section();
+  flags = spin_lock_irqsave_wo_note(&g_si_buffer.splock);
 
-  /* How much space is left in the intbuffer? */
+  space = circbuf_space(&g_si_buffer.circ);
 
-  inndx  = (uint32_t)g_syslog_intbuffer.si_inndx;
+  if (space >= buflen)
-  outndx = (uint32_t)g_syslog_intbuffer.si_outndx;
-
-  endndx = inndx;
-  if (endndx < outndx)
     {
-      endndx += CONFIG_SYSLOG_INTBUFSIZE;
+      circbuf_write(&g_si_buffer.circ, buffer, buflen);
+    }
+  else if (buflen <= sizeof(g_si_buffer.buffer))
+    {
+      syslog_flush_internal(true, buflen - space);
+      circbuf_write(&g_si_buffer.circ, buffer, buflen);
+    }
+  else
+    {
+      syslog_flush_intbuffer(true);
+      space = buflen - sizeof(g_si_buffer.buffer);
+      syslog_write_foreach(buffer, space, true);
+      circbuf_write(&g_si_buffer.circ, buffer + space, buflen - space);
     }
 
-  inuse = (unsigned int)(endndx - outndx);
+  spin_unlock_irqrestore_wo_note(&g_si_buffer.splock, flags);
-
-  /* Is there space for another character */
-
-  if (inuse == CONFIG_SYSLOG_INTBUFSIZE - 1)
-    {
-      char oldch = syslog_remove_intbuffer();
-
-      syslog_write_foreach(&oldch, 1, true);
-
-      ret = -ENOSPC;
-    }
-
-  /* Copy one character */
-
-  g_syslog_intbuffer.si_buffer[inndx] = (uint8_t)ch;
-
-  /* Increment the IN index, handling wrap-around */
-
-  if (++inndx >= CONFIG_SYSLOG_INTBUFSIZE)
-    {
-      inndx -= CONFIG_SYSLOG_INTBUFSIZE;
-    }
-
-  g_syslog_intbuffer.si_inndx = (uint16_t)inndx;
-  leave_critical_section(flags);
-  return ret;
 }
 
 /****************************************************************************
@@ -227,41 +197,9 @@ int syslog_add_intbuffer(int ch)
  *
  ****************************************************************************/
 
-int syslog_flush_intbuffer(bool force)
+void syslog_flush_intbuffer(bool force)
 {
-  irqstate_t flags;
+  syslog_flush_internal(force, sizeof(g_si_buffer.buffer));
-  char c;
-  int ch;
-
-  /* This logic is performed with the scheduler disabled to protect from
-   * concurrent modification by other tasks.
-   */
-
-  flags = enter_critical_section();
-
-  for (; ; )
-    {
-      /* Transfer one character to time.  This is inefficient, but is
-       * done in this way to: (1) Deal with concurrent modification of
-       * the interrupt buffer from interrupt activity, (2) Avoid keeper
-       * interrupts disabled for a long time, and (3) to handler
-       * wrap-around of the circular buffer indices.
-       */
-
-      ch = syslog_remove_intbuffer();
-      if (ch == EOF)
-        {
-          break;
-        }
-
-      c = (char)ch;
-
-      syslog_write_foreach(&c, 1, force);
-    }
-
-  leave_critical_section(flags);
-
-  return ch;
 }
 
 #endif /* CONFIG_SYSLOG_INTBUFFER */

drivers/syslog/syslog_write.c

@@ -233,13 +233,7 @@ ssize_t syslog_write(FAR const char *buffer, size_t buflen)
 #ifdef CONFIG_SYSLOG_INTBUFFER
   if (force)
     {
-      size_t nwritten;
+      syslog_add_intbuffer(buffer, buflen);
-
-      for (nwritten = 0; nwritten < buflen; nwritten++)
-        {
-          syslog_add_intbuffer(buffer[nwritten]);
-        }
-
       return buflen;
     }
   else