Skip to content

Commit

Permalink
sys/log, fs/fcb: Address some review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@vrahane
vrahane committed Feb 15, 2025
1 parent 142177c commit 3bcb976
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 64 deletions.
6 changes: 3 additions & 3 deletions fs/fcb/include/fcb/fcb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,16 +172,16 @@ int fcb_getnext(struct fcb *, struct fcb_entry *loc);
* Get first entry in the provided flash area
*
* @param fcb Pointer to FCB
* @param fa Optional pointer to flash area
* @param fap Optional pointer to flash area
* @param loc Pointer to first FCB entry in the provided flash area
*
* @return 0 on success, non-zero on failure
*/
int fcb_getnext_in_area(struct fcb *fcb, struct flash_area *fa,
int fcb_getnext_in_area(struct fcb *fcb, struct flash_area *fap,
struct fcb_entry *loc);

/**
* Get next area pointer from the FCB pointer to by fcb pointer
* Get next area pointer from the FCB pointer
*
* @param fcb Pointer to the FCB
* @param fap Pointer to the flash_area
Expand Down
6 changes: 3 additions & 3 deletions fs/fcb/src/fcb_getnext.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,14 @@ fcb_start_offset(struct fcb *fcb)
}

int
fcb_getnext_in_area(struct fcb *fcb, struct flash_area *fa,
fcb_getnext_in_area(struct fcb *fcb, struct flash_area *fap,
struct fcb_entry *loc)
{
int rc;

/* If a flash area is specified, find first entry in that area */
if (fa) {
loc->fe_area = fa;
if (fap) {
loc->fe_area = fap;
loc->fe_elem_off = fcb_len_in_flash(fcb, sizeof(struct fcb_disk_area));
loc->fe_elem_ix = 0;
loc->fe_data_len = 0;
Expand Down
10 changes: 3 additions & 7 deletions sys/log/full/include/log/log_fcb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,31 +39,27 @@ struct log_fcb_bmark {
#endif
/* The index of the log entry that the FCB entry contains. */
uint32_t lfb_index;
/* Is this a sector boundary bookmark */
bool lfb_sect_bmark;
};

/** A set of fcb log bookmarks. */
struct log_fcb_bset {
/** Array of bookmarks. */
struct log_fcb_bmark *lfs_bmarks;

/** Enable sector bookmarks */
bool lfs_en_sect_bmarks;

/** The maximum number of bookmarks. */
int lfs_cap;

/** The number of currently used non-sector bookmarks. */
int lfs_non_s_size;
int lfs_non_sect_size;

/** The number of currently usable bookmarks. */
int lfs_size;

/** The index where the next bookmark will get written. */
uint32_t lfs_next;

/** The index where the next non-sector bmark will get written */
uint32_t lfs_next_non_s;
uint32_t lfs_next_non_sect;
};

/**
Expand Down
2 changes: 2 additions & 0 deletions sys/log/full/src/log_fcb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ fcb_walk_back_find_start(struct fcb *fcb, struct log *log,
* If bmark is found, fill up min_diff.
*
* The "index" field corresponds to a log entry index.
* For min_diff, if bookmark was found, fill it up with the difference
* else it will return -1.
*
* If bookmarks are enabled, this function uses them in the search.
*
Expand Down
4 changes: 3 additions & 1 deletion sys/log/full/src/log_fcb2.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ static int log_fcb2_rtr_erase(struct log *log);
* The "index" field corresponds to a log entry index.
*
* If bookmarks are enabled, this function uses them in the search.
* For min_diff, if bookmark was found, fill it up with the difference
* else it will return -1.
*
* @return 0 if an entry was found
* SYS_ENOENT if there are no suitable entries.
Expand Down Expand Up @@ -185,7 +187,7 @@ log_fcb2_start_append(struct log *log, int len, struct fcb2_entry *loc)
* bookmarks
*/
log_fcb_init_bmarks(fcb_log, fcb_log->fl_bset.lfs_bmarks,
fcb_log->fl_bset.lfs_cap);
fcb_log->fl_bset.lfs_cap, true);
#endif

#if MYNEWT_VAL(LOG_STORAGE_WATERMARK)
Expand Down
92 changes: 42 additions & 50 deletions sys/log/full/src/log_fcb_bmark.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@ 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_s = 0;
fcb_log->fl_bset.lfs_non_s_size = 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);
Expand Down Expand Up @@ -172,9 +172,9 @@ log_fcb_closest_bmark(struct fcb_log *fcb_log, uint32_t index,
if (diff < *min_diff) {
*min_diff = diff;
closest = bmark;
MODLOG_INFO(LOG_MODULE_DEFAULT, "index: %u, closest bmark idx: %u, \n",
(unsigned int)index,
(unsigned int)bmark->lfb_index);
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
*/
Expand All @@ -191,57 +191,49 @@ log_fcb_closest_bmark(struct fcb_log *fcb_log, uint32_t index,
#if MYNEWT_VAL(LOG_FCB_SECTOR_BOOKMARKS)
#if MYNEWT_VAL(LOG_FCB)
static void
log_fcb_insert_bmark(struct fcb_log *fcb_log, struct fcb_entry *entry,
uint32_t index, bool sect_bmark, int pos)
log_fcb_insert_sect_bmark(struct fcb_log *fcb_log, struct fcb_entry *entry,
uint32_t index, int pos)
#elif MYNEWT_VAL(LOG_FCB2)
static void
log_fcb_insert_bmark(struct fcb_log *fcb_log, fcb2_entry *entry,
uint32_t index, bool sect_bmark, int pos)
log_fcb_insert_sect_bmark(struct fcb_log *fcb_log, fcb2_entry *entry,
uint32_t index, int pos)
#endif
{
struct log_fcb_bset *bset;

bset = &fcb_log->fl_bset;

if (pos == bset->lfs_cap - 1) {
goto add;
if (pos != bset->lfs_cap - 1) {
memmove(&bset->lfs_bmarks[pos + 1], &bset->lfs_bmarks[pos],
sizeof(bset->lfs_bmarks[0]) * (bset->lfs_size - pos - 1));
}

memcpy(&bset->lfs_bmarks[pos + 1], &bset->lfs_bmarks[pos],
sizeof(bset->lfs_bmarks[0]) * bset->lfs_size - pos - 1);

add:
bset->lfs_bmarks[pos] = (struct log_fcb_bmark) {
.lfb_entry = *entry,
.lfb_index = index,
.lfb_sect_bmark = sect_bmark
};

if (bset->lfs_size < bset->lfs_cap) {
bset->lfs_size++;
}

bset->lfs_next++;
bset->lfs_next %= bset->lfs_cap;
}
#endif

#if MYNEWT_VAL(LOG_FCB)
static void
log_fcb_replace_bmark(struct fcb_log *fcb_log, struct fcb_entry *entry,
uint32_t index, bool sect_bmark, int pos)
log_fcb_replace_non_sect_bmark(struct fcb_log *fcb_log, struct fcb_entry *entry,
uint32_t index, int pos)
#elif MYNEWT_VAL(LOG_FCB2)
static void
log_fcb_replace_bmark(struct fcb_log *fcb_log, struct fcb2_entry *entry,
uint32_t index, bool sect_bmark, int pos)
log_fcb_replace_non_sect_bmark(struct fcb_log *fcb_log, struct fcb2_entry *entry,
uint32_t index, int pos)
#endif
{
int i = 0;
struct log_fcb_bset *bset = &fcb_log->fl_bset;
bool en_sect_bmark = sect_bmark & bset->lfs_en_sect_bmarks;

#if MYNEWT_VAL(LOG_FCB_SECTOR_BOOKMARKS)
if (en_sect_bmark) {
if (bset->lfs_en_sect_bmarks) {
for (i = fcb_log->fl_fcb.f_sector_cnt;
i < (bset->lfs_non_s_size + fcb_log->fl_fcb.f_sector_cnt);
i++) {
Expand All @@ -252,8 +244,8 @@ log_fcb_replace_bmark(struct fcb_log *fcb_log, struct fcb2_entry *entry,
}
} else
#endif
if (!en_sect_bmark) {
for (i = 0; i < bset->lfs_non_s_size; i++) {
{
for (i = 0; i < bset->lfs_non_sect_size; i++) {
if (index == bset->lfs_bmarks[i].lfb_index) {
/* If index matches, no need to replace */
return;
Expand All @@ -264,7 +256,6 @@ log_fcb_replace_bmark(struct fcb_log *fcb_log, struct fcb2_entry *entry,
bset->lfs_bmarks[pos] = (struct log_fcb_bmark) {
.lfb_entry = *entry,
.lfb_index = index,
.lfb_sect_bmark = sect_bmark
};
}

Expand Down Expand Up @@ -300,7 +291,7 @@ log_fcb_add_bmark(struct fcb_log *fcb_log, struct fcb2_entry *entry,
pos = i + 1;
}
}
log_fcb_insert_bmark(fcb_log, entry, index, sect_bmark, pos);
log_fcb_insert_sect_bmark(fcb_log, entry, index, pos);
if (pos >= fcb_log->fl_fcb.f_sector_cnt - 1) {
/* Make sure inserts do not trickle into the non-sector
* bookmarks
Expand All @@ -313,48 +304,49 @@ log_fcb_add_bmark(struct fcb_log *fcb_log, struct fcb2_entry *entry,
}
} else {
/* Replace oldest non-sector bmark */
log_fcb_replace_bmark(fcb_log, entry, index, sect_bmark,
bset->lfs_next_non_s +
bset->lfs_en_sect_bmarks ? fcb_log->fl_fcb.f_sector_cnt
: 0);
log_fcb_replace_non_sect_bmark(fcb_log, entry, index,
bset->lfs_next_non_sect +
(bset->lfs_en_sect_bmarks ?
fcb_log->fl_fcb.f_sector_cnt : 0));
MODLOG_DEBUG(LOG_MODULE_DEFAULT, "replace bmark index: %u, pos: %u, \n",
(unsigned int)index,
(unsigned int)bset->lfs_next_non_s + bset->lfs_en_sect_bmarks ?
fcb_log->fl_fcb.f_sector_cnt : 0);
if (bset->lfs_non_s_size >= MYNEWT_VAL(LOG_FCB_NUM_ABS_BMARKS)) {
bset->lfs_next_non_s = (bset->lfs_next_non_s + 1) %
MYNEWT_VAL(LOG_FCB_NUM_ABS_BMARKS);
(unsigned int)bset->lfs_next_non_sect +
(bset->lfs_en_sect_bmarks ?
fcb_log->fl_fcb.f_sector_cnt : 0));
if (bset->lfs_non_sect_size >= MYNEWT_VAL(LOG_FCB_NUM_ABS_BMARKS)) {
bset->lfs_next_non_sect = (bset->lfs_next_non_sect + 1) %
MYNEWT_VAL(LOG_FCB_NUM_ABS_BMARKS);
} else {
if (!bset->lfs_non_s_size) {
if (!bset->lfs_non_sect_size) {
/* First non-sector bmark position */
bset->lfs_next_non_s = pos;
bset->lfs_next_non_sect = pos;
}
bset->lfs_non_s_size++;
bset->lfs_non_sect_size++;
bset->lfs_size++;
}

assert(bset->lfs_non_s_size <= MYNEWT_VAL(LOG_FCB_NUM_ABS_BMARKS));
assert(bset->lfs_non_sect_size <= MYNEWT_VAL(LOG_FCB_NUM_ABS_BMARKS));
}
#else
if (!sect_bmark) {
if (bset->lfs_size >= MYNEWT_VAL(LOG_FCB_NUM_ABS_BMARKS)) {
/* Replace oldest non-sector bmark */
log_fcb_replace_bmark(fcb_log, entry, index, sect_bmark,
bset->lfs_next_non_s);
log_fcb_replace_non_sect_bmark(fcb_log, entry, index,
bset->lfs_next_non_sect);
MODLOG_DEBUG(LOG_MODULE_DEFAULT, "replace bmark index: %u, pos: %u, \n",
(unsigned int)index,
(unsigned int)bset->lfs_next_non_s);
bset->lfs_next_non_s = (bset->lfs_next_non_s + 1) %
MYNEWT_VAL(LOG_FCB_NUM_ABS_BMARKS);
(unsigned int)bset->lfs_next_non_sect);
bset->lfs_next_non_sect = (bset->lfs_next_non_sect + 1) %
MYNEWT_VAL(LOG_FCB_NUM_ABS_BMARKS);
} else {
log_fcb_replace_bmark(fcb_log, entry, index, sect_bmark,
bset->lfs_size);
log_fcb_replace_non_sect_bmark(fcb_log, entry, index,
bset->lfs_size);
MODLOG_DEBUG(LOG_MODULE_DEFAULT, "replace bmark index: %u, pos: %u, \n",
(unsigned int)index,
bset->lfs_size);
if (!bset->lfs_size) {
/* First non-sector bmark position */
bset->lfs_next_non_s = pos;
bset->lfs_next_non_sect = pos;
}
bset->lfs_size++;
}
Expand Down

0 comments on commit 3bcb976

Please sign in to comment.