nodemcu / nodemcu-firmware

Lua based interactive firmware for ESP8266, ESP8285 and ESP32
https://nodemcu.readthedocs.io
MIT License
7.67k stars 3.13k forks source link

Supporting a reliable flash filesystem #3148

Closed pjsg closed 2 years ago

pjsg commented 4 years ago

I have done a bunch of fuzz testing against spiffs and there are some serious corruption issues there. Whether the LUA stack can generate the particular sequence of operations or not is unclear. I have also looked at https://github.com/ARMmbed/littlefs which is very similar to spiffs but does add support for directories. It too fell down when faced with the fuzzer, and I was hopeful that the required fixes would be forthcoming. However, no such luck yet.

It turns out that the behavior of flash memory is really nasty during power failures. If you are writing a single byte in a page (typically 256 bytes), and the power fails, then there is no guarantee on the value of the rest of the page (see my comment: https://github.com/ARMmbed/littlefs/issues/245#issuecomment-606050523 )

This makes reliable storage really quite difficult!

If there are other filesystem candidates then I'd be prepared to connect them to my fuzzer and see what happens...

NicolSpies commented 4 years ago

My observations over the last few years might confirm the same, before the pipe module I passed data between functions using spiffs json files. Random corruption of the saved file, rendering it unreadable, occurs with a relative low frequency. My workaround was to save two copies of the file.

nwf commented 4 years ago

I'm hardly an expert in the space, but AIUI the move to MLC and the attendant need to whiten and ECC around bitflips has put the days of "at least we can reliably zero bits, surely" behind us.

I suppose the dance to be done is something like the ZFS top-level ring-buffered "uberblocks" (don't scream; I know ZFS is huge and is not appropriate for our space; I am talking only about a particular aspect of it), which "self-certify" their integrity via a robust checksum (IIRC fletcher4). These are the only blocks that ZFS ever reads for which it does not know the expected checksum ahead of time; every other block pointer includes not just the device address but also the checksum of the data that should be there. Assuming devices drain their write caches when instructed (hah), the latest uberblock that correctly self-certifies should point exclusively and transitively at stably-written sectors.

I suppose ubifs and jffs2 are also too bulky to run on NodeMCU, but I'd be curious to know how well they fared against your fuzzer.

Glancing around, some other candidates may include

TerryE commented 4 years ago

It turns out that the behaviour of flash memory is really nasty during power failures.

SPIFFS aim is to balance functionality, code size and performance for a FS over a bare flash device. I think that @pellepl has done a good job here. The power-fail corruption issue is a known one and has been discussed on the SPIFFS issues list. Adding some form of journalling would reduce write throughput maybe 2-3× and increase its code base proportionately. If someone can solve this problem with a magic bullet then please do so, but its going to take some graft to convert this wish into a reality.

In the meantime my personal approach is to accept and mitigate this limitation:

Also note that not having a hierarchical FS on a (typical) 1-3Mb FS is hardly a major limitation, IMO. There is nothing in SPIFFS that prevents you using "/" as a namespace separator and given that file.list() has had an optional pattern match argument, e.g. file.list('^config/'), apps can easily implement the functional equivalent.

pjsg commented 4 years ago

That Yaffs looks interesting, but it does seem to use much too much memory. The px_log doesn't seem to be a general purpose FS, but just for logging fixed size records.

spiffs has served us well, but there are known problems that are not getting resolved -- e.g. https://github.com/pellepl/spiffs/issues/185 from 2017.

What is unclear to me is how many of the issues that people report in nodemcu-firmware with files are actually underlying issues in spiffs.

I'm not advocating any change at the moment....

TerryE commented 4 years ago

What is unclear to me is how many of the issues that people report in nodemcu-firmware with files are actually underlying issues in spiffs.

The wrapper around SPIFFS is very thin so if we do have corruption issues then I don't think that this is our code, but diagnosing SPIFFS issues is hard, especially when we don't have a repeatable script which can take a newly formatted FS and corrupt it. I did think of porting the file module to the lua.cross -e execution environment and this way we can hammer SPIFFS on the host.

NicolSpies commented 4 years ago

@TerryE , I have been trying for years to find some pattern in the apparently random file related failures. The following additional info might help: I only save json data, it work for weeks and then suddenly the file corrupts in a way that it is not readable anymore. The problem also occur when using file.putcontents() and file.getcontents(). @pjsg , are your observations related to json data. If it is, the problem might be sjson module related.

KT819GM commented 4 years ago

Exactly same problem exists for arduino users, so I'm pretty sure it's SPIFFS problem. Not sure though if two copies is a workaround (fail-proof), but maybe we could live with that making shadow copies for some files?

NicolSpies commented 4 years ago

Exactly same problem exists for arduino users, so I'm pretty sure it's SPIFFS problem. Not sure though if two copies is a workaround (fail-proof), but maybe we could live with that making shadow copies for some files?

Shadow copies only reduces the probability of getting stuck but it does not eliminate the problem.

TerryE commented 4 years ago

Personally think that one missing feature would be the ability to run a proper file system check facility on a SPIFFS instance.

pjsg commented 4 years ago

There is a SPIFFS_check API that does some checking. But it doesn't cure all the problems.

KT819GM commented 4 years ago

Oh, just now checked about SPIFFS_check especially on spiffs.h where SPIFFS_ERR_* are described. @TerryE how hard would be to implement Lua functionality for check's and error codes, it would be relatively easy to make controlled restart circuit and reset test unit as many times as needed before file reading error occurs. After error SPIFFS_check could be fired. In production units on every start if node.bootreason() will drop something like 3.6 we could force SPIFFS_check, and so on.

TerryE commented 4 years ago

Can I suggest discussion around how we improve SPIFFS to a different (but important) topic rather than here on "Replacement for SPIFFS" :smile:

pjsg commented 4 years ago

I changed the title of this ticket to be more neutral.

TerryE commented 4 years ago

I changed the title of this ticket to be more neutral.

Bugger. I do think improving SPIFFS and replacing it are two different discussions :laughing:

pjsg commented 4 years ago

My take is that nodemcu needs a reliable filesystem that can handle reading/writing/appending to files and not corrupt the filesystem (at least) in the absence of power fails. And do all of this with reasonable performance, in a reasonably flash-space efficient way without consuming much RAM. The lua application developer should be able to use the file api while making reasonable assumptions about its behavior -- e.g. it is posix-like.

It would be nice to be able to do the above in the presence of power fails as well.

TerryE commented 4 years ago

DOS and Windows OSes and their users lived with this issue for about 3 decades. Phil, I agree with what you say as an aspiration, but we struggle to get some 101 stuff resourced at the moment and I don't see any other committers about to step up to the plate. Hence, my current view is that this isn't a realistic objective in the short term.

devyte commented 4 years ago

Hi, we reached similar conclusions at the Arduino repo, and we have similar reports about random file corruption. Because SPIFFS development/maintenance has stagnated upstream, and because there is nobody to take over the repo there, we decided to deprecate its use, and integrated LittleFS as a replacement. It's not perfect, but overall it works very well, with similar resource use as SPIFFS. In any case, at least development is active there, so there is somebody to collaborate with. @pjsg I find your findings with fuzzing for LittleFS very interesting. Would it be possible to get details about what you found, and your discussions for fixes at the LittleFS repo? We could confirm from the Arduino side and help you push for the fixes.

TerryE commented 4 years ago

@devte, thanks for your iinput. I will take a look at LittleFS as well and post back. The only confusion from a NodeMCU perspective is that the implementation uses lfs as a namespace prefix which is going be really confusing because NodeMCU uses this for Lua Flash Store, which is a memory mapped updatable region that allows us to run large Lua apps direct from flash. :rofl:

pjsg commented 4 years ago

The big issue thread on LittleFS is https://github.com/ARMmbed/littlefs/issues/245 -- which ends with a conclusion by the author of a change that needs to be made to the free space marker. Essentially one of the current problems is that if power fails half way through writing a log entry, then after power up, the code may or may not recognize it as a corrupt log entry and deal with it.

I have a hacked together branch at https://github.com/pjsg/littlefs/tree/afl-fuzzing-revamp which adds the fuzzer support and also merges in a branch that addressed a bunch of issues.

I've just backmerged littlefs master into my branch -- and, as expected, the tests don't fail any more. The one trouble with fuzz testing is that the tests are incredibly sensitive to the code version. I just ran the fuzz testing, and it found 11 crashes in a couple of minutes. An example is below. This one doesn't involve a powerfail -- however there is a restart between API calls (this is the clean restart when the system is idle). Don't worry about the details of the log, but you can probably see the api calls being executed.... What is fascinating is that all of these api calls are required to demonstrate the failure. Leaving out any of them will make the test not fail (or return LFS_E_CORRUPT).

format/mount{r752/11,p112/2,e2} -> 0
  open(1, "6file6.xxxxxxxxxxxxxx", 0x103){r336/3,p48/1,e0} -> 0
{d1}   write(1, , 235) -> 235
  close(1){r272/2,p248/1,e0} -> 0
{d2}   open(1, "4file4.xxxxxxxxxx", 0x103){r592/5,p40/1,e0} -> 0
{d3}   remove("6file6.xxxxxxxxxxxxxx"){r576/5,p16/1,e0} -> 0
{d4}   write(1, , 2007){r2464/18,p1792/7,e1} -> 2007
{d5}   write(1, , 2007){r2048/8,p2048/8,e0} -> 2007
{d6}   write(1, , 2007){r2048/8,p2048/8,e1} -> 2007
{d7}   write(1, , 2007){r2048/8,p2048/8,e0} -> 2007
{d8}   write(1, , 2007){r2064/9,p2048/8,e1} -> 2007
{d9}   write(1, , 2007){r2048/8,p2048/8,e0} -> 2007
{d10}   write(1, , 2007){r1792/7,p1792/7,e1} -> 2007
{d11}   write(1, , 2007){r2048/8,p2048/8,e0} -> 2007
{d12}   close(1){r256/3,p224/2,e0} -> 0
{d13}   open(1, "4file4.xxxxxxxxxx", 0x103){r544/4,p0/0,e0} -> 0
  mount{r864/22,p0/0,e0} -> 0
  open(1, "4file4.xxxxxxxxxx", 0x103){r528/3,p0/0,e0} -> 0
  remove("4file4.xxxxxxxxxx"){r560/5,p16/1,e0} -> 0
{d14}   write(1, , 2007){r2416/16,p1792/7,e1} -> 2007
{d15}   finally close(1){r16/1,p0/0,e0} -> -84
test: afl/test_afl.c:706: run_fuzz_test: Assertion `err != LFS_ERR_CORRUPT' failed.
Aborted
pjsg commented 4 years ago

One of the complexities in LittleFS is that it supports an encrypted flash -- which means that you can't modify a 'line' once it has been written, and you can't tell if an area of flash has been written before. [When you erase the flash, it goes to all 'f's -- but the decrypted version of that could be anything.]

TerryE commented 4 years ago

One of the complexities in LittleFS is that it supports an encrypted flash

IMO, this is a major reason to discount this as an option for the ESP architectures. Why?

Other than "nasty knock-offs" USB and SD flash devices embed a flash controller which implements wear levelling and encryption, but internal to the controller to bare flash device interface, the controller still exploits the NAND-flash scheme implemented by the device.

ESP devices don't have this extra level of control but use a naked flash device. In this scenario, you have to trade off encryption <-> integrity <-> performance <-> device life. Any encryption which frustrates NAND writing rules will hit device life as the number of erase cycles needed to execute any logical device function will increase by factors, ditto performance efficient integrity schemes. There's no free lunch and this is a classic you can "have any 2 of 3" trade-off or in this case if you try to implement on-flash encryption then this becomes 2 of 4.

Note that the ESP8266 ROM code supports a Winbond W25Q32FV class device (here is the data sheet), which claims a 100K erase cycles which would seem to imply that it is uses SLC encoding, but if actual devices used MLC encoded devices, then the erase cycle life could drop by orders of magnitude. We've already has posters on this list who implement SPIFFS write-intensive apps reporting ESP module failure after months -- I assume because of page erase cycle exhaustion. Moving to this type of scheme could turn months into weeks.

BTW, the performance characteristics of its flash implementation aren't documented by Espressiff, but my own testing implies that these are quite nuanced and show a lot of thoughtful design. Essentially flash access runs in one of two modes:

In practice, the SDK and RTOS both employ wrappers around the BootROM SPI routines to do this, and these go to some lengths to avoid loss of ICACHE coherence or loss of performance from unnecessary cache purges. Flash reading disables and reenables ICACHE without dumping, and even flash write seems to be quite smart avoiding dumping, but still maintaining coherence when writing to the ICACHE-mapped address space. Overall read and write performance is fast and comparable, but the killer is the slow page erase times, and the BootROM routines do this synchronously, so the page erase time absolutely dominate write performance. The only way to set a single bit from 0 to 1 is to do a read-erase-rewrite cycle on a minimum 4Kb sized page.

pjsg commented 4 years ago

@TerryE I think that I misspoke -- it supports an encryption layer between the filesystem and the flash. This layer is, of course, not required (and is not supplied as part of littlefs anyway).

It is a legitimate question to ask what a particular workload would do to the flash chip in terms of erase cycles on a page. It would be interesting to compare this between filesystems as another criterion.

One example workload might be (for a 1M flash)

Setup: write 100 files of average size 4k, but varying from 1 byte to 64k (this might be a web site)

create logfile

loop: open logfile append 100 bytes to logfile close logfile if it is more than 100k bytes, then delete logfile.3, rename logfile.2 to logfile.3, logfile.1 to logfile.2, logfile to logfile.1 and create logfile

maybe every 1000 loops, rewrite all the web server files.

Do the loop say 10000 times and look at the wear stats.

TerryE commented 4 years ago

IMO, If we are doing heavy logging of the form that you are suggesting, then a Flash-based cyclic FS similar to #1199 would be the simplest and most efficient way of achieving this.

In terms of write load, your (open for append, append, close) cycle is a very inefficient way of implementing tihs:

We can't easily collect wear stats on an ESP-based SPIFFS, but we could on one within the luac.cross execution environment, ant this would also be a really good test bed to do fuzzing-style stress testing.

pjsg commented 4 years ago

I wasn't proposing to do these tests on an actual ESP part -- just in a simulation environment. I agree that the workload that I suggest is not ideally suited for the ESP, but, if people are burning through the erase cycles in weeks, then they must be doing something like this. Your argument to use flush is one that we probably ought to document carefully to prevent the inefficiencies of open/write/close.

I'm open to building a test harness to evaluate these in a test environment -- I'd like to have a realistic workload where we could evaluate the results. Obviously the workload will involve a lot of writes -- otherwise there will be no effective wear at all....

pjsg commented 4 years ago

I did build such a test harness and it showed that spiffs is much better than littlefs when doing flushes. However, I'm a little bit puzzled by the data that is coming from the spiffs test harness. Spiffs also seems to do a much better job of wear leveling, but I'm not sure that I beleive the stats that I am seeing.

devyte commented 4 years ago

I believe SPIFFS does wear leveling across the entire FS space, including space that is used, but LittleFS does wear leveling only on empty space. If you have a large file spanning across a lot of your FS space, and that file is never changed, then that space is never leveled out. It also means that data is absolutely safe no matter what on LittleFS. If you always write to different areas and delete stuff and rewrite all the time, then you burn a bit, on SPIFFS it could hit on new data, or on static data that wasn't even involved in the operation. Different design choices. This comes from my own memory of reading about it quite some time ago, so please take with a grain of salt.

pjsg commented 4 years ago

That seems to accord with what I see happening. I suspect that the usage pattern makes a huge difference.

pellepl commented 4 years ago

@develo hi, yes you're right - that's the way spiffs does wear leveling. Don't know how lfs does it.

Den ons 10 juni 2020 04:15Develo notifications@github.com skrev:

I believe SPIFFS does wear leveling across the entire FS space, including space that is used, but LittleFS does wear leveling only on empty space. If you have a large file spanning across a lot of your FS space, and that file is never changed, then that space is never leveled out. It also means that data is absolutely safe no matter what on LittleFS. If you always write to different areas and delete stuff and rewrite all the time, then you burn a bit, on SPIFFS it could hit on new data, or on static data that wasn't even involved in the operation. Different design choices. This comes from my own memory of reading about it quite some time ago, so please take with a grain of salt.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nodemcu/nodemcu-firmware/issues/3148#issuecomment-641678129, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABG42VYXNMX2HBRERVZR3GDRV3UDXANCNFSM4NWTIZGQ .

TerryE commented 4 years ago

SPIFFS uses a system of allocating 32 × 256 byte pages per logical 8Kb block (as we have it configured). I/O churn will slowly consume clean pages and mark others as dirty, until you get to point where you are running out of unused pages. If you've got a block with 26 dirty + 6 live pages (for example) and you need another blanked 8Kb block then you must move the 6 live pages before the GC can erase and release the new batch of 32 pages to the unused pool. GC has to kick in before you get to a catch-22 where there isn't enough unused pages left to do this first do this 1st GC move.

Doing GC earlier that "last minute" is safer and avoids the need to do GC at application time critical periods where you would want good SPIFFS performance. Given that apps will typical work on a master timer or request / response cycle, having the app being able to schedule an informative "now is a good point to do some SPIFFS GC if needed" would be useful, that is a file.collectgarbage(), I think.

I am not sure if I understand @devyte's point (or at least it's validity):

LittleFS does wear levelling only on empty space. If you have a large file spanning across a lot of your FS space, and that file is never changed, then that space is never levelled out. It also means that data is absolutely safe no matter what on LittleFS.

if the allocation unity was smaller than a 4Kb flash erase unit, as it is trivial to create R/W scenarios where every FS block had at least one valid page on it. So I can't see how this claim could be true in this case.

devyte commented 4 years ago

@pellepl omg I haven't seen much from you in ages. I'm glad to see you're still around and I hope you're doing well! 😄

devyte commented 4 years ago

@TerryE I am talking about SPIFFS doing both dynamic and static wear leveling, while LittleFS does only dynamic wear leveling. Maybe a more accurate thing to say than my previous comment would be that for LittleFS full blocks are safe no matter what. That is not the case for SPIFFS, where a block can be swapped due to wear levelling even if it's full, and could therefore potentially fail, even if that block has absolutely nothing to do with the write operation at hand. Of course the other side of the coin is that, if there are static files, then LittleFS will wear level only the emtpy space, so that could fail earlier, while SPIFFS will wear level the whole FS space, and would likely last longer. But as I said, my comments come from memory from reading up on these things a while ago, so I could very well be wrong. I certainly haven't done any kind of deep investigation or extensive testing 🤷‍♂️

pellepl commented 4 years ago

@devyte hehe thanks, yeah all fine, hope you are too. You're completely right about the wear leveling in the spiffs case - even a block with only written pages will eventually be moved by GC, even though it won't contribute to freeing any pages - in order to have completely fair leveling. Regarding the other discussion the thought was that a call to spiffs_check will clean away all garbage and inconsistencies from aborted writings. That function did become sort of a beast unfortunately so I am not surprised if there is a corner case I missed. Journalling was to be the saviour in v2, but life moved on for me.

NicolSpies commented 4 years ago

@pjsg, @TerryE , I have done extensive SPIFFS reading and writing testing in the last 28 days in order to attempt to find a pattern in the occurrences of the anomalies reported.

The following test method was used: A 900 byte json file is read from the SPIFFS using file.getcontents(), the json file is decoded as a table using sjson.decode(), a value in the table is modified before it is encoded using sjson.encode() and saved again as a json file using file.putcontents. Using a process of protected calls and a shadow file copy, the testing process could be repeated every few seconds over 100K cycles while logging where the failures occurs. The shadow file copy was used for error correction to enable continued testing without interruption. The testing was done on my development hardware platform that has been in daily use for development over the last few months.

The following was observed:

I am certain I have not exhausted the write cycles of the flash memory used by the SPIFFS. I am using a fraction of the 3MB available in the SPIFFS. I assume wear leveling is being used. It appears as if the anomalies increase with the used of the SPIFFS. It appears as if the increased occurrence can be cleared or reset by properly erasing the flash memory.

I hope the above might be useful to shed some light on what is being experienced.

TerryE commented 4 years ago

@NicolSpies Nichol, I need to brood about this one. :thinking:

pjsg commented 4 years ago

I have found issues with SPIFFS though they were mostly due to having multiple files open at the same time (I think that Nodemcu configures 4 open files). @NicolSpies observations do not surprise me. The real problem is that SPIFFS is not really maintained any more.

The question is "What action do we take?"

TerryE commented 4 years ago

Towards the end of the testing the following errors started increasing: ERROR in flash_read: r=1 at XXXX and ERROR in flash_write: r=1 at YYYY.

Those two ERROR in flash reports are generated in app/platform/platform.c in routines platform_s_flash_read() and platform_s_flash_write() which are mainly wrappers around the SDK spi_flash_read() and spi_flash_write() functions which return the status. This status is the number of bytes read / written in the case of a successful operation or zero in the case of an error.

Both of these spi routines are in the SDK component libmain.a(spi_flash.o) and looking at a disassembly (I won't bother posting this as few readers will understand the xtensa machine code) these both essentially do:

The Cache disable / enable is because the CPU can't execute code from flash (IROM0) whilst using the SPI interface to read or write data from flash, and for the same reason these spi_flash.o functions are in the IRAM1 segment.

But the main thing to note here is that these routines return an error status as well as printing the diagnostic error.

My main issue here is not with the SPIFSS code but with the NodeMCU spiffs.c wrapper which implements the platform-specific flash operation, and this code dates right back to the first release 6 years ago. For example:

static s32_t my_spiffs_read(u32_t addr, u32_t size, u8_t *dst) {
  platform_flash_read(dst, addr, size);
  return SPIFFS_OK;
}

The write wrapper has the same failing, and that is to treat a lower level routine which returns a status as a type void and ignore the status always returning success. So both hard and soft errors fail invisibly. Such silent failures will lead to corruption of the FS, and we can't blame the SPIFFS code for this.

Such hardware errors can either be soft or hard.

At a minimum these routines should do soft retries and collect some stats on their occurrence; stats that can be collected through some reporting function.

We also should have a policy for how hard errors are handled. Simply ignoring them isn't a real policy, IMO.

In general the SPIFSS code used a "return status and check" strategy. There are 160 cases where the routine aborts on error returning any non success status up the call stand and 114 where it does error-specific logic. I don't have the bandwidth to review all these but what I can say is that we should at least look to fixing our own basic errors before discarding SPIFFS out of hand.


@pjsg, you have the C skills to put in some basic retry logic and error stats collection in these spiffs.c routines and to work with @NicolSpies on this. @nwf, I know that you've been looking at this was well. So Philip, can I propose that you take the lead on adding soft error retry and logging to see if this makes the FS any more stable?

At least we get then some stats on error rates and the % of soft vs hard flash errors and the average number of retries needed (say treat as hard error after 3 retries).

Thanks.

TerryE commented 4 years ago

@nwf @HHHartmann @NicolSpies, I am not sure if Philip picked this up or has other immediate pressures. This is a simple and limited scope fix, if any of you (or any other contributor reading this) are interested in doing this PR then let me know here and I will be happy to act as reviewer.

pjsg commented 4 years ago

I looked at this, and my concern was whether the write could be retried. If we don't understand why the write fails, then it is unclear that retrying it is actually a good thing (if we wrote the wrong thing first time round). The read can probably be retried. The other option for the write would be do retry the write operation and then do a read-verify to see if it actually wrote the right stuff -- and only fail if it didn't.

TerryE commented 4 years ago

I looked at this, and my concern was whether the write could be retried.

@pjsg Philip, thanks for getting back on this. You've clearly been thinking about the issues.

IMO, yes it can be retried, but as you suggest: we should then do a read comparison to validate that the write has executed correctly. The slight complication here is that we would need to malloc a temporary work buffer to read back the content and compare this with the original, but I think that this is entirely acceptable if it gives use a more resilient SPIFFS.

Collecting some basic retry stats (how may soft failures that have been recovered since restart; how many hard failures) that we can read through some API call would be a really useful addition here.

. The issue is that the previous write might have left the content with some bits invalidly zero, so the safest / simplest approach is to

NicolSpies commented 4 years ago

In the spirit of gathering information about the SPIFFS random failure, it appears as if the write or read errors is more prevalent with larger json files in the region of 1kB.

A behavior not observed before related to the occurrence of the error, is the disappearance of 99% of the files on the SPIFFS. It appears as if the files are still present but not visible based on the used and remaining space reported.

pjsg commented 3 years ago

I now have a port of the littlefs code info nodemcu which appears to work. You can choose either SPIFFS or LIttlefs at build time. The good news with littlefs is that the author is actively engaged and is actually working on fixing problems to do with power failure and the nasty things that the flash chips can do during that time.

One issue is that littlefs (like FAT) support directories, but the existing file code has limited directory support. I've added mkdir, but it is less clear what to do about file.list() as it doesn't take a directory argument. I don't know how you list files not in the root directory for FAT. file.list already takes a single optional argument (the match pattern) so it isn't obvious how to extend it in a backwards compatible way.

Any thoughts?

HHHartmann commented 3 years ago

I would guess, that FAT has a state and knows which directory it is in and returns the files in the current directory. But that's just reasoning. It would however be nice to get a list of files of a directory without having to change to it first.

The param to file.list() could be matched against filenames as returned by unix find but that would probably not be really efficient. littlefs should work the same way.

To seamlessly use littlefs it would be interesting to allow a non directory version to make sure that filenames like mydir/file and mydir\file work without first having to create the directory. But maybe that't the cost of using the 'new stuff'

pjsg commented 3 years ago

Currently I have the littlefs code as a git submodule -- i.e. there are no changes to it. This will make it much easier to track upstream changes. That code uses "/" as the directory seperator and there isn't any obvious way to change that character. The other approach would be to mangle the filenames in the vfs->littlefs glue layer and swap "/" and some other character that isn't used. But this seems like the path to madness.

I was think of having file.list take either nothing, or a string, or a table. The table form would have up to two members -- path and pattern. The other option would be to say that the second argument to list is the directory name.... This is probably the easiest approach.

pjsg commented 3 years ago

The approach that I'm taking is that you choose at build time whether you want SPIFFS or LITTLEFS. I'm going to see if I can get the NTest file tests running -- it will be a good test!

HHHartmann commented 3 years ago

The approach that I'm taking is that you choose at build time

That's what I understood.

The file tests are a starting point but far from complete. So please don't feel too safe when they pass.

pjsg commented 3 years ago

One of the tests fails -- the rename of an existing filename to an existing filename. This actually works on littlefs and does what you would expect (i.e the destination file contents are lost, and the source file name no longer exists) -- i.e. it behaves more like unix.

I can prevent this from working, but it seems like a useful operation.

On Sun, Dec 20, 2020 at 10:58 PM Gregor Hartmann notifications@github.com wrote:

The approach that I'm taking is that you choose at build time

That's what I understood.

The file tests are a starting point but far from complete. So please don't feel too safe when they pass.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nodemcu/nodemcu-firmware/issues/3148#issuecomment-748743623, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALQLTOEFWSKKJWB4TDQNHLSV3BXVANCNFSM4NWTIZGQ .

nwf commented 3 years ago

:+1: for the utility of rename-over-existing-file. Can SPIFFS be readily made to do the same (or is the longer-term plan to jettison SPIFFS in favor of LITTLEFS)?

Also :+1: for an optional second argument to file.list() for the directory name. If we wish to seriously pursue a VFS, maybe we should have directory objects in addition to file objects, but that might be a bridge too far.

TerryE commented 3 years ago

or is the longer-term plan to jettison SPIFFS in favor of LITTLEFS

Since LittleFS has no inbuilt wear levelling, I expect that the life of ESP modules using it for inbuilt Flash (which is directly addressed) will be a lot less than with SPIFFS so I suggest that at best this is an alternative for the foreseeable future.

pjsg commented 3 years ago

According to the littlefs docs:

Dynamic wear leveling - littlefs is designed with flash in mind, and provides wear leveling over dynamic blocks. Additionally, littlefs can detect bad blocks and work around them.

On Thu, Dec 24, 2020 at 6:44 PM Terry Ellison notifications@github.com wrote:

or is the longer-term plan to jettison SPIFFS in favor of LITTLEFS

Since LittleFS has no inbuilt wear levelling, I expect that the life of ESP modules using it for inbuilt Flash (which is directly addressed) will be a lot less than with SPIFFS so I suggest that at best this is an alternative for the foreseeable future.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nodemcu/nodemcu-firmware/issues/3148#issuecomment-751131057, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALQLTIIBFWPXMMSHKPCQOLSWPG4RANCNFSM4NWTIZGQ .

TerryE commented 3 years ago

Dynamic wear leveling - littlefs is designed with flash in mind...

Philip, thanks for pointing this out. My bad; I should have RTFM: 😱 Here is the Little FS Readme documentation. I have struck out my previous comment. Nonetheless, I feel that the SPIFFS vs LittleFS selection should remain an either / or in the immediate future.