Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

littlefs: make block size configurable at compile time #18141

Merged
merged 4 commits into from
Jul 31, 2022
Merged

littlefs: make block size configurable at compile time #18141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
41c0b1e
pkg/littlefs: make block size configurable
fabian18 May 28, 2022
a29d5c2
pkg/littlefs2: make block size configurable
fabian18 May 28, 2022
4dd5cb3
pkg/littlefs2: add warning if block size is not reasonable
fabian18 Jun 18, 2022
578e628
pkg/littlefs: add warning if block size is not reasonable
fabian18 Jun 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 27 additions & 12 deletions pkg/littlefs/fs/littlefs_fs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,29 +62,29 @@ static int littlefs_err_to_errno(ssize_t err)
}

static int _dev_read(const struct lfs_config *c, lfs_block_t block,
lfs_off_t off, void *buffer, lfs_size_t size)
lfs_off_t off, void *buffer, lfs_size_t size)
{
littlefs_desc_t *fs = c->context;
mtd_dev_t *mtd = fs->dev;

DEBUG("lfs_read: c=%p, block=%" PRIu32 ", off=%" PRIu32 ", buf=%p, size=%" PRIu32 "\n",
(void *)c, block, off, buffer, size);

return mtd_read_page(mtd, buffer, (fs->base_addr + block) * mtd->pages_per_sector,
off, size);
uint32_t page = (fs->base_addr + block) * fs->sectors_per_block * mtd->pages_per_sector;
return mtd_read_page(mtd, buffer, page, off, size);
}

static int _dev_write(const struct lfs_config *c, lfs_block_t block,
lfs_off_t off, const void *buffer, lfs_size_t size)
lfs_off_t off, const void *buffer, lfs_size_t size)
{
littlefs_desc_t *fs = c->context;
mtd_dev_t *mtd = fs->dev;

DEBUG("lfs_write: c=%p, block=%" PRIu32 ", off=%" PRIu32 ", buf=%p, size=%" PRIu32 "\n",
(void *)c, block, off, buffer, size);

return mtd_write_page_raw(mtd, buffer, (fs->base_addr + block) * mtd->pages_per_sector,
off, size);
uint32_t page = (fs->base_addr + block) * fs->sectors_per_block * mtd->pages_per_sector;
return mtd_write_page_raw(mtd, buffer, page, off, size);
}

static int _dev_erase(const struct lfs_config *c, lfs_block_t block)
Expand All @@ -94,7 +94,8 @@ static int _dev_erase(const struct lfs_config *c, lfs_block_t block)

DEBUG("lfs_erase: c=%p, block=%" PRIu32 "\n", (void *)c, block);

return mtd_erase_sector(mtd, fs->base_addr + block, 1);
uint32_t sector = (fs->base_addr + block) * fs->sectors_per_block;
return mtd_erase_sector(mtd, sector, fs->sectors_per_block);
}

static int _dev_sync(const struct lfs_config *c)
Expand All @@ -117,11 +118,24 @@ static int prepare(littlefs_desc_t *fs)

memset(&fs->fs, 0, sizeof(fs->fs));

if (!fs->config.block_count) {
fs->config.block_count = fs->dev->sector_count - fs->base_addr;
}
static_assert(0 > LITTLEFS_MIN_BLOCK_SIZE_EXP ||
6 < LITTLEFS_MIN_BLOCK_SIZE_EXP,
"LITTLEFS_MIN_BLOCK_SIZE_EXP must be at least 7, "
"to configure a reasonable block size of at least 128 bytes.");

size_t block_size = fs->dev->pages_per_sector * fs->dev->page_size;
#if LITTLEFS_MIN_BLOCK_SIZE_EXP >= 0
block_size = ((block_size - 1) + (1u << LITTLEFS_MIN_BLOCK_SIZE_EXP))
/ block_size * block_size;
#endif
fs->sectors_per_block = block_size / (fs->dev->pages_per_sector * fs->dev->page_size);
size_t block_count = fs->dev->sector_count / fs->sectors_per_block;

