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.

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 unexpected behaviour #7426

Closed
4 tasks done
dplasa opened this issue Jul 5, 2020 · 10 comments · Fixed by #7434
Closed
4 tasks done

LittleFS unexpected behaviour #7426

dplasa opened this issue Jul 5, 2020 · 10 comments · Fixed by #7434
Assignees

Comments

@dplasa
Copy link

dplasa commented Jul 5, 2020

Basic Infos

  • This issue complies with the issue POLICY doc.
  • I have read the documentation at readthedocs and the issue is not addressed there.
  • I have tested that the issue is present in current master branch (aka latest git).
  • I have searched the issue tracker for a similar issue.

Platform

  • Hardware: [ESP-12]
  • Core Version: [2.7.1]
  • Development Env: [Arduino IDE]
  • Operating System: [Ubuntu]

Settings in IDE

  • Module: [Generic ESP8266 Module]
  • FS Size [1 MB]

Problem Description

When working with big files on LittleFS, overwriting a big file might fail as LittleFS seems to release the previous file contents only after a sync (i.e. close) of that file as it uses a COW strategy (see littlefs-project/littlefs#123)

So if you have a 1MB FS and a file of 512KB you cannot overwrite that file with 512KB of data. This will fail since during that overwrite up to 1024KB need to be in that 1M FS which has less net space.

Remedy: you have to truncate the file by

  • open(file, "w")
  • close() it
  • re-open it open(file, "w")

This should be somehow documented in the docs. It would also be nice to have a call like
LittleFS.sync(); or another flag LittleFS.open(... "ws") to indicate that writing should be synced. The lower lfs_* calls seem to provide such functionality.

MCVE Sketch

#include <LittleFS.h>
#define BAUDRATE 74880


uint16_t listDir(String indent, String path)
{
  uint16_t dirCount = 0;
  Dir dir = LittleFS.openDir(path);
  while (dir.next())
  {
    ++dirCount;
    if (dir.isDirectory())
    {
      Serial.printf_P(PSTR("%s%s [Dir]\n"), indent.c_str(), dir.fileName().c_str());
      dirCount += listDir(indent + "  ", path + dir.fileName() + "/");
    }
    else
      Serial.printf_P(PSTR("%s%-16s (%ld Bytes)\n"), indent.c_str(), dir.fileName().c_str(), (uint32_t)dir.fileSize());
  }
  return dirCount;
}

void create_file(String path, uint32_t fsize)
{
  File f = LittleFS.open(path, "w");
  char buf[536]; // whatever
  for (auto i = 0; i < fsize; )
  {
    auto n = fsize - i;
    if (n > sizeof(buf))
      n = sizeof(buf);
    auto nw = f.write(buf, n);
    if (nw != n)
      Serial.printf_P(PSTR("\n\nBug at position %u: expected %d written, got %d written...\n"), i, n, nw);
    i += nw;
    if (0 == nw)
      break;
  }
  f.close();
}

void setup(void)
{
  Serial.begin(BAUDRATE);
  Serial.printf_P(PSTR("FS init: %s\n"), LittleFS.begin() ? PSTR("ok") : PSTR("fail!"));
  LittleFS.format();

  listDir("", "/");

  uint32_t maxL = 768 * 1024;

  Serial.printf_P(PSTR("Create /a.bin with size %u..."), maxL);
  uint32_t startTime = millis();
  create_file("/a.bin", maxL);
  Serial.printf_P(PSTR(" took %lu ms!\n"), millis() - startTime);

  listDir("", "/");

  maxL += 1024;
  Serial.printf_P(PSTR("Overwriting /a.bin with size %d..."), maxL);

  startTime = millis();
  create_file("/a.bin", maxL);
  Serial.printf_P(PSTR(" took %lu ms!\n"), millis() - startTime);

  listDir("", "/");
}

void loop() {}

Debug Messages

 ets Jan  8 2013,rst cause:2, boot mode:(3,6)

load 0x4010f000, len 3456, room 16 
tail 0
chksum 0x84
csum 0x84
va5432625
~ld
FS init: ok
Create /a.bin with size 786432... took 15225 ms!
a.bin            (786432 Bytes)
Overwriting /a.bin with size 787456...

Bug at position 212792: expected 536 written, got 0 written...
 took 4063 ms!
a.bin            (786432 Bytes)

@earlephilhower
Copy link
Collaborator

We had this come up before, but on a performance and not functionality level. littlefs-project/littlefs#244 .

open(file, "w"); close() it; re-open it open(file, "w")

Perhaps an even better solution is to truncate and sync immediately on open with "w" or "w+ mode. Users don't have to do anything, it just works.

@earlephilhower earlephilhower self-assigned this Jul 6, 2020
@dplasa
Copy link
Author

dplasa commented Jul 6, 2020

I think this would be the expected behavior of most users if the "w" or "w+" mode open call performs the truncation and syncing internally ;-) As you said - it would just work!

@earlephilhower
Copy link
Collaborator

