Syslog buffering: Use IOBs to buffer data, not an on-stack buffer

drivers/serial/serial.c

@@ -51,6 +51,7 @@
 
 #include <nuttx/irq.h>
 #include <nuttx/arch.h>
+#include <nuttx/sched.h>
 #include <nuttx/semaphore.h>
 #include <nuttx/fs/fs.h>
 #include <nuttx/serial/serial.h>
@@ -365,7 +366,7 @@ static ssize_t uart_write(FAR struct file *filep, FAR const char *buffer,
    * a little differently.
    */
 
-  if (up_interrupt_context() || getpid() == 0)
+  if (up_interrupt_context() || sched_idletask())
     {
 #ifdef CONFIG_SERIAL_REMOVABLE
       /* If the removable device is no longer connected, refuse to write to

drivers/syslog/Kconfig

@@ -86,26 +86,16 @@ config SYSLOG_BUFFER
 	bool "Use buffered output"
 	default n
 	depends on SYSLOG_WRITE
+	select MM_IOB
 	---help---
 		Enables an buffering logic that will be used to serialize debug
 		output from concurrent tasks. This enables allocation of one buffer
-		per thread, each of size CONFIG_SYSLOG_BUFSIZE.
+		per thread, each of size CONFIG_IOB_BUFSIZE.
 
 		The use of SYSLOG buffering is optional.  If not enabled, however,
 		then the output from multiple tasks that attempt to generate SYSLOG
 		output may be interleaved and difficult to read.
 
-config SYSLOG_BUFSIZE
-	int "Output buffer size"
-	default 32
-	range 1 65535
-	depends on SYSLOG_BUFFER
-	---help---
-		If CONFIG_SYSLOG_BUFFER is enabled, then CONFIG_SYSLOG_BUFSIZE
-		provides the size of the per-thread output buffer.  Setting
-		CONFIG_SYSLOG_BUFSIZE to a value less than one effectly disables
-		output SYSLOG buffering.
-
 config SYSLOG_INTBUFFER
 	bool "Use interrupt buffer"
 	default n

drivers/syslog/syslog_stream.c

@@ -44,8 +44,9 @@
 #include <assert.h>
 #include <errno.h>
 
-#include <nuttx/syslog/syslog.h>
 #include <nuttx/streams.h>
+#include <nuttx/mm/iob.h>
+#include <nuttx/syslog/syslog.h>
 
 #include "syslog.h"
 
@@ -59,55 +60,67 @@
 
 static void syslogstream_putc(FAR struct lib_outstream_s *this, int ch)
 {
-#ifdef CONFIG_SYSLOG_BUFFER
-  FAR struct lib_syslogstream_s *stream = (FAR struct lib_syslogstream_s *)this;
-
   /* Discard carriage returns */
 
   if (ch != '\r')
     {
-      /* Add the incoming character to the buffer */
+#ifdef CONFIG_SYSLOG_BUFFER
+      FAR struct lib_syslogstream_s *stream;
+      FAR struct iob_s *iob;
 
-      stream->buf[stream->nbuf] = ch;
+      DEBUGASSERT(this != NULL);
-      stream->nbuf++;
+      stream = (FAR struct lib_syslogstream_s *)this;
-      this->nput++;
+      iob    = stream->iob;
 
-      /* Is the buffer full?  Did we encounter a new line? */
+      /* Do we have an IO buffer? */
 
-      if (stream->nbuf >= CONFIG_SYSLOG_BUFSIZE || ch == '\n')
+      if (iob != NULL)
         {
-          /* Yes.. then flush the buffer */
+          /* Yes.. Add the incoming character to the buffer */
 
-          (void)this->flush(this);
+          iob->io_data[iob->io_len] = ch;
-        }
+          iob->io_len++;
-    }
-#else
-  int ret;
-
-  /* Try writing until the write was successful or until an irrecoverable
-   * error occurs.
-   */
-
-  do
-    {
-      /* Write the character to the supported logging device.  On failure,
-       * syslog_putc returns EOF with the errno value set;
-       */
-
-      ret = syslog_putc(ch);
-      if (ret != EOF)
-        {
           this->nput++;
-          return;
-        }
 
-      /* The special errno value -EINTR means that syslog_putc() was
+          /* Is the buffer full?  Did we encounter a new line? */
-       * awakened by a signal.  This is not a real error and must be
+
-       * ignored in this context.
+          if (iob->io_len >= CONFIG_IOB_BUFSIZE || ch == '\n')
-       */
+            {
-    }
+              /* Yes.. then flush the buffer */
-  while (get_errno() == -EINTR);
+
+              (void)this->flush(this);
+            }
+        }
+      else
 #endif
+        {
+          int ret;
+
+          /* Try writing until the write was successful or until an
+           * irrecoverable error occurs.
+           */
+
+          do
+            {
+              /* Write the character to the supported logging device.  On
+               * failure, syslog_putc returns EOF with the errno value set;
+               */
+
+              ret = syslog_putc(ch);
+              if (ret != EOF)
+                {
+                  this->nput++;
+                  return;
+                }
+
+              /* The special errno value -EINTR means that syslog_putc() was
+               * awakened by a signal.  This is not a real error and must be
+               * ignored in this context.
+               */
+            }
+          while (get_errno() == -EINTR);
+        }
+    }
 }
 
 /****************************************************************************
@@ -117,25 +130,31 @@ static void syslogstream_putc(FAR struct lib_outstream_s *this, int ch)
 #ifdef CONFIG_SYSLOG_BUFFER
 static int syslogstream_flush(FAR struct lib_outstream_s *this)
 {
-  FAR struct lib_syslogstream_s *stream = (FAR struct lib_syslogstream_s *)this;
+  FAR struct lib_syslogstream_s *stream;
+  FAR struct iob_s *iob;
   int ret = OK;
 
-  /* Is there anything buffered? */
+  DEBUGASSERT(this != NULL);
+  stream = (FAR struct lib_syslogstream_s *)this;
+  iob    = stream->iob;
 
