From 661866b7559be43fa58ec25f9783daf0632b4f03 Mon Sep 17 00:00:00 2001 From: Michal Bloch Date: Tue, 13 Dec 2022 17:28:02 +0100 Subject: [PATCH] libdlog: implement a null backend Set via 'backend=null' config entry, drops logs. Note that backend-independent features (like limiter) need to be disabled separately; a conf file is provided. Change-Id: Id0afe298960ae8a57d72b0eba3b4cdb33811e7f0 Signed-off-by: Michal Bloch --- Makefile.am | 1 + configs/86-disable-dlog.conf_inactive | 12 ++++++++++++ include/dlogutil.h | 1 + include/logcommon.h | 3 ++- packaging/dlog.spec | 2 ++ src/libdlog/log.c | 25 +++++++++++++++++-------- src/libdlogutil/lib.c | 9 +++++++++ src/libdlogutil/logretrieve.c | 2 ++ src/log-redirect-stdout/internal.c | 11 +++++++++++ src/log-redirect-stdout/lib.c | 14 ++++++++++++++ src/logger/logger.c | 5 +++++ src/logger/logger_internal.h | 3 ++- src/shared/logconfig.c | 6 ++++++ src/tests/libdlog_base_pos.c | 7 +++++++ 14 files changed, 91 insertions(+), 10 deletions(-) create mode 100644 configs/86-disable-dlog.conf_inactive diff --git a/Makefile.am b/Makefile.am index e7b96aa..ee8327b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -894,6 +894,7 @@ dlogconf_DATA = \ configs/20-pipe.conf \ configs/25-logger.conf \ configs/30-zero-copy.conf \ + configs/86-disable-dlog.conf_inactive \ configs/99-dlog-logger.disable-platform-logging-for-testsuite.conf modulesloadddir = /usr/lib/modules/modules-load.d diff --git a/configs/86-disable-dlog.conf_inactive b/configs/86-disable-dlog.conf_inactive new file mode 100644 index 0000000..2e2c081 --- /dev/null +++ b/configs/86-disable-dlog.conf_inactive @@ -0,0 +1,12 @@ +# This config file contains values that remove all possible features, minimizing +# dlog overhead. By default this file is not active, rename it so that the file +# extension is just '.conf'. Use for debugging dlog itself or for performance +# related reasons (for example raw measurements). + +backend=null +limiter=0 +deduplicate_method= +qos_file_path= +dynamic_config_path= +stash_failed_log_method= + diff --git a/include/dlogutil.h b/include/dlogutil.h index 7936709..2587710 100644 --- a/include/dlogutil.h +++ b/include/dlogutil.h @@ -370,6 +370,7 @@ int dlogutil_config_mode_set_compressed_memory_dump(dlogutil_config_s *config, c * @retval TIZEN_ERROR_INVALID_PARAMETER No buffers selected * @retval TIZEN_ERROR_NOT_SUPPORTED Unsupported buffer set (KMSG + non-KMSG) * @retval TIZEN_ERROR_NOT_SUPPORTED Unsupported backend (zero-copy) + * @retval TIZEN_ERROR_NO_DATA No buffers were opened (incl. due to null backend) * @retval TIZEN_ERROR_IO_ERROR Couldn't read config file * @retval TIZEN_ERROR_IO_ERROR Couldn't contact log backend * @retval TIZEN_ERROR_OUT_OF_MEMORY There's not enough memory diff --git a/include/logcommon.h b/include/logcommon.h index 34568e1..2105222 100644 --- a/include/logcommon.h +++ b/include/logcommon.h @@ -72,7 +72,8 @@ extern "C" { #endif typedef enum { - BACKEND_NONE, + BACKEND_NONE, // unset or invalid + BACKEND_NULL, // "null", well-defined BACKEND_PIPE, BACKEND_ANDROID_LOGGER, BACKEND_ZERO_COPY diff --git a/packaging/dlog.spec b/packaging/dlog.spec index 1591899..af4c1d0 100644 --- a/packaging/dlog.spec +++ b/packaging/dlog.spec @@ -243,6 +243,8 @@ chsmack -e 'System' %{_libexecdir}/dlog-log-critical %{_sysconfdir}/dlog.conf %{_sysconfdir}/dlog.conf.d/20-pipe.conf %{_sysconfdir}/dlog.conf.d/25-logger.conf +# not in a separate package for debugging convenience +%{_sysconfdir}/dlog.conf.d/86-disable-dlog.conf_inactive %attr(755,log,log) /var/log/dlog %attr(2551,log,log) %{_libexecdir}/dlog-log-critical /usr/lib/tmpfiles.d/dlog-run.conf diff --git a/src/libdlog/log.c b/src/libdlog/log.c index 035c63b..dbb7ecc 100644 --- a/src/libdlog/log.c +++ b/src/libdlog/log.c @@ -47,6 +47,18 @@ #define DEFAULT_CONFIG_DEBUGMODE 0 #define DEFAULT_CONFIG_LIMITER_APPLY_TO_ALL_BUFFERS 0 + +/* A pseudo-backend that does nothing. Useful for removing the overhead of dlog + * for debugging and measurement purposes, also it simplifies some checking as + * the `write_to_log` pointer below never needs to be NULL. Note that features + * independent of the backend (such as limiter or dynamic config) are controlled + * separately if dlog is to be disabled completely and that minimal overhead is + * always present (e.g. building the message via vprintf). */ +static int write_to_log_null (log_id_t log_id, log_priority prio, const char *tag, const char *msg, struct timespec *tp_mono, int32_t pid, int32_t tid) +{ + return DLOG_ERROR_NONE; +} + /** * @brief Points to a function which writes a log message * @details The function pointed to depends on the backend used @@ -57,7 +69,7 @@ * @return Returns the number of bytes written on success and a negative error value on error. * @see __dlog_init_backend */ -int (*write_to_log)(log_id_t log_id, log_priority prio, const char *tag, const char *msg, struct timespec *tp_mono, int32_t pid, int32_t tid) = NULL; +int (*write_to_log)(log_id_t log_id, log_priority prio, const char *tag, const char *msg, struct timespec *tp_mono, int32_t pid, int32_t tid) = write_to_log_null; void (*destroy_backend)(void); int (*stash_failed_log)(log_id_t log_id, log_priority prio, const char *tag, const char *msg) = NULL; @@ -148,6 +160,8 @@ static int __configure_backend(struct log_config *config) __dlog_init_android(config); else if (!strcmp(backend, "zero-copy")) __dlog_init_zero_copy(config); + else if (!strcmp(backend, "null")) + ; // already the default else return 0; @@ -492,12 +506,7 @@ static int __write_to_log(log_id_t log_id, int prio, const char *tag, const char if (should_disable_cancels) pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &old_cancel_state); - /* The only thing that needs to be protected here is `write_to_log` since - * all other resources already have their own specific locks (and even the - * pointer could be made to point at a null handler instead of a true NULL) - * but giving this guarantee makes everything a lot simpler as it removes - * the risk of something suddenly becoming NULL during processing. */ - if (!initialize() || !write_to_log) + if (!initialize()) // TODO: We could consider stashing the failed log here ret = DLOG_ERROR_NOT_PERMITTED; else if (secure_log && !enable_secure_logs) @@ -795,7 +804,7 @@ void __dlog_fini(void) destroy_backend(); destroy_backend = NULL; } - write_to_log = NULL; + write_to_log = write_to_log_null; stash_failed_log = NULL; is_initialized = false; first = true; diff --git a/src/libdlogutil/lib.c b/src/libdlogutil/lib.c index d1c60e2..8c263c2 100644 --- a/src/libdlogutil/lib.c +++ b/src/libdlogutil/lib.c @@ -143,6 +143,8 @@ static backend_t check_backend(struct log_config *conf) return BACKEND_ZERO_COPY; if (!strcmp(backend, "logger")) return BACKEND_ANDROID_LOGGER; + if (!strcmp(backend, "null")) + return BACKEND_NULL; // valid, unlike BACKEND_NONE return BACKEND_NONE; } @@ -175,6 +177,8 @@ EXPORT_API int dlogutil_config_connect(dlogutil_config_s *config, dlogutil_state backend_t const backend = check_backend(&conf); if (backend == BACKEND_NONE) return TIZEN_ERROR_IO_ERROR; + if (backend == BACKEND_NULL) + return TIZEN_ERROR_NO_DATA; if (backend == BACKEND_ZERO_COPY) { /* FIXME! There are no buffers under @@ -346,6 +350,8 @@ EXPORT_API int dlogutil_buffer_get_default_ts_type(log_id_t buffer, dlogutil_sor backend_t const backend = check_backend(&conf); if (backend == BACKEND_NONE) return TIZEN_ERROR_INVALID_PARAMETER; + if (backend == BACKEND_NULL) + return TIZEN_ERROR_NO_DATA; *type = get_proper_sort_by(DLOGUTIL_SORT_DEFAULT, 1 << buffer, backend, &conf, &(bool){ false }); return TIZEN_ERROR_NONE; @@ -382,6 +388,9 @@ EXPORT_API int dlogutil_buffer_check_ts_type_available(log_id_t buffer, dlogutil case BACKEND_ANDROID_LOGGER: *available = (type == DLOGUTIL_SORT_SENT_REAL); break; + case BACKEND_NULL: + *available = false; + break; case BACKEND_NONE: default: return TIZEN_ERROR_INVALID_PARAMETER; diff --git a/src/libdlogutil/logretrieve.c b/src/libdlogutil/logretrieve.c index 7ef5162..34b3678 100644 --- a/src/libdlogutil/logretrieve.c +++ b/src/libdlogutil/logretrieve.c @@ -50,6 +50,8 @@ int create_initial_fdis(struct fd_info ***fdis, int enabled_buffers, backend_t b { assert(fdis); assert(conf); + assert(backend != BACKEND_NONE); + assert(backend != BACKEND_NULL); __attribute__((cleanup(fdi_array_free))) struct fd_info **fdi_ptrs = NULL; unsigned int mask; diff --git a/src/log-redirect-stdout/internal.c b/src/log-redirect-stdout/internal.c index e537870..94e3c73 100644 --- a/src/log-redirect-stdout/internal.c +++ b/src/log-redirect-stdout/internal.c @@ -121,6 +121,15 @@ static int setup_single_zero_copy(log_id_t buffer, const char *tag, log_priority return 0; } +static int setup_single_null(log_id_t buffer, const char *tag, log_priority prio, struct log_config *config, int *fd) +{ + *fd = open("/dev/null", O_WRONLY); + if (*fd < 0) + return -errno; + + return 0; +} + int setup_single_unstructed(log_id_t buffer, const char *tag, log_priority prio, int *fd) { __attribute__((cleanup(log_config_free))) struct log_config config; @@ -148,6 +157,8 @@ int setup_single_unstructed(log_id_t buffer, const char *tag, log_priority prio, return setup_single_pipe(buffer, tag, prio, &config, fd); else if (!strcmp(backend, "zero-copy")) return setup_single_zero_copy(buffer, tag, prio, &config, fd); + else if (!strcmp(backend, "null")) + return setup_single_null(buffer, tag, prio, &config, fd); else return -ENOTSUP; } diff --git a/src/log-redirect-stdout/lib.c b/src/log-redirect-stdout/lib.c index 5dca219..e32f7b5 100644 --- a/src/log-redirect-stdout/lib.c +++ b/src/log-redirect-stdout/lib.c @@ -79,6 +79,20 @@ EXPORT_API _Bool dlog_is_log_fd(int fd) && strncmp(link_path, PIPE_FIFO_PATH_TEMPLATE_BASE, sizeof PIPE_FIFO_PATH_TEMPLATE_BASE - 1)) return false; + /* The null backend opens connections to /dev/null, which + * also happens often for non-dlog FDs. We don't really have + * a way to check, so assume that these are non-dlog since + * this maintains the behaviour from before this was added. + * + * The main use case for this API is to avoid closing log + * FDs, but for /dev/null this makes little difference as + * logs are dropped either way. + * + * Note that this check is redundant since it is covered by + * the block above, so is mostly there for completeness. */ + if (!strncmp(link_path, "/dev/null", sizeof "/dev/null" - 1)) + return false; + /* In theory somebody could manually open a connection * to /dev/log_* which technically wouldn't be handled * by dlog, but then it's still a logging FD in a way, diff --git a/src/logger/logger.c b/src/logger/logger.c index 8c56cef..1d80b6c 100644 --- a/src/logger/logger.c +++ b/src/logger/logger.c @@ -1075,6 +1075,11 @@ int prepare_config_data(struct logger_config_data *data) goto end; } + if (!strcmp(backend, "null")) { + ret = -ENODATA; + goto end; + } + for (int i = 0; i < LOG_ID_MAX; i++) { char key[MAX_CONF_KEY_LEN]; const int r = snprintf(key, sizeof key, "logger_dev_throttling_%s", log_name_by_id((log_id_t)i)); diff --git a/src/logger/logger_internal.h b/src/logger/logger_internal.h index bbb133c..31fdc8f 100644 --- a/src/logger/logger_internal.h +++ b/src/logger/logger_internal.h @@ -133,7 +133,8 @@ extern struct backend_data { * │ client │ │ /dev/log_x │ ║ │ reader │ ║ │ │ * └─────────┘ └─────────────────┘ ║ └────────┘ ║ └─────────────────┘ * ╚═════════════╝ - */ + * + * Of course, the null backend has no relevant data structures. */ struct logger { diff --git a/src/shared/logconfig.c b/src/shared/logconfig.c index 17ac3ec..7743311 100644 --- a/src/shared/logconfig.c +++ b/src/shared/logconfig.c @@ -102,6 +102,9 @@ const char *log_config_claim_backend(struct log_config *config) * The zero-copy backend is never autodetected because it * makes large trade-offs unsuitable for the general public. * + * The null backend would have been the most elegant to pick, + * and also the most useless. Mentioning here for completeness. + * * Pipe has basically no requirements so it is always used as the * backup if nothing better is available. */ const char *const path = log_config_get((const struct log_config *)config, "main"); @@ -470,6 +473,9 @@ dlogutil_sorting_order_e get_proper_sort_by(dlogutil_sorting_order_e sort_by, dl if (backend == BACKEND_ZERO_COPY) return DLOGUTIL_SORT_SENT_MONO; // ditto + if (backend == BACKEND_NULL) // return something arbitrary to avoid digging through the config + return DLOGUTIL_SORT_SENT_REAL; + dlogutil_sorting_order_e pipe_daemon_sorting = get_order_from_config(conf); if (sort_by == DLOGUTIL_SORT_DEFAULT) return pipe_daemon_sorting; // pick the compatible sorting to make things easier diff --git a/src/tests/libdlog_base_pos.c b/src/tests/libdlog_base_pos.c index 7ac140a..af81cfc 100644 --- a/src/tests/libdlog_base_pos.c +++ b/src/tests/libdlog_base_pos.c @@ -27,6 +27,13 @@ int main(void) assert(!limiter_created); __dlog_fini(); + log_config_set(&CONFIG, "backend", "null"); + __configure(); + assert(dlog_print(DLOG_ERROR, "tag", "msg") == DLOG_ERROR_NONE); + assert(write_to_log != wtl_pipe); + assert(write_to_log != wtl_android); + __dlog_fini(); + log_config_set(&CONFIG, "backend", "logger"); log_config_set(&CONFIG, "limiter", "1"); log_config_set(&CONFIG, "debugmode", "0"); -- 2.7.4