sergiotarxz/nuttx
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

drivers/syslog/ramlog.c: Fix ramlog readers never woken up when using ramlog as syslog or console.

Browse Source 
We also make an attempt to avoid the thundering herd problem if there are multiple readers/pollers.

Patch also removes forcing CONFIG_RAMLOG_CRLF in nuttx/syslog/ramlog.h as there is no point of wasting precious RAM for useless characters.
This commit is contained in:
Juha Niskanen 2019-11-14 07:40:35 -06:00 committed by Gregory Nutt
parent 8189381285
commit 99a501b668
4 changed files with 105 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

138
drivers/syslog/ramlog.c
View File

@ -101,8 +101,11 @@ static int ramlog_flush(void);
/* Helper functions */
static void ramlog_pollnotify(FAR struct ramlog_dev_s *priv,
pollevent_t eventset);
#ifndef CONFIG_RAMLOG_NONBLOCKING
static int ramlog_readnotify(FAR struct ramlog_dev_s *priv);
#endif
static void ramlog_pollnotify(FAR struct ramlog_dev_s *priv,
pollevent_t eventset);
static ssize_t ramlog_addchar(FAR struct ramlog_dev_s *priv, char ch);
/* Character driver methods */
@ -188,6 +191,32 @@ static int ramlog_flush(void)
}
#endif
/****************************************************************************
* Name: ramlog_readnotify
****************************************************************************/
#ifndef CONFIG_RAMLOG_NONBLOCKING
static int ramlog_readnotify(FAR struct ramlog_dev_s *priv)
{
irqstate_t flags;
int i;
/* Notify all waiting readers that they can read from the FIFO */
flags = enter_critical_section();
for (i = 0; i < priv->rl_nwaiters; i++)
{
nxsem_post(&priv->rl_waitsem);
}
leave_critical_section(flags);
/* Return number of notified readers. */
return i;
}
#endif
/****************************************************************************
* Name: ramlog_pollnotify
****************************************************************************/
@ -306,16 +335,12 @@ static ssize_t ramlog_read(FAR struct file *filep, FAR char *buffer, size_t len)
if (nread > 0)
{
/* Yes.. re-enable interrupts and the break out to return what
* we have.
*/
/* Yes.. break out to return what we have. */
break;
}
/* If the driver was opened with O_NONBLOCK option, then don't wait.
* Re-enable interrupts and return EGAIN.
*/
/* If the driver was opened with O_NONBLOCK option, then don't wait. */
if (filep->f_oflags & O_NONBLOCK)
{
@ -325,7 +350,7 @@ static ssize_t ramlog_read(FAR struct file *filep, FAR char *buffer, size_t len)
/* Otherwise, wait for something to be written to the circular
* buffer. Increment the number of waiters so that the ramlog_write()
* will not that it needs to post the semaphore to wake us up.
* will note that it needs to post the semaphore to wake us up.
*/
sched_lock();
@ -362,16 +387,12 @@ static ssize_t ramlog_read(FAR struct file *filep, FAR char *buffer, size_t len)
if (ret < 0)
{
/* No.. One of the two nxsem_wait's failed. */
/* Were we awakened by a signal? Did we read anything before
* we received the signal?
/* Return the error. We did handle the case where we read
* anything already before waiting.
*/
if (ret != -EINTR || nread >= 0)
{
/* Yes.. return the error. */
nread = ret;
}
nread = ret;
/* Break out to return what we have. Note, we can't exactly
* "break" out because whichever error occurred, we do not hold
@ -390,14 +411,14 @@ static ssize_t ramlog_read(FAR struct file *filep, FAR char *buffer, size_t len)
ch = priv->rl_buffer[priv->rl_tail];
/* Increment the tail index and re-enable interrupts */
/* Increment the tail index. */
if (++priv->rl_tail >= priv->rl_bufsize)
{
priv->rl_tail = 0;
}
/* Add the character to the user buffer */
/* Add the character to the user buffer. */
buffer[nread] = ch;
nread++;
@ -432,6 +453,7 @@ static ssize_t ramlog_write(FAR struct file *filep, FAR const char *buffer, size
{
FAR struct inode *inode = filep->f_inode;
FAR struct ramlog_dev_s *priv;
int readers_waken;
ssize_t nwritten;
char ch;
int ret;
@ -446,7 +468,7 @@ static ssize_t ramlog_write(FAR struct file *filep, FAR const char *buffer, size
*
* The write logic only needs to modify the rl_head index. Therefore,
* there is a difference in the way that rl_head and rl_tail are protected:
* rl_tail is protected with a semaphore; rl_tail is protected by disabling
* rl_tail is protected with a semaphore; rl_head is protected by disabling
* interrupts.
*/
@ -497,27 +519,29 @@ static ssize_t ramlog_write(FAR struct file *filep, FAR const char *buffer, size
if (nwritten > 0)
{
irqstate_t flags;
#ifndef CONFIG_RAMLOG_NONBLOCKING
int i;
#endif
readers_waken = 0;
#ifndef CONFIG_RAMLOG_NONBLOCKING
/* Are there threads waiting for read data? */
flags = enter_critical_section();
#ifndef CONFIG_RAMLOG_NONBLOCKING
for (i = 0; i < priv->rl_nwaiters; i++)
{
/* Yes.. Notify all of the waiting readers that more data is available */
nxsem_post(&priv->rl_waitsem);
}
readers_waken = ramlog_readnotify(priv);
#endif
/* Notify all poll/select waiters that they can write to the FIFO */
/* If there are multiple readers, some of them might block despite
* POLLIN because first reader might read all data. Favor readers
* and notify poll waiters only if no reader was awaken, even if the
* latter may starve.
*
* This also implies we do not have to make these two notify
* operations a critical section.
*/
ramlog_pollnotify(priv, POLLIN);
leave_critical_section(flags);
if (readers_waken == 0)
{
/* Notify all poll/select waiters that they can read from the FIFO */
ramlog_pollnotify(priv, POLLIN);
}
}
/* We always have to return the number of bytes requested and NOT the
@ -537,7 +561,8 @@ int ramlog_poll(FAR struct file *filep, FAR struct pollfd *fds, bool setup)
FAR struct inode *inode = filep->f_inode;
FAR struct ramlog_dev_s *priv;
pollevent_t eventset;
size_t ndx;
irqstate_t flags;
size_t next_head;
int ret;
int i;
@ -583,29 +608,31 @@ int ramlog_poll(FAR struct file *filep, FAR struct pollfd *fds, bool setup)
goto errout;
}
/* Should immediately notify on any of the requested events?
* First, check if the xmit buffer is full.
*/
/* Should immediately notify on any of the requested events? */
eventset = 0;
ndx = priv->rl_head + 1;
if (ndx >= priv->rl_bufsize)
flags = enter_critical_section();
next_head = priv->rl_head + 1;
if (next_head >= priv->rl_bufsize)
{
ndx = 0;
next_head = 0;
}
if (ndx != priv->rl_tail)
/* First, check if the receive buffer is not full. */
if (next_head != priv->rl_tail)
{
eventset |= POLLOUT;
}
/* Check if the receive buffer is empty */
/* Check if the receive buffer is not empty. */
if (priv->rl_head != priv->rl_tail)
{
eventset |= POLLIN;
}
leave_critical_section(flags);
if (eventset)
{
@ -756,6 +783,7 @@ int ramlog_syslog_channel(void)
int ramlog_putc(int ch)
{
FAR struct ramlog_dev_s *priv = &g_sysdev;
int readers_waken = 0;
int ret;
#ifdef CONFIG_RAMLOG_CRLF
@ -790,6 +818,28 @@ int ramlog_putc(int ch)
return ret;
}
#ifndef CONFIG_RAMLOG_NONBLOCKING
/* Are there threads waiting for read data? */
readers_waken = ramlog_readnotify(priv);
#endif
/* If there are multiple readers, some of them might block despite
* POLLIN because first reader might read all data. Favor readers
* and notify poll waiters only if no reader was awaken, even if the
* latter may starve.
*
* This also implies we do not have to make these two notify
* operations a critical section.
*/
if (readers_waken == 0)
{
/* Notify all poll/select waiters that they can read from the FIFO */
ramlog_pollnotify(priv, POLLIN);
}
/* Return the character added on success */
return ch;

17
include/nuttx/syslog/ramlog.h
View File

@ -2,7 +2,7 @@
* include/nuttx/syslog/ramlog.h
* The RAM logging driver
*
* Copyright (C) 2012, 2014-2015 Gregory Nutt. All rights reserved.
* Copyright (C) 2012, 2014-2015, 2019 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <gnutt@nuttx.org>
*
* Redistribution and use in source and binary forms, with or without
@ -33,6 +33,7 @@
* POSSIBILITY OF SUCH DAMAGE.
*
****************************************************************************/
/* The RAM logging driver is a driver that was intended to support debugging
* output (syslogging) when the normal serial output is not available. For
* example, if you are using a Telnet or USB serial console, the debug
@ -61,7 +62,9 @@
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
/* Configuration ************************************************************/
/* CONFIG_RAMLOG - Enables the RAM logging feature
* CONFIG_RAMLOG_CONSOLE - Use the RAM logging device as a system console.
* If this feature is enabled (along with CONFIG_DEV_CONSOLE), then all
@ -72,7 +75,7 @@
* CONFIG_RAMLOG_SYSLOG - Use the RAM logging device for the syslogging
* interface. If this feature is enabled then all debug output (only)
* will be re-directed to the circular buffer in RAM. This RAM log can
* be viewied from NSH using the 'dmesg' command. NOTE: Unlike the
* be viewed from NSH using the 'dmesg' command. NOTE: Unlike the
* limited, generic character driver SYSLOG device, the RAMLOG *can* be
* used to generate debug output from interrupt level handlers.
* CONFIG_RAMLOG_NPOLLWAITERS - The number of threads than can be waiting
@ -100,15 +103,6 @@
# define CONFIG_RAMLOG_BUFSIZE 1024
#endif
/* When used as a console or syslogging device, the RAM log will pre-pend
* line-feeds with carriage returns.
*/
#if defined(CONFIG_RAMLOG_CONSOLE) || defined(CONFIG_RAMLOG_SYSLOG)
# undef CONFIG_RAMLOG_CRLF
# define CONFIG_RAMLOG_CRLF 1
#endif
/****************************************************************************
* Public Data
****************************************************************************/
@ -126,6 +120,7 @@ extern "C"
/****************************************************************************
* Public Function Prototypes
****************************************************************************/
/****************************************************************************
* Name: ramlog_register
*

3
sched/sched/sched.h
View File

@ -322,11 +322,12 @@ extern volatile uint32_t g_cpuload_total;
#endif
/* Declared in sched_lock.c *************************************************/
/* Pre-emption is disabled via the interface sched_lock(). sched_lock()
* works by preventing context switches from the currently executing tasks.
* This prevents other tasks from running (without disabling interrupts) and
* gives the currently executing task exclusive access to the (single) CPU
* resources. Thus, sched_lock() and its companion, sched_unlcok(), are
* resources. Thus, sched_lock() and its companion, sched_unlock(), are
* used to implement some critical sections.
*
* In the single CPU case, Pre-emption is disabled using a simple lockcount

6
sched/sched/sched_lock.c
View File

@ -59,10 +59,10 @@
* works by preventing context switches from the currently executing tasks.
* This prevents other tasks from running (without disabling interrupts) and
* gives the currently executing task exclusive access to the (single) CPU
* resources. Thus, sched_lock() and its companion, sched_unlcok(), are
* resources. Thus, sched_lock() and its companion, sched_unlock(), are
* used to implement some critical sections.
*
* In the single CPU case, Pre-emption is disabled using a simple lockcount
* In the single CPU case, pre-emption is disabled using a simple lockcount
* in the TCB. When the scheduling is locked, the lockcount is incremented;
* when the scheduler is unlocked, the lockcount is decremented. If the
* lockcount for the task at the head of the g_readytorun list has a
@ -181,7 +181,7 @@ int sched_lock(void)
(void)up_fetchadd16(&g_global_lockcount, 1);
#endif
/* This operation is save if CONFIG_ARCH_HAVE_FETCHADD is defined. NOTE
/* This operation is safe if CONFIG_ARCH_HAVE_FETCHADD is defined. NOTE
* we cannot use this_task() because it calls sched_lock().
*/