kasjer commented on code in PR #3365:
URL: https://github.com/apache/mynewt-core/pull/3365#discussion_r1982336023
##########
sys/log/full/src/log_fcb_bmark.c:
##########
@@ -61,74 +195,256 @@ log_fcb_rotate_bmarks(struct fcb_log *fcb_log)
i--;
}
bset->lfs_size--;
- bset->lfs_next = bset->lfs_size;
}
}
void
log_fcb_clear_bmarks(struct fcb_log *fcb_log)
{
fcb_log->fl_bset.lfs_size = 0;
- fcb_log->fl_bset.lfs_next = 0;
+ fcb_log->fl_bset.lfs_next_non_sect = 0;
+ fcb_log->fl_bset.lfs_non_sect_size = 0;
+ memset(fcb_log->fl_bset.lfs_bmarks, 0,
+ sizeof(struct log_fcb_bmark) *
+ fcb_log->fl_bset.lfs_cap);
}
-const struct log_fcb_bmark *
-log_fcb_closest_bmark(const struct fcb_log *fcb_log, uint32_t index)
+struct log_fcb_bmark *
+log_fcb_get_bmarks(struct log *log, uint32_t *bmarks_size)
{
- const struct log_fcb_bmark *closest;
- const struct log_fcb_bmark *bmark;
- uint32_t min_diff;
+ struct fcb_log *fcb_log = (struct fcb_log *)log->l_arg;
+
+ *bmarks_size = fcb_log->fl_bset.lfs_cap;
+
+ return fcb_log->fl_bset.lfs_bmarks;
+}
+
+struct log_fcb_bmark *
+log_fcb_closest_bmark(struct fcb_log *fcb_log, uint32_t index,
+ int *min_diff)
+{
+ struct log_fcb_bmark *closest;
+ struct log_fcb_bmark *bmark;
uint32_t diff;
int i;
- min_diff = UINT32_MAX;
+ *min_diff = -1;
closest = NULL;
+ /* This works for both sector as well as non-sector bmarks
+ * because we calculate the min diff and iterate to the end
+ * of the bmarks array keeping track of min diff
+ */
for (i = 0; i < fcb_log->fl_bset.lfs_size; i++) {
bmark = &fcb_log->fl_bset.lfs_bmarks[i];
+#if MYNEWT_VAL(LOG_FCB)
+ if (!fcb_log->fl_bset.lfs_bmarks[i].lfb_entry.fe_area) {
+#if MYNEWT_VAL(LOG_FCB_SECTOR_BOOKMARKS)
+ if (i < fcb_log->fl_bset.lfs_sect_cap) {
+ /* Jump to the non-sector bookmarks since sector
+ * bookmarks are empty here on
+ */
+ i = fcb_log->fl_bset.lfs_sect_cap - 1;
+ continue;
+ }
+#else
+ /* Empty non-sector bookmark, nothing more to do
+ * Previous closest bookmark is the closest one */
+ break;
+#endif
+ }
+#elif MYNEWT_VAL(LOG_FCB2)
+ if (!fcb_log->fl_bset.lfs_bmarks[i].lfb_entry.fe_range) {
+#if MYNEWT_VAL(LOG_FCB_SECTOR_BOOKMARKS)
+ if (i < fcb_log->fl_bset.lfs_sect_cap) {
+ /* Jump to the non-sector bookmarks since sector
+ * bookmarks are empty here on
+ */
+ i = fcb_log->fl_bset.lfs_sect_cap - 1;
+ continue;
+ }
+#else
+ /* Empty non-sector bookmark, nothing more to do
+ * Previous closest bookmark is the closest one */
+ break;
+#endif
+ }
+#endif
if (bmark->lfb_index <= index) {
diff = index - bmark->lfb_index;
- if (diff < min_diff) {
- min_diff = diff;
+ if (diff < *min_diff) {
+ *min_diff = diff;
closest = bmark;
+ MODLOG_DEBUG(LOG_MODULE_DEFAULT, "index: %u, closest bmark
idx: %u, \n",
+ (unsigned int)index,
+ (unsigned int)bmark->lfb_index);
+ /* We found the exact match, no need to keep searching for a
+ * better match
+ */
+ if (*min_diff == 0) {
+ break;
+ }
}
}
}
return closest;
}
+#if MYNEWT_VAL(LOG_FCB_SECTOR_BOOKMARKS)
#if MYNEWT_VAL(LOG_FCB)
-void
-log_fcb_add_bmark(struct fcb_log *fcb_log, const struct fcb_entry *entry,
- uint32_t index)
+static int
+log_fcb_insert_sect_bmark(struct fcb_log *fcb_log, struct fcb_entry *entry,
+ uint32_t index)
#elif MYNEWT_VAL(LOG_FCB2)
-void
-log_fcb_add_bmark(struct fcb_log *fcb_log, const struct fcb2_entry *entry,
- uint32_t index)
+static int
+log_fcb_insert_sect_bmark(struct fcb_log *fcb_log, struct fcb2_entry *entry,
+ uint32_t index)
#endif
{
struct log_fcb_bset *bset;
bset = &fcb_log->fl_bset;
- if (bset->lfs_cap == 0) {
- return;
+ bset->lfs_bmarks[bset->lfs_next_sect] = (struct log_fcb_bmark) {
+ .lfb_entry = *entry,
+ .lfb_index = index,
+ };
+
+ if (bset->lfs_size < fcb_log->fl_bset.lfs_sect_cap) {
+ bset->lfs_size++;
+ bset->lfs_next_sect = (bset->lfs_next_sect - 1) %
+ fcb_log->fl_bset.lfs_sect_cap;
Review Comment:
## Nice explanation, except it's not correct.
I guess you never actually tried how it would work
Here is test code that shows how this modulo logic behaves.
Since error in code is when `lfs_next_sect` is decremented from 0, following
code prints 10 values of lfs_next_sect starting from 4 to see what sequence of
lfs_next_sect would look like.
Starting from capacity 5 as in your example
```c
uint32_t lfs_next_sect;
int lfs_sect_cap[] = {5, 7, 8, 192, 256};
for (int i = 0; i < 5; ++i) {
/* Let start from 4 and continue towards expected rollover */
lfs_next_sect = 4;
printf("Test when capacity is %d\n", lfs_sect_cap[i]);
for (int j = 0; j < 10; ++j) {
lfs_next_sect = (lfs_next_sect - 1) % lfs_sect_cap[i];
printf("lfs_next_sec = %lu\n", lfs_next_sect);
}
}
```
and here is output from this code
## Most interesting result is from capacity 5 as per your example
### Test when capacity is 5
```
lfs_next_sec = 3
lfs_next_sec = 2
lfs_next_sec = 1
lfs_next_sec = 0
lfs_next_sec = 0
lfs_next_sec = 0
lfs_next_sec = 0
lfs_next_sec = 0
lfs_next_sec = 0
lfs_next_sec = 0
```
### Test when capacity is 7
```
lfs_next_sec = 3
lfs_next_sec = 2
lfs_next_sec = 1
lfs_next_sec = 0
lfs_next_sec = 3
lfs_next_sec = 2
lfs_next_sec = 1
lfs_next_sec = 0
lfs_next_sec = 3
lfs_next_sec = 2
```
### Test when capacity is 8
```
lfs_next_sec = 3
lfs_next_sec = 2
lfs_next_sec = 1
lfs_next_sec = 0
lfs_next_sec = 7
lfs_next_sec = 6
lfs_next_sec = 5
lfs_next_sec = 4
lfs_next_sec = 3
lfs_next_sec = 2
```
### Test when capacity is 192
```
lfs_next_sec = 3
lfs_next_sec = 2
lfs_next_sec = 1
lfs_next_sec = 0
lfs_next_sec = 63
lfs_next_sec = 62
lfs_next_sec = 61
lfs_next_sec = 60
lfs_next_sec = 59
lfs_next_sec = 58
```
### Test when capacity is 256
```
lfs_next_sec = 3
lfs_next_sec = 2
lfs_next_sec = 1
lfs_next_sec = 0
lfs_next_sec = 255
lfs_next_sec = 254
lfs_next_sec = 253
lfs_next_sec = 252
lfs_next_sec = 251
lfs_next_sec = 250
```
computation is only valid when capacity is power of 2
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
