lib: nrf_modem: add semaphore to flash trace backend and reinit fixes

Add semaphore to flash trace backend. This solves concurrency issues
when reading out traces while writing.
Fixes for reinitializing application (warm start) and continue writing
and reading trace data.

Signed-off-by: Eivind Jølsgard <[email protected]>

lib/nrf_modem_lib/trace_backends/flash/flash.c

@@ -42,12 +42,14 @@ static __noinit size_t read_offset;
 static __noinit struct fcb_entry loc;
 static __noinit struct flash_sector *sector;
 
-static size_t trace_bytes_unread;
-static size_t flash_buf_written;
-static uint8_t flash_buf[BUF_SIZE];
+static __noinit size_t trace_bytes_unread;
+static __noinit size_t flash_buf_written;
+static __noinit uint8_t flash_buf[BUF_SIZE];
 
 static bool is_initialized;
 
+static struct k_sem fcb_sem;
+
 static int trace_backend_clear(void);
 
 static size_t buffer_append(const void *data, size_t len)
@@ -87,6 +89,8 @@ static int buffer_flush_to_flash(void)
 		return -ENODATA;
 	}
 
+	k_sem_take(&fcb_sem, K_FOREVER);
+
 	err = fcb_append(&trace_fcb, flash_buf_written, &loc_flush);
 	if (err) {
 		if (IS_ENABLED(CONFIG_NRF_MODEM_TRACE_FLASH_NOSPACE_ERASE_OLDEST)) {
@@ -101,40 +105,43 @@ static int buffer_flush_to_flash(void)
 			err = fcb_walk(&trace_fcb, loc_flush.fe_sector, fcb_walk_callback, NULL);
 			if (err) {
 				LOG_ERR("fcb_walk failed, err %d", err);
-				return err;
+				goto out;
 			}
 
 			/* Erase the oldest sector and append again. */
 			err = fcb_rotate(&trace_fcb);
 			if (err) {
 				LOG_ERR("fcb_rotate failed, err %d", err);
-				return err;
+				goto out;
 			}
 			err = fcb_append(&trace_fcb, flash_buf_written, &loc_flush);
 		}
 
 		if (err) {
 			LOG_ERR("fcb_append failed, err %d", err);
-			return -ENOSPC;
+			err = -ENOSPC;
+			goto out;
 		}
 	}
 
 	err = flash_area_write(
 		trace_fcb.fap, FCB_ENTRY_FA_DATA_OFF(loc_flush), flash_buf, flash_buf_written);
 	if (err) {
 		LOG_ERR("flash_area_write failed, err %d", err);
-		return err;
+		goto out;
 	}
 
 	err = fcb_append_finish(&trace_fcb, &loc_flush);
 	if (err) {
 		LOG_ERR("fcb_append_finish failed, err %d", err);
-		return err;
+		goto out;
 	}
 
 	flash_buf_written = 0;
 
-	return 0;
+out:
+	k_sem_give(&fcb_sem);
+	return err;
 }
 
 static int trace_flash_erase(void)
@@ -160,6 +167,8 @@ int trace_backend_init(trace_backend_processed_cb trace_processed_cb)
 		return -EFAULT;
 	}
 
+	k_sem_init(&fcb_sem, 0, 1);
+
 	trace_processed_callback = trace_processed_cb;
 
 	err = flash_area_open(FIXED_PARTITION_ID(MODEM_TRACE), &modem_trace_area);
@@ -184,6 +193,8 @@ int trace_backend_init(trace_backend_processed_cb trace_processed_cb)
 	if (magic != TRACE_MAGIC_INITIALIZED) {
 		LOG_DBG("Initializing");
 		read_offset = 0;
+		trace_bytes_unread = 0;
+		flash_buf_written = 0;
 		memset(&loc, 0, sizeof(loc));
 		sector = NULL;
 		magic = TRACE_MAGIC_INITIALIZED;
@@ -212,23 +223,15 @@ int trace_backend_init(trace_backend_processed_cb trace_processed_cb)
 	err = fcb_init(FIXED_PARTITION_ID(MODEM_TRACE), &trace_fcb);
 	if (err) {
 		LOG_ERR("fcb_init error: %d", err);
+		k_sem_give(&fcb_sem);
 		return err;
 	}
 
-	/* Get trace size */
-	err = fcb_getnext(&trace_fcb, &loc);
-	while (!err) {
-		trace_bytes_unread += loc.fe_data_len;
-		err = fcb_getnext(&trace_fcb, &loc);
-	}
-
-	loc.fe_sector = 0;
-	loc.fe_elem_off = 0;
-
 	is_initialized = true;
 
 	LOG_DBG("Modem trace flash storage initialized\n");
 
+	k_sem_give(&fcb_sem);
 	return 0;
 }
 
@@ -237,6 +240,9 @@ size_t trace_backend_data_size(void)
 	return trace_bytes_unread;
 }
 
+/* Read from offset
+ * FCB sem has to be taken before calling this function!
+ */
 static int read_from_offset(void *buf, size_t len)
 {
 	int err;
@@ -257,23 +263,13 @@ static int read_from_offset(void *buf, size_t len)
 		read_offset = 0;
 	}
 
-	/* Erase if done with previous sector. */
-	if (sector && (sector != loc.fe_sector)) {
-		err = fcb_rotate(&trace_fcb);
-		if (err) {
-			return to_read;
-		}
-	}
-
-	sector = loc.fe_sector;
-
 	return to_read;
 }
 
 int trace_backend_read(void *buf, size_t len)
 {
 	int err;
-	size_t to_read;
+	size_t to_read = 0;
 
 	if (!is_initialized) {
 		return -EPERM;
@@ -283,17 +279,22 @@ int trace_backend_read(void *buf, size_t len)
 		return -EINVAL;
 	}
 
-	if (read_offset != 0) {
-		return read_from_offset(buf, len);
+	k_sem_take(&fcb_sem, K_FOREVER);
+
+	if (read_offset != 0 && loc.fe_sector) {
+		err = read_from_offset(buf, len);
+		goto out;
 	}
 
 	err = fcb_getnext(&trace_fcb, &loc);
 	if (err == -ENOTSUP && !flash_buf_written) {
 		/* Nothing to read */
 		loc.fe_sector = 0;
 		loc.fe_elem_off = 0;
+		read_offset = 0;
 		sector = NULL;
-		return -ENODATA;
+		err = -ENODATA;
+		goto out;
 	} else if (err == -ENOTSUP && flash_buf_written) {
 		to_read = MIN(flash_buf_written, len);
 		memcpy(buf, flash_buf, to_read);
@@ -306,21 +307,29 @@ int trace_backend_read(void *buf, size_t len)
 		flash_buf_written -= to_read;
 		trace_bytes_unread -= to_read;
 
-		if (sector) {
-			err = fcb_rotate(&trace_fcb);
-			if (err) {
-				return to_read;
-			}
-			sector = NULL;
-		}
-
-		return to_read;
+		err = to_read;
+		goto out;
 
 	} else if (err) {
-		return err;
+		goto out;
+	}
+
+	err = read_from_offset(buf, len);
+
+out:
+	/* Erase if done with previous sector. */
+	if (sector && (sector != loc.fe_sector)) {
+		err = fcb_rotate(&trace_fcb);
+		if (err) {
+			k_sem_give(&fcb_sem);
+			return err;
+		}
 	}
 
-	return read_from_offset(buf, len);
+	sector = loc.fe_sector;
+
+	k_sem_give(&fcb_sem);
+	return err;
 }
 
 static int stream_write(const void *buf, size_t len)
@@ -376,6 +385,7 @@ int trace_backend_clear(void)
 		return -EPERM;
 	}
 
+	k_sem_take(&fcb_sem, K_FOREVER);
 	LOG_DBG("Clearing trace storage");
 	flash_buf_written = 0;
 	err = fcb_clear(&trace_fcb);
@@ -386,6 +396,8 @@ int trace_backend_clear(void)
 	read_offset = 0;
 	sector = NULL;
 
+	k_sem_give(&fcb_sem);
+
 	return err;
 }