From d337653922663db31ba4c573a6a2c771a492c7d4 Mon Sep 17 00:00:00 2001 From: Michal Bloch Date: Thu, 18 Jan 2024 14:42:48 +0100 Subject: [PATCH] zlogger: don't let logs slip through cracks in time dlogutil receives all logs and has to figure out which of those it has already printed. It used to do this by looking at the timestamp, but if the clock_gettime() call and the memcpy() call in the client happened to be across the edge of two iterations then util will think that log was already printed in the previous one, losing it. Change this so that it only looks at previous offset. Change-Id: I13168087ef460e74c203ab0f0311b91f6f2421eb --- src/libdlogutil/fdi_zero_copy.c | 44 +++++++++++++++++++++++++++-------------- src/libdlogutil/fdi_zero_copy.h | 1 + 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/src/libdlogutil/fdi_zero_copy.c b/src/libdlogutil/fdi_zero_copy.c index b8f7dc5..4e45e23 100644 --- a/src/libdlogutil/fdi_zero_copy.c +++ b/src/libdlogutil/fdi_zero_copy.c @@ -72,6 +72,10 @@ static int zero_copy_clear(struct fd_info *fdi) static int zero_copy_create(struct fd_info *fdi, const struct log_config *conf, log_id_t id, list_head *used_paths, log_id_t aliased[static LOG_ID_MAX]) { + __attribute__((cleanup(free_ptr))) uint16_t *last_block_offsets = calloc(ZLOGGER_BLOCK_COUNT, sizeof *last_block_offsets); + if (!last_block_offsets) + return -ENOMEM; + // We can't clear, so the flag is R not RW fdi->fd = open(ZERO_COPY_DUMP_DEVICE_NAME, O_RDONLY | O_CLOEXEC); if (fdi->fd < 0) @@ -91,6 +95,8 @@ static int zero_copy_create(struct fd_info *fdi, const struct log_config *conf, zpd->dump = false; zpd->monitor = false; zpd->eof = false; + zpd->last_block_offsets = last_block_offsets; + last_block_offsets = NULL; fdi->priv_data = zpd; @@ -120,6 +126,7 @@ static void zero_copy_destroy(struct fd_info *fdi) close(fdi->fd); free_entry_list(&zpd->items); + free(zpd->last_block_offsets); free(zpd); } @@ -219,7 +226,7 @@ static void append_dlogutil_entry_to_item_list(dlogutil_entry_s *de, struct zlog *items = item; } -static void load_single_device(volatile struct zlogger_block *zb, struct zlog_entry_list **items, uint64_t min_timestamp, uint64_t max_timestamp, char *bitmap) +static void load_single_device(volatile struct zlogger_block *zb, struct zlog_entry_list **items, char *bitmap, uint16_t *last_offsets) { /* TODO: document this mumbo jumbo. * Requires expert earth magic to understand, probably. */ @@ -227,15 +234,19 @@ static void load_single_device(volatile struct zlogger_block *zb, struct zlog_en static size_t const BLOCK_COUNT = ZLOGGER_MAP_SIZE / sizeof *zb; for (size_t i = 0; i < BLOCK_COUNT; i++, zb++) { bool enabled = bitmap[i / CHAR_BIT] & (1 << (i % CHAR_BIT)); - if (!enabled) + if (!enabled) { + last_offsets[i] = 0; continue; + } - if (zb->head.offset > ZLOGGER_DATA_MAX) + const uint16_t current_offset = zb->head.offset; + if (current_offset > ZLOGGER_DATA_MAX) continue; - volatile struct zlogger_entry *ze = (struct zlogger_entry *) zb->data; - while ((uintptr_t) ze < (uintptr_t) zb->data + zb->head.offset) { + volatile struct zlogger_entry *ze = (struct zlogger_entry *) (zb->data + last_offsets[i]); + + while ((uintptr_t) ze < (uintptr_t) zb->data + current_offset) { /* Note, not `volatile` - we need this to stay consistent * across computations. Perhaps we should copy the whole @@ -253,19 +264,18 @@ static void load_single_device(volatile struct zlogger_block *zb, struct zlog_en if (CRC != ze->CRC) break; - if (ze->time >= min_timestamp && ze->time < max_timestamp) { - dlogutil_entry_s *const de = alloc_dlogutil_entry_from_zlog_entry(ze, block_len); - if (de) { - de->pid = zb->head.pid; - de->tid = zb->head.tid; - append_dlogutil_entry_to_item_list(de, items); - } else { - // TODO: consider -ENOMEM or something - } + dlogutil_entry_s *const de = alloc_dlogutil_entry_from_zlog_entry(ze, block_len); + if (de) { + de->pid = zb->head.pid; + de->tid = zb->head.tid; + append_dlogutil_entry_to_item_list(de, items); + } else { + // TODO: consider -ENOMEM or something } ze = (struct zlogger_entry *) ((uintptr_t) ze + sizeof *ze + block_len); } + last_offsets[i] = current_offset; } } @@ -349,6 +359,7 @@ static int zero_copy_read(struct fd_info *fdi) * and not only all logs since opening. We still call the ioctl so that * the next ioctl will know that we have read the existing logs. */ memset(bitmap, ~(char)0, ZLOGGER_BITMAP_SIZE); + memset(zpd->last_block_offsets, 0, ZLOGGER_BLOCK_COUNT * sizeof *zpd->last_block_offsets); zpd->read_everything = false; } @@ -356,7 +367,10 @@ static int zero_copy_read(struct fd_info *fdi) volatile void *const addr = mmap(NULL, ZLOGGER_MAP_SIZE, PROT_READ, MAP_SHARED, fdi->fd, ZLOGGER_MAP_SIZE * i); if (addr == MAP_FAILED) break; - load_single_device(addr, &zpd->items, zpd->prev_read_ts, now, bitmap + (ZLOGGER_BLOCK_MAP_COUNT / CHAR_BIT) * i); + load_single_device(addr, &zpd->items + , bitmap + (ZLOGGER_BLOCK_MAP_COUNT / CHAR_BIT) * i + , zpd->last_block_offsets + ZLOGGER_BLOCK_MAP_COUNT * i + ); munmap((void *) addr, ZLOGGER_MAP_SIZE); } diff --git a/src/libdlogutil/fdi_zero_copy.h b/src/libdlogutil/fdi_zero_copy.h index 275ff22..e5a9a5d 100644 --- a/src/libdlogutil/fdi_zero_copy.h +++ b/src/libdlogutil/fdi_zero_copy.h @@ -41,5 +41,6 @@ struct zero_copy_priv_data { bool monitor; bool eof; bool read_everything; + uint16_t *last_block_offsets; }; -- 2.7.4