@dplasa, your MCVE is not failing for me. I did have to change f = LittleFS.open(path, "w"); into File f = LittleFS.open(path, "w");; but OTW I'm running exactly what you posted.

I set FS=1MB, uploaded, and both writes take ~15000ms for me. No OOS errors, no other problems.

I'm running git head, but I don't see any commits since 2.7.1 which would affect this.

Can you please double-check your MCVE ASAP? We're really close to 2.7.2, so if you want a fix in, it's gotta be very soon.

@earlephilhower earlephilhower added the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Jul 6, 2020
@dplasa
Copy link
Author

dplasa commented Jul 7, 2020

@earlephilhower I've re-run my example (also had to add File f ...) and I see the same result:

 ets Jan  8 2013,rst cause:2, boot mode:(3,6)

load 0x4010f000, len 3664, room 16 
tail 0
chksum 0xee
csum 0xee
v7b48b9de
~ld

SDK:2.2.2-dev(38a443e)/Core:2.7.2-2-g7b48b9de=20702002/lwIP:STABLE-2_1_2_RELEASE/glue:1.2-30-g92add50/BearSSL:5c771be
FS init: ok
Create /a.bin with size 786432... took 15266 ms!
a.bin            (786432 Bytes)
Overwriting /a.bin with size 787456...

Bug at position 212792: expected 536 written, got 0 written...
 took 4144 ms!
a.bin            (786432 Bytes)

@dplasa
Copy link
Author

dplasa commented Jul 7, 2020

Upload info

esptool.py v2.8
Serial port /dev/ttyUSB0
Connecting....
Chip is ESP8266EX
Features: WiFi
Crystal is 26MHz
MAC: 2c:f4:32:49:dc:a8
Uploading stub...
Running stub...
Stub running...
Changing baud rate to 3000000
Changed.
Configuring flash size...
Auto-detected Flash size: 4MB
Compressed 304240 bytes to 222882...
Wrote 304240 bytes (222882 compressed) at 0x00000000 in 2.0 seconds (effective 1198.4 kbit/s)...
Hash of data verified.
Compressed 16384 bytes to 39...
Wrote 16384 bytes (39 compressed) at 0x003fc000 in 0.0 seconds (effective 8243.3 kbit/s)...
Hash of data verified.

Leaving...
Hard resetting via RTS pin...

I selected "4MB (1MB FS; OTA ~1019kB)"

@earlephilhower
Copy link
Collaborator

Hmmm..maybe there was something in the 2.7.1->2.7.2 change that affected it as that's my exact config (D1 Minin, FS=1MB, OTA ~1MB). I will check out 2.7.1 and re-run and then 2.7.2 and compare.

earlephilhower added a commit to earlephilhower/Arduino that referenced this issue Jul 7, 2020
Fixes esp8266#7426

LittleFS doesn't update the on-flash data structures when a file is
reopened as O_TRUNC until the file is closed.  This means the space of
the original, inaccessible file cannot be used, causing OOS errors in
cases when a large file is being overwritten.

Explicitly call the file sync operation to update the on-flash metadata
as soon as a file is opened.  For normal files it's a no-op, but for
O_TRUNC modes it will free the space, allowing full overwrite of large
files.
@earlephilhower earlephilhower removed the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Jul 7, 2020
@earlephilhower
Copy link
Collaborator

Able to repro it, now. I had other changes in my repo but a git reset --hard was able to make it happen. Single-line fix in open takes care of things.

LFS has made some interesting design choices, that's for sure. They did things this way to allow fully atomic file overwrites...

@dplasa
Copy link
Author

dplasa commented Jul 7, 2020

Thanks for the fix!

@dplasa dplasa closed this as completed Jul 7, 2020
@earlephilhower
Copy link
Collaborator

@dplasa, please verify the fix and report back. You can just add the 1 line or do a full git checkout of the PR.

Once we get confirmation it can merge, then GH will close this issue.

@earlephilhower earlephilhower reopened this Jul 7, 2020
@dplasa
Copy link
Author

dplasa commented Jul 7, 2020

@earlephilhower it works! Added the lfs_file_sync(...) line, recompiled example, fixed it!

Create /a.bin with size 786432... took 15134 ms!
a.bin            (786432 Bytes)
Overwriting /a.bin with size 787456... took 15085 ms!
a.bin            (787456 Bytes)

Looks good in my opinion.

earlephilhower added a commit that referenced this issue Jul 8, 2020
* Free space of overwritten files in LittleFS

Fixes #7426

LittleFS doesn't update the on-flash data structures when a file is
reopened as O_TRUNC until the file is closed.  This means the space of
the original, inaccessible file cannot be used, causing OOS errors in
cases when a large file is being overwritten.

Explicitly call the file sync operation to update the on-flash metadata
as soon as a file is opened.  For normal files it's a no-op, but for
O_TRUNC modes it will free the space, allowing full overwrite of large
files.

* Add host test case for change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants