FatFS not working - Githubissues

deadprogram commented 9 months ago

The FatFS is not working correctly, apparently ever since https://github.com/tinygo-org/tinygo/issues/3460

$ tinygo flash -target nano-rp2040 -stack-size 8kb -monitor ./examples/console/fatfs/machine/
Connected to /dev/ttyACM0. Press Ctrl-C to exit.
SPI Configured. Reading flash info
==> lsblk

-------------------------------------
 Device Information:  
-------------------------------------
 flash data start: 10026000
 flash data end:  11000000
-------------------------------------

==> mount
Could not mount filesystem: fatfs: (13) There is no valid FAT volume

==> format
Successfully formatted filesystem.

==> mount
Could not mount filesystem: fatfs: (13) There is no valid FAT volume

bgould commented 9 months ago

I was able to confirm the above behavior (seemingly corrupted filesystem after formatting) using external SPI flash memory as well. Incidentally, if I flash Circuit Python onto the board and let it create its default files, I can mount the filesystem and read the files successfully:

$ tinygo flash -target=feather-m4 -size=short -monitor
   code    data     bss |   flash     ram
 153264    2320    6972 |  155584    9292
Connected to /dev/ttyACM1. Press Ctrl-C to exit.
SPI Configured. Reading flash info
==> 
==> mount
Successfully mounted LittleFS filesystem.

