SYSLOG: Flesh out interrupt buffer logic

drivers/Kconfig

@@ -524,7 +524,4 @@ menuconfig DRIVERS_WIRELESS
 		Drivers for various wireless devices.
 
 source drivers/wireless/Kconfig
-
-comment "System Logging Device Options"
-
 source drivers/syslog/Kconfig

drivers/syslog/Kconfig

@@ -5,6 +5,20 @@
 
 comment "System Logging"
 
+config SYSLOG_INTBUFFER
+	bool "Use interrupt buffer"
+	default n
+	---help---
+		Enables an interrupt buffer that will be used to serialize debug
+		output from interrupt handlers.
+
+config SYSLOG_INTBUFSIZE
+	int "Interrupt buffer size"
+	default 2048
+	depends on SYSLOG_INTBUFFER
+	---help---
+		The size of the interrupt buffer in bytes.
+
 config RAMLOG
 	bool "RAM log device support"
 	default n
@@ -77,7 +91,7 @@ config SYSLOG_CONSOLE
 	default n
 	depends on DEV_CONSOLE
 	---help---
-		Use the syslog logging device as a system console. If this feature is enabled 
+		Use the syslog logging device as a system console. If this feature is enabled
 		(along with DEV_CONSOLE), then all console output will be re-directed to syslog
 		output (syslog_putc). This is useful, for example, if the only console is a Telnet
 		console.  Then in that case, console output from non-Telnet threads will go to

drivers/syslog/syslog.h

@@ -42,6 +42,8 @@
 
 #include <nuttx/config.h>
 
+#include <stdbool.h>
+
 /****************************************************************************
  * Pre-processor Definitions
  ****************************************************************************/

drivers/syslog/syslog_channel.c

@@ -73,12 +73,14 @@ static int syslog_default_flush(void)
   return OK;
 }
 
+#ifndef CONFIG_SYSLOG_INTBUFFER
 static int syslog_force(int ch)
 {
   DEBUGASSERT(g_syslog_channel != NULL && g_syslog_channel->sc_force != NULL);
 
   return g_syslog_channel->sc_force(ch);
 }
+#endif
 
 /****************************************************************************
  * Public Functions
@@ -163,7 +165,7 @@ int syslog_putc(int ch)
        * buffer.
        */
 
-      (void)syslog_flush_intbuffer(&g_syslog_channel, false);
+      (void)syslog_flush_intbuffer(g_syslog_channel, false);
 #endif
 
       return g_syslog_channel->sc_putc(ch);
@@ -198,7 +200,7 @@ int syslog_flush(void)
    * buffer.
    */
 
-  (void)syslog_flush_intbuffer(&g_syslog_channel, true);
+  (void)syslog_flush_intbuffer(g_syslog_channel, true);
 #endif
 
   /* Then flush all of the buffered output to the SYSLOG device */

drivers/syslog/syslog_intbuffer.c

@@ -38,9 +38,13 @@
 
 #include <nuttx/config.h>
 
+#include <stdbool.h>
+#include <stdio.h>
 #include <assert.h>
+#include <errno.h>
 
 #include <nuttx/syslog/syslog.h>
+#include <nuttx/irq.h>
 
 #include "syslog.h"
 
@@ -50,10 +54,122 @@
  * Pre-processor Definitions
  ****************************************************************************/
 
+/* Extend the size of the interrupt buffer so that a "[truncated]\n"
+ * indication can be append to the end.
+ *
+ * The usable capacity of the interrupt buffer is (CONFIG_SYSLOG_INTBUFSIZE - 1).
+ */
+
+#define SYSLOG_BUFOVERRUN_MESSAGE  "[truncated]\n"
+#define SYSLOG_BUFOVERRUN_SIZE     13
+
+#if CONFIG_SYSLOG_INTBUFSIZE > (65535 - SYSLOG_BUFOVERRUN_SIZE)
+#  undef  CONFIG_SYSLOG_INTBUFSIZE
+#  define CONFIG_SYSLOG_INTBUFSIZE (65535 - SYSLOG_BUFOVERRUN_SIZE)
+#  define SYSLOG_INTBUFSIZE        65535
+#else
+#  define SYSLOG_INTBUFSIZE \
+     (CONFIG_SYSLOG_INTBUFSIZE + SYSLOG_BUFOVERRUN_SIZE)
+#endif
+
+#define USABLE_INTBUFSIZE          (CONFIG_SYSLOG_INTBUFSIZE - 1)
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/* This structure encapsulates the interrupt buffer state */
+
+struct g_syslog_intbuffer_s
+{
+  volatile uint16_t si_inndx;
+  volatile uint16_t si_outndx;
+  uint8_t si_buffer[SYSLOG_INTBUFSIZE];
+};
+
 /****************************************************************************
  * Private Data
  ****************************************************************************/
 
+static struct g_syslog_intbuffer_s g_syslog_intbuffer;
+static const char g_overrun_msg[SYSLOG_BUFOVERRUN_SIZE] = SYSLOG_BUFOVERRUN_MESSAGE;
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: syslog_remove_intbuffer
+ *
+ * Description:
+ *   Extract any characters that may have been added to the interrupt buffer
+ *   to the SYSLOG device.
+ *
+ * Input Parameters:
+ *   None
+ *
+ * Returned Value:
+ *   On success, the extracted character is returned.  EOF is returned if
+ *   the interrupt buffer is empty.
+ *
+ * Assumptions:
+ *   Interrupts may or may not be disabled.
+ *
+ ****************************************************************************/
+
+int syslog_remove_intbuffer(void)
+{
+  irqstate_t flags;
+  uint32_t inndx;
+  uint32_t outndx;
+  uint32_t endndx;
+  int inuse = 0;
+  int ch;
+
+  /* Extraction of the character and adjustment of the circular buffer
+   * indices must be performed in a critical section to protect from
+   * concurrent modification from interrupt handlers.
+   */
+
+  flags = enter_critical_section();
+
+  /* How much space is left in the inbuffer? */
+
+  inndx  = (uint32_t)g_syslog_intbuffer.si_inndx;
+  outndx = (uint32_t)g_syslog_intbuffer.si_outndx;
+  if (inndx != outndx)
+    {
+      /* Handle the case where the endndx has wrapped around */
+
+      endndx = outndx;
+      if (endndx < outndx)
+        {
+          endndx += SYSLOG_INTBUFSIZE;
+        }
+
+      inuse = (int)(endndx - outndx);
+
+      /* Take the next character from the interrupt buffer */
+
+      ch = g_syslog_intbuffer.si_buffer[outndx];
+
+      /* Increment the OUT index, handling wrap-around */
+
+      if (++outndx >= SYSLOG_INTBUFSIZE)
+        {
+          outndx -= SYSLOG_INTBUFSIZE;
+        }
+
+      g_syslog_intbuffer.si_outndx = (uint16_t)outndx;
+    }
+
+  leave_critical_section(flags);
+
+  /* Now we can send the extracted character to the SYSLOG device */
+
+  return (inuse > 0) ? ch : EOF;
+}
+
 /****************************************************************************
  * Public Functions
  ****************************************************************************/
@@ -74,16 +190,88 @@
  *   errno value is returned on any failure.
  *
  * Assumptions:
- *   Called only from interrupt handling logic; Interrupts will be disabled.
+ *   - Called only from interrupt handling logic; Interrupts will be
+ *     disabled.
+ *   - Requires caution because there may be an interrupted execution of
+ *     syslog_flush_intbuffer():  Only the outndx can be modified.
  *
  ****************************************************************************/
 
 int syslog_add_intbuffer(int ch)
 {
-#warning Missing logic
+  uint32_t inndx;
-  return -ENOSYS;
+  uint32_t outndx;
+  uint32_t endndx;
+  unsigned int inuse;
+  int i;
+
+  /* How much space is left in the inbuffer? */
+
+  inndx  = (uint32_t)g_syslog_intbuffer.si_inndx;
+  outndx = (uint32_t)g_syslog_intbuffer.si_outndx;
+
+  endndx = outndx;
+  if (endndx < outndx)
+    {
+      endndx += SYSLOG_INTBUFSIZE;
+    }
+
+  inuse = (unsigned int)(endndx - outndx);
+
+  /* Is there space for another character (reserving space for the overrun
+   * message)?
+   */
+
+  if (inuse == USABLE_INTBUFSIZE)
+    {
+      /* Copy the truncated message one character at a time, handing index
+       * wrap-around on each character.
+       */
+
+      for (i = 0; i < SYSLOG_BUFOVERRUN_SIZE; i++)
+        {
+          /* Copy one character */
+
+          g_syslog_intbuffer.si_buffer[inndx] = (uint8_t)g_overrun_msg[i];
+
+          /* Increment the IN index, handling wrap-around */
+
+          if (++inndx >= SYSLOG_INTBUFSIZE)
+            {
+              inndx -= SYSLOG_INTBUFSIZE;
+            }
+
+          DEBUGASSERT(inndx != outndx);
+        }
+
+      g_syslog_intbuffer.si_inndx = (uint16_t)inndx;
+      return -ENOSPC;
+    }
+  else if (inuse < USABLE_INTBUFSIZE)
+    {
+       /* Copy one character */
+
+       g_syslog_intbuffer.si_buffer[inndx] = (uint8_t)ch;
+
+       /* Increment the IN index, handling wrap-around */
+
+       if (++inndx >= SYSLOG_INTBUFSIZE)
+         {
+           inndx -= SYSLOG_INTBUFSIZE;
+         }
+
+      g_syslog_intbuffer.si_inndx = (uint16_t)inndx;
+      return OK;
+    }
+  else
+    {
+      /* This character goes to the bit bucket.  We have already copied
+       * the overrun message so there is nothing else to do.
+       */
+
+      return -ENOSPC;
+    }
 }
