From 0331703fc700d57e2556971847bf0d4a4a162de1 Mon Sep 17 00:00:00 2001 From: chao an Date: Mon, 16 Dec 2024 16:43:23 +0800 Subject: [PATCH] syslog/inbuffer: refactor intbuffer to circbuf Replace character copy with string copy to improve performance Signed-off-by: chao an --- drivers/syslog/syslog.h | 10 +- drivers/syslog/syslog_intbuffer.c | 168 ++++++++++-------------------- drivers/syslog/syslog_write.c | 8 +- 3 files changed, 58 insertions(+), 128 deletions(-) diff --git a/drivers/syslog/syslog.h b/drivers/syslog/syslog.h index 13d4acb20d0..af32be328a4 100644 --- a/drivers/syslog/syslog.h +++ b/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 - * errno value is returned on any failure. + * None * * 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 - * errno value is returned on any failure. + * None * * 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 /**************************************************************************** diff --git a/drivers/syslog/syslog_intbuffer.c b/drivers/syslog/syslog_intbuffer.c index 8af4e46de4f..9fe06333b22 100644 --- a/drivers/syslog/syslog_intbuffer.c +++ b/drivers/syslog/syslog_intbuffer.c @@ -32,7 +32,8 @@ #include #include -#include +#include +#include #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; - volatile uint16_t si_outndx; - uint8_t si_buffer[CONFIG_SYSLOG_INTBUFSIZE]; + struct circbuf_s circ; + spinlock_t splock; + 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 - * the interrupt buffer is empty. + * None * * 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; - int ret = EOF; + char *buffer; + size_t size; - /* 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. + /* This logic is performed with the scheduler disabled to protect from + * concurrent modification by other tasks. */ - flags = enter_critical_section(); + flags = spin_lock_irqsave_wo_note(&g_si_buffer.splock); - /* Check if the interrupt buffer is empty */ - - outndx = (uint32_t)g_syslog_intbuffer.si_outndx; - if (outndx != (uint32_t)g_syslog_intbuffer.si_inndx) + do { - /* Not empty.. Take the next character from the interrupt buffer */ - - ret = g_syslog_intbuffer.si_buffer[outndx]; - - /* Increment the OUT index, handling wrap-around */ - - if (++outndx >= CONFIG_SYSLOG_INTBUFSIZE) + buffer = circbuf_get_readptr(&g_si_buffer.circ, &size); + if (size > 0) { - 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); - - /* Now we can send the extracted character to the SYSLOG device */ - - return ret; + spin_unlock_irqrestore_wo_note(&g_si_buffer.splock, flags); } /**************************************************************************** @@ -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 - * errno value is returned on any failure. + * None * * 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; - uint32_t outndx; - uint32_t endndx; - unsigned int inuse; - int ret = OK; + size_t space; /* 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; - outndx = (uint32_t)g_syslog_intbuffer.si_outndx; - - endndx = inndx; - if (endndx < outndx) + if (space >= buflen) { - 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); - - /* 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; + spin_unlock_irqrestore_wo_note(&g_si_buffer.splock, flags); } /**************************************************************************** @@ -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; - 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; + syslog_flush_internal(force, sizeof(g_si_buffer.buffer)); } #endif /* CONFIG_SYSLOG_INTBUFFER */ diff --git a/drivers/syslog/syslog_write.c b/drivers/syslog/syslog_write.c index 7c3f57139b4..285f7547ab4 100644 --- a/drivers/syslog/syslog_write.c +++ b/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; - - for (nwritten = 0; nwritten < buflen; nwritten++) - { - syslog_add_intbuffer(buffer[nwritten]); - } - + syslog_add_intbuffer(buffer, buflen); return buflen; } else