==> ls
drwxrwxrwx     0 .fseventsd
-rwxrwxrwx     0 .metadata_never_index
-rwxrwxrwx     0 .Trashes
-rwxrwxrwx     0 settings.toml
-rwxrwxrwx    22 code.py
drwxrwxrwx     0 lib
-rwxrwxrwx   155 boot_out.txt
==> cat code.py
00000000: 70 72 69 6e 74 28 22 48 65 6c 6c 6f 20 57 6f 72    print("Hello Wor
00000010: 6c 64 21 22 29 0a                                  ld!").

bgould commented 9 months ago

I think it is possible that this never actually worked properly ...

When first added, the FATFS_READONLY flag was set to be read-only: https://github.com/tinygo-org/tinyfs/commit/0610c74a0ce1ba9278f2aab54aaa52803a05f61c#diff-617510d59fc788cf34e3af911f36ff1ccc2b0478a9c2b9f440a5f1c18a838654R15

Read-write was enabled when the sdcard driver was developed - - but the necessary enhancements for FatFS to work properly on flash devices is st

I believe the primary reason that I didn't enable writing for fatfs was because the flash driver would need to do some sort of more advanced buffering than it is currently doing, and fatfs is not good for flash devices anyhow without some sort of wear-leveling handled in software. I suspect/believe that this consideration does not apply to SD cards, so if this PR makes it work then I would be in favor of it. - https://github.com/tinygo-org/tinyfs/pull/5#issuecomment-864496409

In both of those cases, format operations using the library maybe was never tested, if the filesystems were created on some other system first.

I'm thinking that a few improvements might be needed here

[x] make sure the format operation is working
[ ] improve configurability of read-only/read-write ... maybe ideally read/write should be opt-in for safety, though that would be a breaking change
[ ] add a block-sized buffer that can automatically/efficiently handle erasing blocks as writes happen (the FatFS callbacks are structured differently than the LittleFS callbacks, for LittleFS there is an explicit "erase" callback that makes such logic unnecessary)
[ ] Add documentation about when FatFS should be read-only or read-write

deadprogram commented 9 months ago

Great research @bgould thank you very much for all this info. I was looking in all the wrong places.

improve configurability of read-only/read-write ... maybe ideally read/write should be opt-in for safety, though that would be a breaking change

That is very confusing. We are better off documenting this and offering a read-only mode for experts, but defaulting to r/w.

add a block-sized buffer that can automatically/efficiently handle erasing blocks as writes happen (the FatFS callbacks are structured differently than the LittleFS callbacks, for LittleFS there is an explicit "erase" callback that makes such logic unnecessary)

Not surprising. I learned a bit about how this is harder than it first looks when trying to implement ReaderWriterCloser on the Flash. The buffer sounds like a decent approach to add.

make sure the format operation is working

Yes please!

Add documentation about when FatFS should be read-only or read-write

also yes please and thank you!

bgould commented 9 months ago

make sure the format operation is working

There is definitely a bug or something with Format operation, even this is failing in full-sized Go, and I know this used to be working. Continuing to investigate

package main

import (
    "fmt"
    "os"

    "tinygo.org/x/tinyfs"
    "tinygo.org/x/tinyfs/fatfs"
)

func main() {

    // create/format/mount the filesystem
    fs := fatfs.New(tinyfs.NewMemoryDevice(4096, 64, 64))
    _ = fs
    if err := fs.Format(); err != nil {
        fmt.Println("Could not format", err)
        os.Exit(1)
    }
}

$ go run .
Exception 0xc0000005 0x0 0x0 0x7ff600d91450
PC=0x7ff600d91450
signal arrived during external code execution

runtime.cgocall(0x7ff600d8d8e0, 0xc000107e38)
        C:/Users/bcg/tools/go1.21.4/go/src/runtime/cgocall.go:157 +0x3e fp=0xc000107e10 sp=0xc000107dd8 pc=0x7ff600d048fe
tinygo.org/x/tinyfs/fatfs._Cfunc_f_mkfs(0x0, 0x1, 0x0, 0xc000108000, 0x200)
        _cgo_gotypes.go:245 +0x55 fp=0xc000107e38 sp=0xc000107e10 pc=0x7ff600d8c8d5
tinygo.org/x/tinyfs/fatfs.(*FATFS).Format.func1(0x7ff600d5e4ff?, 0xc000107ea8)
        C:/Users/bcg/src/tinyfs/fatfs/go_fatfs.go:199 +0xb5 fp=0xc000107e88 sp=0xc000107e38 pc=0x7ff600d8cd95
tinygo.org/x/tinyfs/fatfs.(*FATFS).Format(0x1000?)
        C:/Users/bcg/src/tinyfs/fatfs/go_fatfs.go:199 +0x4d fp=0xc000107ed0 sp=0xc000107e88 pc=0x7ff600d8cc8d
main.main()
        C:/Users/bcg/src/tinyfs/examples/simple-fatfs/main.go:16 +0x55 fp=0xc000107f40 sp=0xc000107ed0 pc=0x7ff600d8d5b5
runtime.main()
        C:/Users/bcg/tools/go1.21.4/go/src/runtime/proc.go:267 +0x2b2 fp=0xc000107fe0 sp=0xc000107f40 pc=0x7ff600d36af2
runtime.goexit()
        C:/Users/bcg/tools/go1.21.4/go/src/runtime/asm_amd64.s:1650 +0x1 fp=0xc000107fe8 sp=0xc000107fe0 pc=0x7ff600d5e8e1

goroutine 2 [force gc (idle)]:
runtime.gopark(0x0?, 0x0?, 0x0?, 0x0?, 0x0?)
        C:/Users/bcg/tools/go1.21.4/go/src/runtime/proc.go:398 +0xce fp=0xc00004ffa8 sp=0xc00004ff88 pc=0x7ff600d36f0e
runtime.goparkunlock(...)
        C:/Users/bcg/tools/go1.21.4/go/src/runtime/proc.go:404
runtime.forcegchelper()
        C:/Users/bcg/tools/go1.21.4/go/src/runtime/proc.go:322 +0xb8 fp=0xc00004ffe0 sp=0xc00004ffa8 pc=0x7ff600d36d98
runtime.goexit()
        C:/Users/bcg/tools/go1.21.4/go/src/runtime/asm_amd64.s:1650 +0x1 fp=0xc00004ffe8 sp=0xc00004ffe0 pc=0x7ff600d5e8e1
created by runtime.init.6 in goroutine 1
        C:/Users/bcg/tools/go1.21.4/go/src/runtime/proc.go:310 +0x1a

goroutine 3 [GC sweep wait]:
runtime.gopark(0x0?, 0x0?, 0x0?, 0x0?, 0x0?)
        C:/Users/bcg/tools/go1.21.4/go/src/runtime/proc.go:398 +0xce fp=0xc000051f78 sp=0xc000051f58 pc=0x7ff600d36f0e
runtime.goparkunlock(...)
        C:/Users/bcg/tools/go1.21.4/go/src/runtime/proc.go:404
runtime.bgsweep(0x0?)
        C:/Users/bcg/tools/go1.21.4/go/src/runtime/mgcsweep.go:280 +0x94 fp=0xc000051fc8 sp=0xc000051f78 pc=0x7ff600d21e74
runtime.gcenable.func1()
        C:/Users/bcg/tools/go1.21.4/go/src/runtime/mgc.go:200 +0x25 fp=0xc000051fe0 sp=0xc000051fc8 pc=0x7ff600d17205
runtime.goexit()
        C:/Users/bcg/tools/go1.21.4/go/src/runtime/asm_amd64.s:1650 +0x1 fp=0xc000051fe8 sp=0xc000051fe0 pc=0x7ff600d5e8e1
created by runtime.gcenable in goroutine 1
        C:/Users/bcg/tools/go1.21.4/go/src/runtime/mgc.go:200 +0x66

goroutine 4 [GC scavenge wait]:
runtime.gopark(0xc000010150?, 0x7ff600ddf748?, 0x1?, 0x0?, 0xc00004cb60?)
        C:/Users/bcg/tools/go1.21.4/go/src/runtime/proc.go:398 +0xce fp=0xc000061f70 sp=0xc000061f50 pc=0x7ff600d36f0e
r8      0x0
r9      0xc000108000
r10     0x3a
r11     0x40
r12     0xffffffffffffffff
r13     0x1000
r14     0xc00004c000
r15     0x3c9
rip     0x7ff600d91450
rflags  0x10206
cs      0x33
fs      0x53
gs      0x2b
exit status 2

bgould commented 9 months ago

Turns out that the simple-fatfs example needed updating with a call to the Configure() method, and a couple of other tweaks, but at least is working from tinygo run (on Linux, not working on Windows) with the tweaks in https://github.com/tinygo-org/tinyfs/pull/16

tinyfs/examples/simple-fatfs $ tinygo run .
Formatting...
formatting:  0x00007fc0b05af030 1 [512/512]0x00007fc0b06406d0 512
Mounting...
making directory /tmp
opening file /tmp/test.txt
Filesystem free: 473088
closing file
dir stat: name=tmp size=0 dir=true
file stat: name=test.txt size=340 dir=false
opening file read only
read 57 bytes from file: `01234567890abcdef01234567890abcdef01234567890abcdef012345`read 57 bytes from file: `67890abcdef01234567890abcdef01234567890abcdef01234567890a`read 57 bytes from file: `bcdef01234567890abcdef01234567890abcdef01234567890abcdef0`read 57 bytes from file: `1234567890abcdef01234567890abcdef01234567890abcdef0123456`read 57 bytes from file: `7890abcdef01234567890abcdef01234567890abcdef01234567890ab`read 55 bytes from file: `cdef01234567890abcdef01234567890abcdef01234567890abcdef`Filesystem free: 472576
  directory entry: test.txt 340 false
done

soypat commented 7 months ago

FYI Created an idiomatic FAT filesystem library in pure Go over here: https://github.com/soypat/fat. Be great to get some testing in with it to see if we can replace these pesky CGo libraries!

tinygo-org / tinyfs

FatFS not working #15