-  if (stream->nbuf > 0)
+  /* Do we have an IO buffer? Is there anything buffered? */
+
+  if (iob != NULL && iob->io_len > 0)
     {
       /* Yes write the buffered data */
 
       do
         {
-          ssize_t nbytes = syslog_write(stream->buf, stream->nbuf);
+          ssize_t nbytes = syslog_write((FAR const char *)iob->io_data,
+                                        (size_t)iob->io_len);
           if (nbytes < 0)
             {
               ret = -get_errno();
             }
           else
             {
-              stream->nbuf = 0;
+              iob->io_len = 0;
               ret = OK;
             }
         }
@@ -151,7 +170,7 @@ static int syslogstream_flush(FAR struct lib_outstream_s *this)
  ****************************************************************************/
 
 /****************************************************************************
- * Name: syslogstream
+ * Name: syslogstream_create
  *
  * Description:
  *   Initializes a stream for use with the configured syslog interface.
@@ -159,23 +178,74 @@ static int syslogstream_flush(FAR struct lib_outstream_s *this)
  *
  * Input parameters:
  *   stream - User allocated, uninitialized instance of struct
- *            lib_syslogstream_s to be initialized.
+ *            lib_lowoutstream_s to be initialized.
  *
  * Returned Value:
  *   None (User allocated instance initialized).
  *
  ****************************************************************************/
 
-void syslogstream(FAR struct lib_syslogstream_s *stream)
+void syslogstream_create(FAR struct lib_syslogstream_s *stream)
 {
+  DEBUGASSERT(stream != NULL);
+
 #ifdef CONFIG_SYSLOG_BUFFER
+  FAR struct iob_s *iob;
+
+  /* Initialize the common fields */
+
   stream->public.put   = syslogstream_putc;
   stream->public.flush = syslogstream_flush;
   stream->public.nput  = 0;
-  stream->nbuf         = 0;
+
+  /* Allocate an IOB */
+
+  iob                  = iob_alloc(true);
+  stream->iob          = iob;
+
+  if (iob != NULL)
+    {
+      /* Initialize the IOB */
+
+      iob->io_len      = 0;
+      iob->io_offset   = 0;
+      iob->io_pktlen   = 0;
+    }
 #else
   stream->public.put   = syslogstream_putc;
   stream->public.flush = lib_noflush;
   stream->public.nput  = 0;
 #endif
 }
+
+/****************************************************************************
+ * Name: syslogstream_destroy
+ *
+ * Description:
+ *   Free resources held by the syslog stream.
+ *
+ * Input parameters:
+ *   stream - User allocated, uninitialized instance of struct
+ *            lib_lowoutstream_s to be initialized.
+ *
+ * Returned Value:
+ *   None (Resources freed).
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SYSLOG_BUFFER
+void syslogstream_destroy(FAR struct lib_syslogstream_s *stream)
+{
+  DEBUGASSERT(stream != NULL);
+
+  /* Verify that there is an IOB attached (there should be) */
+
+  if (stream->iob != NULL)
+    {
+      /* Free the IOB */
+
+      iob_free(stream->iob);
+      stream->iob = NULL;
+    }
+}
+#endif

drivers/syslog/vsyslog.c

