Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

syslog/ramlog: add support of ramlog flush worker #13599

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion drivers/syslog/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change RAMLOG_FLUSH_WORKER_TIMEOUT_MS to depend on RAMLOG_FLUSH_WORKER


config RAMLOG_FLUSH_WORKER_TIMEOUT_MS
int "RAMLOG lower flush worker timeout in milliseconds"
default 10
---help---
ROMLOG flush work timeout in milliseconds, default is 10.

endif # RAMLOG_FLUSH_WORKER

endif # RAMLOG_FLUSH

endif # RAMLOG

config SYSLOG_BUFFER
bool "Use buffered output"
Expand Down
145 changes: 134 additions & 11 deletions drivers/syslog/ramlog.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@

#define RAMLOG_MAGIC_NUMBER 0x12345678

#ifdef CONFIG_RAMLOG_FLUSH_WORKER_TIMEOUT_MS
# define RAMLOG_FLUSH_WORKER_TIMEOUT_TICKS \
MSEC2TICK(CONFIG_RAMLOG_FLUSH_WORKER_TIMEOUT_MS)
#endif

/****************************************************************************
* Private Types
****************************************************************************/
Expand Down Expand Up @@ -98,6 +103,10 @@ struct ramlog_dev_s

uint32_t rl_bufsize; /* Size of the Circular RAM buffer */
struct list_node rl_list; /* The head of ramlog_user_s list */
#ifdef CONFIG_RAMLOG_FLUSH_WORKER
struct work_s rl_work; /* For deferring notifications to LPWORK queue */
volatile uint32_t rl_tail; /* The tail index (where data is removed) */
#endif
};

/****************************************************************************
Expand Down Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change begin/end to tail/head

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');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why need find \n? let's up_nputs handle multiple \n to \r\n byself.


up_nputs(start, pos - start);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why need dup to up_nputs? which may generate the infinite recursion.

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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not flush all buffer in once

{
work_queue(LPWORK, &priv->rl_work, ramlog_flush_worker,
(FAR void *)priv, RAMLOG_FLUSH_WORKER_TIMEOUT_TICKS);
}

leave_critical_section(flags);
}
#endif

/****************************************************************************
* Name: ramlog_copybuf
****************************************************************************/
Expand Down Expand Up @@ -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))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge with line 454

{
work_queue(LPWORK, &priv->rl_work,
ramlog_flush_worker, (FAR void *)priv,
RAMLOG_FLUSH_WORKER_TIMEOUT_TICKS);
}
}
}
#endif
}

/* We always have to return the number of bytes requested and NOT the
Expand Down Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not merge ramlog_flush_internal into ramlog_flush?

return 0;
}
#endif

/****************************************************************************
* Name: ramlog_register
*
Expand Down
4 changes: 4 additions & 0 deletions drivers/syslog/syslog_channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,11 @@ static const struct syslog_channel_ops_s g_ramlog_channel_ops =
{
ramlog_putc,
ramlog_putc,
#ifdef CONFIG_RAMLOG_FLUSH
ramlog_flush,
#else
NULL,
#endif
ramlog_write
};

Expand Down
16 changes: 16 additions & 0 deletions include/nuttx/syslog/ramlog.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,22 @@ ssize_t ramlog_write(FAR syslog_channel_t *channel,
FAR const char *buffer, size_t buflen);
#endif

/****************************************************************************
* 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);
#endif

#undef EXTERN
#ifdef __cplusplus
}
Expand Down
Loading