if (!fs->config.block_size) {
fs->config.block_size = fs->dev->page_size * fs->dev->pages_per_sector;
fs->config.block_size = block_size;
}
if (!fs->config.block_count) {
fs->config.block_count = block_count - fs->base_addr;
}
if (!fs->config.prog_size) {
fs->config.prog_size = fs->dev->page_size;
Expand Down Expand Up @@ -443,7 +457,8 @@ static int _statvfs(vfs_mount_t *mountp, const char *restrict path, struct statv
mutex_unlock(&fs->lock);

buf->f_bsize = fs->fs.cfg->block_size; /* block size */
buf->f_frsize = fs->fs.cfg->block_size; /* fundamental block size */
buf->f_frsize = fs->dev->page_size *
fs->dev->pages_per_sector; /* fundamental block size */
buf->f_blocks = fs->fs.cfg->block_count; /* Blocks total */
buf->f_bfree = buf->f_blocks - nb_blocks; /* Blocks free */
buf->f_bavail = buf->f_blocks - nb_blocks; /* Blocks available to non-privileged processes */
Expand Down
9 changes: 9 additions & 0 deletions pkg/littlefs2/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,13 @@ config LITTLEFS2_BLOCK_CYCLES
Sets the maximum number of erase cycles before blocks are evicted as a part
of wear leveling. -1 disables wear-leveling.

config LITTLEFS2_MIN_BLOCK_SIZE_EXP
int "Minimum acceptable block size"
range -1 15
default -1
help
Sets the exponent of the minimum acceptable block size in bytes (2^n).
The actual block size may be larger due to device properties.
The default value (-1) sets the block size to the smalles possible value.

endif # MODULE_LITTLEFS2_FS
40 changes: 27 additions & 13 deletions pkg/littlefs2/fs/littlefs2_fs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,29 +62,29 @@ static int littlefs_err_to_errno(ssize_t err)
}

static int _dev_read(const struct lfs_config *c, lfs_block_t block,
lfs_off_t off, void *buffer, lfs_size_t size)
lfs_off_t off, void *buffer, lfs_size_t size)
{
littlefs2_desc_t *fs = c->context;
mtd_dev_t *mtd = fs->dev;

DEBUG("lfs_read: c=%p, block=%" PRIu32 ", off=%" PRIu32 ", buf=%p, size=%" PRIu32 "\n",
(void *)c, block, off, buffer, size);

return mtd_read_page(mtd, buffer, (fs->base_addr + block) * mtd->pages_per_sector,
off, size);
uint32_t page = (fs->base_addr + block) * fs->sectors_per_block * mtd->pages_per_sector;
return mtd_read_page(mtd, buffer, page, off, size);
}

static int _dev_write(const struct lfs_config *c, lfs_block_t block,
lfs_off_t off, const void *buffer, lfs_size_t size)
lfs_off_t off, const void *buffer, lfs_size_t size)
{
littlefs2_desc_t *fs = c->context;
mtd_dev_t *mtd = fs->dev;

DEBUG("lfs_write: c=%p, block=%" PRIu32 ", off=%" PRIu32 ", buf=%p, size=%" PRIu32 "\n",
(void *)c, block, off, buffer, size);

return mtd_write_page_raw(mtd, buffer, (fs->base_addr + block) * mtd->pages_per_sector,
off, size);
uint32_t page = (fs->base_addr + block) * fs->sectors_per_block * mtd->pages_per_sector;
return mtd_write_page_raw(mtd, buffer, page, off, size);
}

static int _dev_erase(const struct lfs_config *c, lfs_block_t block)
Expand All @@ -94,8 +94,8 @@ static int _dev_erase(const struct lfs_config *c, lfs_block_t block)

DEBUG("lfs_erase: c=%p, block=%" PRIu32 "\n", (void *)c, block);

return mtd_erase_sector(mtd, fs->base_addr + block, 1);
}
uint32_t sector = (fs->base_addr + block) * fs->sectors_per_block;
return mtd_erase_sector(mtd, sector, fs->sectors_per_block);}