@@ -68,15 +68,17 @@
 int _vsyslog(int priority, FAR const IPTR char *fmt, FAR va_list *ap)
 {
   struct lib_syslogstream_s stream;
+  int ret;
+
 #ifdef CONFIG_SYSLOG_TIMESTAMP
   struct timespec ts;
-  int ret = -1;
 
   /* Get the current time.  Since debug output may be generated very early
    * in the start-up sequence, hardware timer support may not yet be
    * available.
    */
 
+  ret = -EAGAIN;
   if (OSINIT_HW_READY())
     {
       /* Prefer monotonic when enabled, as it can be synchronized to
@@ -114,7 +116,7 @@ int _vsyslog(int priority, FAR const IPTR char *fmt, FAR va_list *ap)
     {
       /* Use the normal SYSLOG stream */
 
-      syslogstream(&stream);
+      syslogstream_create(&stream);
     }
 
 #if defined(CONFIG_SYSLOG_TIMESTAMP)
@@ -124,5 +126,18 @@ int _vsyslog(int priority, FAR const IPTR char *fmt, FAR va_list *ap)
                     ts.tv_sec, ts.tv_nsec/1000);
 #endif
 
-  return lib_vsprintf(&stream.public, fmt, *ap);
+  /* Generate the output */
+
+  ret = lib_vsprintf(&stream.public, fmt, *ap);
+
+#ifdef CONFIG_SYSLOG_BUFFER
+  /* Destroy the syslog stream buffer */
+
+  if (priority != LOG_EMERG)
+    {
+      syslogstream_destroy(&stream);
+    }
+#endif
+
+  return ret;
 }

include/nuttx/sched.h

@@ -760,7 +760,7 @@ FAR struct tcb_s *sched_self(void);
 
 void sched_foreach(sched_foreach_t handler, FAR void *arg);
 
-/* Give a task ID, look up the corresponding TCB */
+/* Given a task ID, look up the corresponding TCB */
 
 FAR struct tcb_s *sched_gettcb(pid_t pid);
 

include/nuttx/streams.h

@@ -190,14 +190,17 @@ struct lib_rawsostream_s
 };
 
 /* This is a special stream that does buffered character I/O.  NOTE that is
- * CONFIG_SYSLOG_BUFFER is not defined, it is the same as struct lib_outstream_s */
+ * CONFIG_SYSLOG_BUFFER is not defined, it is the same as struct
+ * lib_outstream_s
+ */
+
+struct iob_s;  /* Forward reference */
 
 struct lib_syslogstream_s
 {
   struct lib_outstream_s public;
 #ifdef CONFIG_SYSLOG_BUFFER
-  unsigned int           nbuf;
+  FAR struct iob_s *iob;
-  char                   buf[CONFIG_SYSLOG_BUFSIZE];
 #endif
 };
 
@@ -357,7 +360,7 @@ void lib_nullinstream(FAR struct lib_instream_s *nullinstream);
 void lib_nulloutstream(FAR struct lib_outstream_s *nulloutstream);
 
 /****************************************************************************
- * Name: syslogstream
+ * Name: syslogstream_create
  *
  * Description:
  *   Initializes a stream for use with the configured syslog interface.
@@ -372,7 +375,28 @@ void lib_nulloutstream(FAR struct lib_outstream_s *nulloutstream);
  *
  ****************************************************************************/
 
-void syslogstream(FAR struct lib_syslogstream_s *stream);
+void syslogstream_create(FAR struct lib_syslogstream_s *stream);
+
+/****************************************************************************
+ * Name: syslogstream_destroy
+ *
+ * Description:
+ *   Free resources held by the syslog stream.
+ *
+ * Input parameters:
+ *   stream - User allocated, uninitialized instance of struct
+ *            lib_lowoutstream_s to be initialized.
+ *
+ * Returned Value:
+ *   None (Resources freed).
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SYSLOG_BUFFER
+void syslogstream_destroy(FAR struct lib_syslogstream_s *stream);
+#else
+#  define syslogstream_destroy(s)
+#endif
 
 /****************************************************************************
  * Name: emergstream

mm/iob/iob_alloc.c

@@ -45,6 +45,7 @@
 
 #include <nuttx/irq.h>
 #include <nuttx/arch.h>
+#include <nuttx/sched.h>
 #include <nuttx/mm/iob.h>
 
 #include "iob.h"
@@ -172,7 +173,7 @@ FAR struct iob_s *iob_alloc(bool throttled)
 {
   /* Were we called from the interrupt level? */
 
-  if (up_interrupt_context())
+  if (up_interrupt_context() || sched_idletask())
     {
       /* Yes, then try to allocate an I/O buffer without waiting */