[nrf_qspi_nor] LittleFS file system fails to mount if LFS rcache buffer is not word aligned

[ball-hayden]

Describe the bug

When using LittleFS on an nRF52840 PCA10056, I find that fs_mount hangs when using the following config:

CONFIG_FILE_SYSTEM=y
CONFIG_FILE_SYSTEM_LITTLEFS=y
CONFIG_FLASH=y
CONFIG_FLASH_MAP=y
CONFIG_FLASH_PAGE_LAYOUT=y
CONFIG_NORDIC_QSPI_NOR=y
CONFIG_NORDIC_QSPI_NOR_FLASH_LAYOUT_PAGE_SIZE=4096

On investigation, nrfx_qspi_read requires that the destination buffer supplied is 32-bit word-aligned. As it stands, there is no guarantee that this is the case when using the flash API (as far as I can tell).

To Reproduce

I've attempted to reproduce this using samples/subsys/fs/littlefs, but haven't been able to coerce the compiler into not word-aligning the rcache buffer.

I am, unfortunately, hitting this bug in my own project.

Expected behavior

Calling fs_mount as documented in samples/subsys/fs/littlefs should result in the file system being mounted, regardless of decisions the compiler has made about the location of the cache.

Impact

Without an appropriate workaround, this is preventing us from reading qSPI flash storage.

Screenshots or console output

In my project, the call to fs_mount results in the following output

[00:00:00.019,134] <inf> littlefs: LittleFS version 2.1, disk version 2.0
[00:00:00.019,439] <inf> littlefs: FS at MX25R64:0x1000 is 256 0x1000-byte blocks with 512 cycle
[00:00:00.019,439] <inf> littlefs: sizes: rd 16 ; pr 16 ; ca 64 ; la 32
uart:~$

Note that the mount never completes or fails.

Environment (please complete the following information):

• OS: MacOS
• Toolchain: gnuarmemb
• Commit SHA or Version used: Nordic fork, SHA: 367eec2dd60d4e15ab6b9aff22d33686312997ed

Additional context

Initially reported as a proposed naive patch at nrfconnect/sdk-zephyr#291.

Applying the above patch does resolve the issue, which IMHO confirms this is an issue with word alignment of the buffer.

[pabigot]

I've opened #24128 to add the alignment check that was missing, but that doesn't really solve the problem. The flash_read API allows for alignment requirements on offset and size, but fails to specify whether the destination buffer must be aligned. For this driver, it apparently does, and I don't think we can change that.

The bigger question is how you got an unaligned buffer. One way I can reproduce this is to assign a misaligned buffer in the lfs_config structure provided through struct fs_littlefs::read_buffer:

diff --git a/samples/subsys/fs/littlefs/src/main.c b/samples/subsys/fs/littlefs/src/main.c
index 2d87a0d461..16c7cc497c 100644
--- a/samples/subsys/fs/littlefs/src/main.c
+++ b/samples/subsys/fs/littlefs/src/main.c
@@ -36,6 +36,12 @@ void main(void)
 
        snprintf(fname, sizeof(fname), "%s/boot_count", mp->mnt_point);
 
+       u8_t rb[65];
+       u8_t pb[65];
+
+       storage.cfg.read_buffer = rb+1;
+       storage.cfg.prog_buffer = pb+1;
+
        rc = flash_area_open(id, &pfa);
        if (rc < 0) {
                printk("FAIL: unable to find flash area %u: %d\n",

In current master that would cause a silent failure as you see; with #24128 it produces:

*** Booting Zephyr OS build zephyr-v2.2.0-1209-gba1bdca57486  ***
Area 4 at 0x0 on MX25R64 for 65536 bytes
FAIL: mount id 4 at /lfs: -22
[00:00:00.008,514] <inf> littlefs: LittleFS version 2.1, disk version 2.0
[00:00:00.008,575] <inf> littlefs: FS at MX25R64:0x0 is 16 0x1000-byte blocks wi
th 512 cycle
[00:00:00.008,575] <inf> littlefs: sizes: rd 16 ; pr 16 ; ca 64 ; la 32
[00:00:00.008,605] <wrn> littlefs: can't mount (LFS -22); formatting
[00:00:00.008,636] <err> littlefs: format failed (LFS -22)
[00:00:00.008,636] <err> fs: fs mount error (-22)

which is a better failure mode.

Are you allocating your own buffers in the mount point configuration, and if so are you making sure they're aligned? The driver does support dynamically allocating buffers from a pool, and will do so with the correct size, so you don't have to do it yourself. Alignment should be automatically satisfied in that case as long as all the sizes involved are multiples of 4.

[ball-hayden]

Are you allocating your own buffers in the mount point configuration, and if so are you making sure they're aligned?

I'm initialising the buffers as per the sample:

FS_LITTLEFS_DECLARE_DEFAULT_CONFIG(storage);
static struct fs_mount_t lfs_storage_mnt = {
	.type = FS_LITTLEFS,
	.fs_data = &storage,
	.storage_dev = (void *)DT_FLASH_AREA_STORAGE_ID,
	.mnt_point = "/lfs",
};

Looking at the macro definition, I don't think there's any requirement for the buffers to be word-aligned?

zephyr/include/fs/littlefs.h

Lines 71 to 72 in 093fec6

	static u8_t name ## _read_buffer[cache_sz]; \
	static u8_t name ## _prog_buffer[cache_sz]; \

(cache_sz here in the default case appears to be CONFIG_FS_LITTLEFS_CACHE_SIZE=64)

It does seem odd to me that the compiler wouldn't choose to word-align a 64 byte array, but I really don't understand enough to work out why that might be.

The driver does support dynamically allocating buffers from a pool, and will do so with the correct size, so you don't have to do it yourself.

That's interesting. How do I go about that?

[pabigot]

That's interesting. How do I go about that?

You'd have to initialize the structure yourself. (I misread the source and thought the default left them null.)

There's no obligation on allocating a u8_t array with any alignment, so what you see makes sense. Please try the patch in #24146.

[ball-hayden]

Ah - that makes sense.

I've applied the patch in #24146 and that works nicely - thank you.