static int _dev_sync(const struct lfs_config *c)
{
Expand All @@ -117,11 +117,24 @@ static int prepare(littlefs2_desc_t *fs)

memset(&fs->fs, 0, sizeof(fs->fs));

if (!fs->config.block_count) {
fs->config.block_count = fs->dev->sector_count - fs->base_addr;
}
static_assert(0 > CONFIG_LITTLEFS2_MIN_BLOCK_SIZE_EXP ||
6 < CONFIG_LITTLEFS2_MIN_BLOCK_SIZE_EXP,
"CONFIG_LITTLEFS2_MIN_BLOCK_SIZE_EXP must be at least 7, "
"to configure a reasonable block size of at least 128 bytes.");

size_t block_size = fs->dev->pages_per_sector * fs->dev->page_size;
#if CONFIG_LITTLEFS2_MIN_BLOCK_SIZE_EXP >= 0
block_size = ((block_size - 1) + (1u << CONFIG_LITTLEFS2_MIN_BLOCK_SIZE_EXP))
/ block_size * block_size;
#endif
fs->sectors_per_block = block_size / (fs->dev->pages_per_sector * fs->dev->page_size);
size_t block_count = fs->dev->sector_count / fs->sectors_per_block;

if (!fs->config.block_size) {
fs->config.block_size = fs->dev->page_size * fs->dev->pages_per_sector;
fs->config.block_size = block_size;
}
if (!fs->config.block_count) {
fs->config.block_count = block_count - fs->base_addr;
}
if (!fs->config.prog_size) {
fs->config.prog_size = fs->dev->page_size;
Expand Down Expand Up @@ -449,7 +462,8 @@ static int _statvfs(vfs_mount_t *mountp, const char *restrict path, struct statv
mutex_unlock(&fs->lock);

buf->f_bsize = fs->fs.cfg->block_size; /* block size */
buf->f_frsize = fs->fs.cfg->block_size; /* fundamental block size */
buf->f_frsize = fs->dev->page_size *
fs->dev->pages_per_sector; /* fundamental block size */
buf->f_blocks = fs->fs.cfg->block_count; /* Blocks total */
buf->f_bfree = buf->f_blocks - nb_blocks; /* Blocks free */
buf->f_bavail = buf->f_blocks - nb_blocks; /* Blocks available to non-privileged processes */
Expand Down
8 changes: 8 additions & 0 deletions sys/include/fs/littlefs2_fs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,13 @@ extern "C" {
* of wear leveling. -1 disables wear-leveling. */
#define CONFIG_LITTLEFS2_BLOCK_CYCLES (512)
#endif

#ifndef CONFIG_LITTLEFS2_MIN_BLOCK_SIZE_EXP
/**
* The exponent of the minimum acceptable block size in bytes (2^n).
* The desired block size is not guaranteed to be applicable but will be respected. */
#define CONFIG_LITTLEFS2_MIN_BLOCK_SIZE_EXP (-1)
benpicco marked this conversation as resolved.
Show resolved Hide resolved
#endif
/** @} */

/**
Expand All @@ -88,6 +95,7 @@ typedef struct {
* total number of block is defined in @p config.
* if set to 0, the total number of sectors from the mtd is used */
uint32_t base_addr;
uint16_t sectors_per_block; /**< number of sectors per block */
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this a uint16_t because I chose the maximum of CONFIG_LITTLEFS2_MIN_BLOCK_SIZE_EXP to be 2^15 in Kconfig.

#if CONFIG_LITTLEFS2_FILE_BUFFER_SIZE || DOXYGEN
/** file buffer to use internally if CONFIG_LITTLEFS2_FILE_BUFFER_SIZE
* is set */
Expand Down
8 changes: 8 additions & 0 deletions sys/include/fs/littlefs_fs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ extern "C" {
* If set, it must be program size */
#define LITTLEFS_PROG_BUFFER_SIZE (0)
#endif

#ifndef LITTLEFS_MIN_BLOCK_SIZE_EXP
/**
* The exponent of the minimum acceptable block size in bytes (2^n).
* The desired block size is not guaranteed to be applicable but will be respected. */
#define LITTLEFS_MIN_BLOCK_SIZE_EXP (-1)
#endif
/** @} */

/**
Expand All @@ -72,6 +79,7 @@ typedef struct {
* total number of block is defined in @p config.
* if set to 0, the total number of sectors from the mtd is used */
uint32_t base_addr;
uint16_t sectors_per_block; /**< number of sectors per block */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This addition resulted in the subsequent buffers no longer being aligned two 4 bytes by accident, which triggered unaligned memory accesses. On Cortex M0+, such as used by the samr21-xpro, this then resulted in hard faults.

But the issue really is that alignment requirements need to be communicated to the compiler. It was just luck that this worked before.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well not so much luck since the uint32_t will always be word-aligned.

Moving sectors_per_block to end end of the struct should be the simple fix. (The buffers are already word-multiples)

Copy link
Member

@maribu maribu Aug 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt anyone placed them after a uint32_t member to align them without adding a comment to warn about breakage. To me, this looks like we got lucky here :)

(E.g. iolist_t and pkt_snip_t have a common memory layout, but that is documented to avoid anyone from breaking it. I would expect similar comments here if that was inentionally.)

#if LITTLEFS_FILE_BUFFER_SIZE || DOXYGEN
/** file buffer to use internally if LITTLEFS_FILE_BUFFER_SIZE is set */
uint8_t file_buf[LITTLEFS_FILE_BUFFER_SIZE];
Expand Down
10 changes: 8 additions & 2 deletions tests/pkg_littlefs/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,10 @@ static void tests_littlefs_statvfs(void)

int res = vfs_statvfs("/test-littlefs/", &stat1);
TEST_ASSERT_EQUAL_INT(0, res);
TEST_ASSERT_EQUAL_INT(_dev->page_size * _dev->pages_per_sector, stat1.f_bsize);
TEST_ASSERT_EQUAL_INT(_dev->page_size *
_dev->pages_per_sector *
littlefs_desc.sectors_per_block,
stat1.f_bsize);
TEST_ASSERT_EQUAL_INT(_dev->page_size * _dev->pages_per_sector, stat1.f_frsize);
TEST_ASSERT((_dev->pages_per_sector * _dev->page_size * _dev->sector_count) >=
stat1.f_blocks);
Expand All @@ -400,7 +403,10 @@ static void tests_littlefs_statvfs(void)
res = vfs_statvfs("/test-littlefs/", &stat2);
TEST_ASSERT_EQUAL_INT(0, res);

TEST_ASSERT_EQUAL_INT(_dev->page_size * _dev->pages_per_sector, stat2.f_bsize);
TEST_ASSERT_EQUAL_INT(_dev->page_size *
_dev->pages_per_sector *
littlefs_desc.sectors_per_block,
stat2.f_bsize);
TEST_ASSERT_EQUAL_INT(_dev->page_size * _dev->pages_per_sector, stat2.f_frsize);
TEST_ASSERT(stat1.f_bfree > stat2.f_bfree);
TEST_ASSERT(stat1.f_bavail > stat2.f_bavail);
Expand Down
10 changes: 8 additions & 2 deletions tests/pkg_littlefs2/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,10 @@ static void tests_littlefs_statvfs(void)

int res = vfs_statvfs("/test-littlefs/", &stat1);
TEST_ASSERT_EQUAL_INT(0, res);
TEST_ASSERT_EQUAL_INT(_dev->page_size * _dev->pages_per_sector, stat1.f_bsize);
TEST_ASSERT_EQUAL_INT(_dev->page_size *
_dev->pages_per_sector *
littlefs_desc.sectors_per_block,
stat1.f_bsize);
TEST_ASSERT_EQUAL_INT(_dev->page_size * _dev->pages_per_sector, stat1.f_frsize);
TEST_ASSERT((_dev->pages_per_sector * _dev->page_size * _dev->sector_count) >=
stat1.f_blocks);
Expand All @@ -402,7 +405,10 @@ static void tests_littlefs_statvfs(void)
res = vfs_statvfs("/test-littlefs/", &stat2);
TEST_ASSERT_EQUAL_INT(0, res);

TEST_ASSERT_EQUAL_INT(_dev->page_size * _dev->pages_per_sector, stat2.f_bsize);
TEST_ASSERT_EQUAL_INT(_dev->page_size *
_dev->pages_per_sector *
littlefs_desc.sectors_per_block,
stat2.f_bsize);
TEST_ASSERT_EQUAL_INT(_dev->page_size * _dev->pages_per_sector, stat2.f_frsize);
TEST_ASSERT(stat1.f_bfree > stat2.f_bfree);
TEST_ASSERT(stat1.f_bavail > stat2.f_bavail);
Expand Down