EtchedPixels / FUZIX

FuzixOS: Because Small Is Beautiful
Other
2.13k stars 267 forks source link

rpipico: added proper reboot and halt support #1049

Closed veremenko-y closed 3 months ago

veremenko-y commented 3 months ago

Couldn't make my pi pico to shutdown cleanly, so I did the following:

Updated /etc folder in the image. Added killall, substroot, links to halt and shutdown. Implemented proper reboot and halt.

veremenko-y commented 3 months ago

Hm... After testing a bit more, I still corrupts file system on shutdown. Something doesn't flush properly here before the shutdown. If I put bunch of echoes in between every statement in the rc.halt it works well.

Any tips on which part of the kernel I should look at?

EtchedPixels commented 3 months ago

The core kernel code should have written back everything as far as it is concerned (and the fact this isn't showing up on other targets suggests it is).

I would look at two things I think

Might also be worth seeing what "remount / ro; reboot -f" does in terms of corruption with or without a delay

Pico is really David Given's thing, it's not a platform I deal with

veremenko-y commented 3 months ago

Had to make more cables to plug the SD card into pico, but I can confirm now that shutdown and reboot do not dirty filesystem on SD card.

EtchedPixels commented 3 months ago

I would guess there is some kind of flash writeback not happening then.

Looking at the dhara docs it has a

dhara_sync(): Make sure the changes to the map are committed.

I don't see what anywhere in the driver, so possibly the devflash driver needs something like

blk->flush = devflash_sync;

and devflash_sync would be something like

              dhara_errror_t err;
              dhara_map_sync(&dhara_map, &err);
              if (err != DHARA_E_NONE ) { kprintf("something"); udata.u_error = EIO ; return -1; }
              return 0;

that would update the map on sync() and sync() is called on exit.

Not sure why this isn't present or if it needs to be called less often than sync or something because it causes more writing. If so then it probably belongs in the platform reboot path and on an idle timer of some sort.

@davidgiven ?

davidgiven commented 3 months ago

Sorry, too long ago --- possibly I just forgot? Or the dhara_map_sync() only came along later?

It would make sense to call it from sync(), but I agree that frequency is a concern here. I don't know if Fuzix calls it a lot. At the very least it should be called on unmount, so that remount / ro causes a flush. But then you'd be out of luck if the system crashed.

veremenko-y commented 3 months ago

I agree that frequency is a concern here

I'm with you on that. I see that dhara_map_sync only flushes if journal is dirty, so I can try to check manually in flush callback to profile it.

I'll probably get back to it a little later. For no practical reasons I'm trying to run FUZIX in multiuser mode with 1 UART and 2 USB CDC ttys, and it generally works besides concurrency issues...

EtchedPixels commented 3 months ago

Fuzix calls sync on any task end rather than timed/shutdown.

veremenko-y commented 3 months ago

I ran two versions of the kernel, with or without sync running and collected stats (by adding to ^Z idump).

With dhara sync

ls
ls /bin
touch test
# Flash Stats                                                    
   prog 455 erase 57  = total 512                                
fs write: 341 sync 9 = total 350   
cat > test2
123123123
# Flash Stats                                                    
   prog 531 erase 66  = total 597                                
fs write: 398 sync 12 = total 410 

Without dhara sync

ls
ls /bin
touch test
# Flash Stats                                                    
   prog 457 erase 57  = total 514                                
fs write: 343 sync 9 = total 352    
cat > test2
123123123
# Flash Stats                                                    
   prog 525 erase 66  = total 591                                
fs write: 394 sync 12 = total 406                                

Amount of program/erase cycles is only slightly increased.

On a side note, I have two questions:

  1. Is there a forum to talk and ask questions about FUZIX besides this repo?
  2. I have the code that allows to run UART and USB consoles independently in multiuser mode. Do you want this code up streamed or platform is not really being used enough?
EtchedPixels commented 3 months ago

It would be good to have stuff upstream when possible yes.

There is a google group for Fuzix but it seems everything ends up here instead!

davidgiven commented 3 months ago

I have very reluctantly enabled github discussions on one of my projects, and it actually seems to work, even having a little email-driven functionality... haven't found a way to import mailing list archives into it yet, though.

Regarding multiple tty support, I could actually use that myself!

veremenko-y commented 3 months ago

Regarding multiple tty support, I could actually use that myself!

Alright, I'm convinced, it's going to be used. I'm going to cleanup my branch and make another PR.