-#endif
 
 /****************************************************************************
  * Name: syslog_flush_intbuffer
@@ -108,8 +296,37 @@ int syslog_add_intbuffer(int ch)
 int syslog_flush_intbuffer(FAR const struct syslog_channel_s *channel,
                            bool force)
 {
-#warning Missing logic
+  syslog_putc_t putfunc;
-  return -ENOSYS;
+  int ch;
+  int ret = OK;
+
+  /* Select which putc function to use for this flush */
+
+  putfunc = force ? channel->sc_putc : channel->sc_force;
+
+  /* This logic is performed with the scheduler disabled to protect from
+   * concurrent modification by other tasks.
+   */
+
+  sched_lock();
+  do
+    {
+      /* Transfer one character to time.  This is inefficient, but is
+       * done in this way to: (1) Deal with concurrent modification of
+       * the interrutp buffer from interrupt activity, (2) Avoid keeper
+       * interrupts disabled for a long time, and (3) to handler
+       * wraparound of the circular buffer indices.
+       */
+
+      ch = syslog_remove_intbuffer();
+      if (ch != EOF)
+        {
+          ret = putfunc(ch);
+        }
+    }
+  while (ch != EOF && ret >= 0);
+
+  return ret;
 }
 
 #endif /* CONFIG_SYSLOG_INTBUFFER */

include/nuttx/syslog/syslog.h

@@ -49,6 +49,9 @@
  ****************************************************************************/
 /* Configuration ************************************************************/
 /* CONFIG_SYSLOG - Enables generic system logging features.
+ * CONFIG_SYSLOG_INTBUFFER - Enables an interrupt buffer that will be used
+ *   to serialize debug output from interrupt handlers.
+ * CONFIG_SYSLOG_INTBUFSIZE - The size of the interrupt buffer in bytes.
  * CONFIG_SYSLOG_DEVPATH - The full path to the system logging device
  *
  * In addition, some SYSLOG device must also be enabled that will provide
@@ -80,19 +83,31 @@
 #  define CONFIG_SYSLOG_DEVPATH "/dev/ttyS1"
 #endif
 
+#if defined(CONFIG_SYSLOG_INTBUFFER) && !defined(CONFIG_SYSLOG_INTBUFSIZE)
+#  define CONFIG_SYSLOG_INTBUFSIZE 2048
+#endif
+
+#if CONFIG_SYSLOG_INTBUFSIZE > 65535
+#  undef  CONFIG_SYSLOG_INTBUFSIZE
+#  define CONFIG_SYSLOG_INTBUFSIZE 65535
+#endif
+
 /****************************************************************************
  * Public Types
  ****************************************************************************/
 
 /* This structure provides the interface to a SYSLOG device */
 
+typedef CODE int (*syslog_putc_t)(int ch);
+typedef CODE int (*syslog_flush_t)(void);
+
 struct syslog_channel_s
 {
   /* I/O redirection methods */
 
-  CODE int (*sc_putc)(int ch);  /* Normal buffered output */
+  syslog_putc_t sc_putc;    /* Normal buffered output */
-  CODE int (*sc_force)(int ch); /* Low-level output for interrupt handlers */
+  syslog_putc_t sc_force;   /* Low-level output for interrupt handlers */
-  CODE int (*sc_flush)(void);   /* Flush buffered output (on crash) */
+  syslog_flush_t sc_flush;  /* Flush buffered output (on crash) */
 
   /* Implementation specific logic may follow */
 };