Skip to content

Commit

Permalink
Fixed lookahead overflow and removed unbounded lookahead pointers
Browse files Browse the repository at this point in the history
As pointed out by davidefer, the lookahead pointer modular arithmetic
does not work around integer overflow when the pointer size is not a
multiple of the block count.

To avoid overflow problems, the easy solution is to stop trying to
work around integer overflows and keep the lookahead offset inside the
block device. To make this work, the ack was modified into a resetable
counter that is decremented every block allocation.

As a plus, quite a bit of the allocation logic ended up simplified.
  • Loading branch information
geky committed Apr 11, 2018
1 parent 89a7630 commit 9637b96
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 23 deletions.
43 changes: 22 additions & 21 deletions lfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -270,8 +270,7 @@ int lfs_deorphan(lfs_t *lfs);
static int lfs_alloc_lookahead(void *p, lfs_block_t block) {
lfs_t *lfs = p;

lfs_block_t off = (((lfs_soff_t)(block - lfs->free.begin)
% (lfs_soff_t)(lfs->cfg->block_count))
lfs_block_t off = ((block - lfs->free.off)
+ lfs->cfg->block_count) % lfs->cfg->block_count;

if (off < lfs->free.size) {
Expand All @@ -283,36 +282,38 @@ static int lfs_alloc_lookahead(void *p, lfs_block_t block) {

static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) {
while (true) {
while (lfs->free.off != lfs->free.size) {
lfs_block_t off = lfs->free.off;
lfs->free.off += 1;
while (lfs->free.i != lfs->free.size) {
lfs_block_t off = lfs->free.i;
lfs->free.i += 1;
lfs->free.ack -= 1;

if (!(lfs->free.buffer[off / 32] & (1U << (off % 32)))) {
// found a free block
*block = (lfs->free.begin + off) % lfs->cfg->block_count;
*block = (lfs->free.off + off) % lfs->cfg->block_count;

// eagerly find next off so an alloc ack can
// discredit old lookahead blocks
while (lfs->free.off != lfs->free.size &&
(lfs->free.buffer[lfs->free.off / 32] &
(1U << (lfs->free.off % 32)))) {
lfs->free.off += 1;
while (lfs->free.i != lfs->free.size &&
(lfs->free.buffer[lfs->free.i / 32]
& (1U << (lfs->free.i % 32)))) {
lfs->free.i += 1;
lfs->free.ack -= 1;
}

return 0;
}
}

// check if we have looked at all blocks since last ack
if (lfs->free.off == lfs->free.ack - lfs->free.begin) {
LFS_WARN("No more free space %d", lfs->free.off + lfs->free.begin);
if (lfs->free.ack == 0) {
LFS_WARN("No more free space %d", lfs->free.i + lfs->free.off);
return LFS_ERR_NOSPC;
}

lfs->free.begin += lfs->free.size;
lfs->free.size = lfs_min(lfs->cfg->lookahead,
lfs->free.ack - lfs->free.begin);
lfs->free.off = 0;
lfs->free.off = (lfs->free.off + lfs->free.size)
% lfs->cfg->block_count;
lfs->free.size = lfs_min(lfs->cfg->lookahead, lfs->free.ack);
lfs->free.i = 0;

// find mask of free blocks from tree
memset(lfs->free.buffer, 0, lfs->cfg->lookahead/8);
Expand All @@ -324,7 +325,7 @@ static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) {
}

static void lfs_alloc_ack(lfs_t *lfs) {
lfs->free.ack = lfs->free.begin+lfs->free.off + lfs->cfg->block_count;
lfs->free.ack = lfs->cfg->block_count;
}


Expand Down Expand Up @@ -2103,9 +2104,9 @@ int lfs_format(lfs_t *lfs, const struct lfs_config *cfg) {

// create free lookahead
memset(lfs->free.buffer, 0, lfs->cfg->lookahead/8);
lfs->free.begin = 0;
lfs->free.size = lfs_min(lfs->cfg->lookahead, lfs->cfg->block_count);
lfs->free.off = 0;
lfs->free.size = lfs_min(lfs->cfg->lookahead, lfs->cfg->block_count);
lfs->free.i = 0;
lfs_alloc_ack(lfs);

// create superblock dir
Expand Down Expand Up @@ -2182,9 +2183,9 @@ int lfs_mount(lfs_t *lfs, const struct lfs_config *cfg) {
}

// setup free lookahead
lfs->free.begin = 0;
lfs->free.size = 0;
lfs->free.off = 0;
lfs->free.size = 0;
lfs->free.i = 0;
lfs_alloc_ack(lfs);

// load superblock
Expand Down
4 changes: 2 additions & 2 deletions lfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,9 @@ typedef struct lfs_superblock {
} lfs_superblock_t;

typedef struct lfs_free {
lfs_block_t begin;
lfs_block_t size;
lfs_block_t off;
lfs_block_t size;
lfs_block_t i;
lfs_block_t ack;
uint32_t *buffer;
} lfs_free_t;
Expand Down

0 comments on commit 9637b96

Please sign in to comment.