Another try...
Regards,
Honza
Steven Dake wrote:
> _logsys_log_printf should duplicate the code in logsys_log_vprintf and
> not call logsys_log_vprintf directly. This api is called hundreds of
> thousands of times in a 1-2 minute run and as is can consume 1.5% of the
> cpu utilization. I don't want to make it worse.
>
> Regards
> -steve
>
>
>
> On Thu, 2009-04-23 at 11:28 +0200, Jan Friesse wrote:
>> Patch based on top of r2125.
>>
>> Performance impact should be one more call (shouldn't be so bad).
>>
>> Regards,
>> Honza
>>
>> Steven Dake wrote:
>>> I can't tell, but it looks like this could have some performance impact.
>>>
>>> Can you rework this patch on top of Fabio's logsys rework patch and
>>> possibly introduce a new internal but externally exported api that does
>>> the job for this special case without sprintfing in all cases or parsing
>>> the va args list in all cases (except when a va list is specified)?
>>>
>>> Regards
>>> -steve
>>>
>>> On Tue, 2009-03-31 at 17:04 +0200, Jan Friesse wrote:
>>>> Attached patch solves problem with ipc_log_printf, which calls
>>>> _logsys_log_printf. Arguments for this function was va_list, so logged
>>>> information was va_list address, instead of "what user wants".
>>>>
>>>> Solution is based on adding new function _logsys_log_vprintf, which
>>>> takes va_list as argument. Old _logsys_log_printf is implemented by new
>>>> _logsys_log_vprintf (no code duplication).
>>>>
>>>> Another problem which patch fixing is, that old logsys_log_printf used
>>>> vsprintf to static allocated buffer. This could cause buffer owerflow,
>>>> so vsnprintf is used now.
>>>>
>>>>
>>>> plain text document attachment (logsys_log_vprintf.patch)
>>>> Index: include/corosync/engine/logsys.h
>>>> ===================================================================
>>>> --- include/corosync/engine/logsys.h (revision 1974)
>>>> +++ include/corosync/engine/logsys.h (working copy)
>>>> @@ -155,6 +155,15 @@
>>>> const char *format,
>>>> ...) __attribute__((format(printf, 6, 7)));
>>>>
>>>> +extern void _logsys_log_vprintf (
>>>> + int subsys,
>>>> + const char *function_name,
>>>> + const char *file_name,
>>>> + int file_line,
>>>> + unsigned int level,
>>>> + const char *format,
>>>> + va_list ap) __attribute__((format(printf,6,0)));
>>>> +
>>>> extern void _logsys_log_rec (
>>>> int subsys,
>>>> const char *function_name,
>>>> Index: exec/logsys.c
>>>> ===================================================================
>>>> --- exec/logsys.c (revision 1974)
>>>> +++ exec/logsys.c (working copy)
>>>> @@ -795,18 +795,17 @@
>>>> records_written++;
>>>> }
>>>>
>>>> -void _logsys_log_printf (
>>>> +void _logsys_log_vprintf (
>>>> int subsys,
>>>> const char *function_name,
>>>> const char *file_name,
>>>> int file_line,
>>>> unsigned int level,
>>>> const char *format,
>>>> - ...)
>>>> + va_list ap)
>>>> {
>>>> char logsys_print_buffer[COMBINE_BUFFER_SIZE];
>>>> unsigned int len;
>>>> - va_list ap;
>>>>
>>>> if (logsys_mode & LOG_MODE_NOSUBSYS) {
>>>> subsys = 0;
>>>> @@ -814,9 +813,8 @@
>>>> if (level > logsys_loggers[subsys].priority) {
>>>> return;
>>>> }
>>>> - va_start (ap, format);
>>>> - len = vsprintf (logsys_print_buffer, format, ap);
>>>> - va_end (ap);
>>>> + len = vsnprintf (logsys_print_buffer, COMBINE_BUFFER_SIZE, format, ap);
>>>> +
>>>> if (logsys_print_buffer[len - 1] == '\n') {
>>>> logsys_print_buffer[len - 1] = '\0';
>>>> len -= 1;
>>>> @@ -849,6 +847,22 @@
>>>> }
>>>> }
>>>>
>>>> +void _logsys_log_printf (
>>>> + int subsys,
>>>> + const char *function_name,
>>>> + const char *file_name,
>>>> + int file_line,
>>>> + unsigned int level,
>>>> + const char *format,
>>>> + ...)
>>>> +{
>>>> + va_list ap;
>>>> +
>>>> + va_start (ap, format);
>>>> + _logsys_log_vprintf(subsys, function_name, file_name, file_line, level,
>>>> format, ap);
>>>> + va_end (ap);
>>>> +}
>>>> +
>>>> /*
>>>> * External Configuration and Initialization API
>>>> */
>>>> Index: exec/main.c
>>>> ===================================================================
>>>> --- exec/main.c (revision 1974)
>>>> +++ exec/main.c (working copy)
>>>> @@ -601,7 +601,7 @@
>>>>
>>>> va_start (ap, format);
>>>>
>>>> - _logsys_log_printf (ipc_subsys_id, __FUNCTION__,
>>>> + _logsys_log_vprintf (ipc_subsys_id, __FUNCTION__,
>>>> __FILE__, __LINE__, LOG_LEVEL_ERROR, format, ap);
>>>>
>>>> va_end (ap);
>>>> _______________________________________________
>>>> Openais mailing list
>>>> [email protected]
>>>> https://lists.linux-foundation.org/mailman/listinfo/openais
>
commit 08d159ab674a4c853c7ec78d19fec08752379c84
Author: Jan Friesse <[email protected]>
Date: Thu Apr 23 11:59:51 2009 +0200
Logsys try2
diff --git a/trunk/exec/logsys.c b/trunk/exec/logsys.c
index 9b6f727..723eddd 100644
--- a/trunk/exec/logsys.c
+++ b/trunk/exec/logsys.c
@@ -1065,6 +1065,60 @@ void _logsys_log_rec (
records_written++;
}
+void _logsys_log_vprintf (
+ int subsysid,
+ const char *function_name,
+ const char *file_name,
+ int file_line,
+ unsigned int level,
+ const char *format,
+ va_list ap)
+{
+ char logsys_print_buffer[COMBINE_BUFFER_SIZE];
+ unsigned int len;
+
+ if (subsysid <= -1) {
+ subsysid = LOGSYS_MAX_SUBSYS_COUNT;
+ }
+
+ if ((level > logsys_loggers[subsysid].syslog_priority) &&
+ (level > logsys_loggers[subsysid].logfile_priority)) {
+ return;
+ }
+
+ len = vsprintf (logsys_print_buffer, format, ap);
+ if (logsys_print_buffer[len - 1] == '\n') {
+ logsys_print_buffer[len - 1] = '\0';
+ len -= 1;
+ }
+
+ /*
+ * Create a log record
+ */
+ _logsys_log_rec (subsysid,
+ function_name,
+ file_name,
+ file_line,
+ (level+1) << 28,
+ logsys_print_buffer, len + 1,
+ LOGSYS_REC_END);
+
+ if ((logsys_loggers[LOGSYS_MAX_SUBSYS_COUNT].mode & LOGSYS_MODE_THREADED) == 0) {
+ /*
+ * Output (and block) if the log mode is not threaded otherwise
+ * expect the worker thread to output the log data once signaled
+ */
+ log_printf_to_logs (logsys_loggers[subsysid].subsys,
+ file_name, function_name, file_line, level,
+ logsys_print_buffer);
+ } else {
+ /*
+ * Signal worker thread to display logging output
+ */
+ wthread_signal ();
+ }
+}
+
void _logsys_log_printf (
int subsysid,
const char *function_name,
diff --git a/trunk/exec/main.c b/trunk/exec/main.c
index 51f87b4..b9efb18 100644
--- a/trunk/exec/main.c
+++ b/trunk/exec/main.c
@@ -604,12 +604,13 @@ static void corosync_sending_allowed_release (void *sending_allowed_private_data
static int ipc_subsys_id = -1;
+static void ipc_log_printf (const char *format, ...) __attribute__((format(printf, 1, 2)));
static void ipc_log_printf (const char *format, ...) {
va_list ap;
va_start (ap, format);
- _logsys_log_printf (ipc_subsys_id, __FUNCTION__,
+ _logsys_log_vprintf (ipc_subsys_id, __FUNCTION__,
__FILE__, __LINE__, LOGSYS_LEVEL_ERROR, format, ap);
va_end (ap);
diff --git a/trunk/include/corosync/engine/logsys.h b/trunk/include/corosync/engine/logsys.h
index 2606196..916c138 100644
--- a/trunk/include/corosync/engine/logsys.h
+++ b/trunk/include/corosync/engine/logsys.h
@@ -114,6 +114,15 @@ extern unsigned int _logsys_subsys_create (const char *subsys);
extern int _logsys_rec_init (unsigned int size);
+extern void _logsys_log_vprintf (
+ int subsysid,
+ const char *function_name,
+ const char *file_name,
+ int file_line,
+ unsigned int level,
+ const char *format,
+ va_list ap) __attribute__((format(printf, 6, 0)));
+
extern void _logsys_log_printf (
int subsysid,
const char *function_name,
_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais
