littlefs-project / littlefs

A little fail-safe filesystem designed for microcontrollers
BSD 3-Clause "New" or "Revised" License
4.92k stars 774 forks source link

Issues testing with Winbond W25Q: random LFS_ERR_NOENT + LFS_ERR_INVAL after poweroff #868

Closed alessandrofrancesconi closed 10 months ago

alessandrofrancesconi commented 10 months ago

I'm working with a hardware based on Arduino Zero board (SAMD21) and Winbond W25Q128JV flash memory. My application must be able to handle random power failures, so I'm trying littlefs for this purpose since that:

littlefs is designed to handle random power failures. All file operations have strong copy-on-write guarantees and if power is lost the filesystem will fall back to the last known good state.

I'm not a real expert on NOR flash-memory usage, so I'm asking you some help to validate my implementation.

I've started setting the lfs_config structure, looking at the W25Q architecture described in the official DS: https://www.winbond.com/resource-files/w25q128jv_dtr%20revc%2003272018%20plus.pdf

The W25Q128JV array is organized into 65,536 programmable pages of 256-bytes each. Up to 256 bytes can be programmed at a time. Pages can be erased in groups of 16 (4KB sector erase), groups of 128 (32KB block erase), groups of 256 (64KB block erase) or the entire chip (chip erase). The W25Q128JV has 4,096 erasable sectors and 256 erasable blocks respectively. The small 4KB sectors allow for greater flexibility in applications that require data and parameter storage

Ending up with these parameters:

lfs_cfg.read_size = 256; // Minimum size of a block read in bytes
lfs_cfg.prog_size = 256; // Minimum size of a block program in bytes
lfs_cfg.block_size = 4096; // Size of an erasable block in bytes.
lfs_cfg.block_count = 256; // Number of erasable blocks on the device.
lfs_cfg.cache_size = 256;
lfs_cfg.lookahead_size = 128;
lfs_cfg.block_cycles = 500;

Then for low-level operations, I'm using Adafruit_SPIFlash library that is compatible with Winbond W25Qxxx memories (also it uses DMA for faster operations). It provides the following set of functions:

uint32_t readBuffer(uint32_t address, uint8_t *buffer, uint32_t len);
uint32_t writeBuffer(uint32_t address, uint8_t const *buffer, uint32_t len);
bool eraseBlock(uint32_t blockNumber);
virtual bool syncDevice(void);

Full list here and here

So I've tried to use these functions for littlefs' read/prog operations:

static Adafruit_FlashTransport_SPI flashTransport(CHIP_SELECT, SPI);
static Adafruit_SPIFlash flash(&flashTransport);

int lfs_bd_read(const struct lfs_config *cfg, lfs_block_t block, lfs_off_t off, void *buffer, lfs_size_t size) {
  return flash.readBuffer(block * cfg->block_size + off, (uint8_t*)buffer, size) != size ? LFS_ERR_IO : LFS_ERR_OK;
}
int lfs_bd_write(const struct lfs_config *cfg, lfs_block_t block, lfs_off_t off, const void *buffer, lfs_size_t size) {
  return flash.writeBuffer(block * cfg->block_size + off, (const uint8_t*)buffer, size) != size ? LFS_ERR_IO : LFS_ERR_OK;
}
int lfs_bd_erase(const struct lfs_config *cfg, lfs_block_t block) {
  return flash.eraseBlock(block) ? LFS_ERR_OK : LFS_ERR_IO;
}
int lfs_bd_sync(const struct lfs_config *cfg) {
  return flash.syncDevice() ? LFS_ERR_OK : LFS_ERR_IO;
}

Here is the code I use to test: I create a TXT file and I will it with some strings. then I read it back.

  const char* path = "TEST.TXT";
  FlashFile f;
  if (lfs_file_open(&lfs, &f, path, LFS_O_RDWR | LFS_O_CREAT | LFS_O_TRUNC) != LFS_ERR_OK){
    SerialUSB.print("Test fail on open for write");
    return false;
  }

  const char* string = "0123456789012345678901234567890123456789\n";
  const int rows = 10;
  for (int i = 0; i < rows; ++i) 
  {
    if(lfs_file_write(&lfs, &f, string, strlen(string)) < 0) 
    {
      SerialUSB.print("Test fail on write");
      return ERROR_FLASH;
    }
  }

  // close and read back
  if (lfs_file_close(&lfs, &f) != LFS_ERR_OK) {
    SerialUSB.print("Test fail on close");
    return false;
  }

  if (lfs_file_open(&lfs, &f, path, LFS_O_RDONLY) != LFS_ERR_OK) {
    SerialUSB.print("Test fail on open for read");
    return false;
  }

  int buffSize = strlen(string) + 1;
  char* buff = (char*)malloc(buffSize);
  for (int i = 0; i < rows; ++i) 
  {
    memset(buff, 0, buffSize);
    if(lfs_file_read(&lfs, &f, buff, buffSize) < 0) 
    {
      SerialUSB.print("Test fail on read");
      free(buff);
      return false;
    }
  }

  free(buff);
  lfs_file_close(&lfs, &f);
  return true;

I encapsulate the test procedure in a while(true) loop in order to stress it. I currently have two issues:

The second point is what scares me as I'm expecting that littlefs should manage power offs fairly good. Anyway, first I would like to know if my core implementation is correct. Can you help me? Thanks.

alessandrofrancesconi commented 10 months ago

A little update: as you see the "writing" part writes 10 rows of 0123456789012345678901234567890123456789\n. The actual sync is executed by the close operation after the loop.

Now, if I add a lfs_file_sync inside the loop, then I cut-off the power during the test, now the resulting error at the reboot is LFS_ERR_CORRUPT.

Again I still need to check the validity of my implementation. But if it is correct, I'm wondering what the authors mean with "Power-loss resilience" of the library. Maybe I'm misunderstanding the point?

alessandrofrancesconi commented 10 months ago

I've noticed that flash.eraseBlock(block) function from SPIFlash internally uses opcode "0xD8", that is the "Block 64K Erase" operation on Winbond. So I've changed the lfs_cfg.block_size parameter into:

lfs_cfg.block_size = 1024 * 64;

That seems to be a solution of most of my issues. I will keep this open until I've finished some tests...