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

oofats: Check on block size during mkfs is done on block numbers, rather than bytes. #4305

Closed
hoihu opened this issue Nov 16, 2018 · 11 comments

Comments

@hoihu
Copy link
Sponsor Contributor

hoihu commented Nov 16, 2018

During development of support for the FAT FS for nRF52832 I stumbled across this line
https://github.com/micropython/micropython/blob/master/lib/oofatfs/ff.c#L5144

where it indicates (in my opinion) that the minimum requirement for mkfs to work is to have at least 50 blocks (independant of the block size!).

Since the nRF52 has a flash block size of 4kB, this check always resulted in OSError 5 since the maximum allowed FS space on this system is about 100kB. I guess the check for 50 makes sense for a block size of 512 Bytes.

Settings this check to be lower worked on my system and the flash FS was then working ok. I guess I could have added a SW layer where the flash page size is independant of the FS block size ( I guess that this must have been done on the STM32 device too?) but it was the most obvious and simple solution for me.

I wonder if this a bug or if I misinterpreted something.

Thanks for any feedback

@dpgeorge
Copy link
Member

I was under the impression that the minimum for a FAT FS is measured in blocks, since it stores a few copies of the FAT in different locations, and I guessed these locations would be based on block number, irrespective of size of the block.

But doing some tests (with the vfs_fat_ramdisk.py test) shows that this is not the case: to format a device and write a single small file, the following blocks are used:

  • block size=512 bytes; blocks 0, 1, 2, 10, 18, 26 used for formatting, file written to block 34 (17k)
  • block size=1024 bytes; blocks 0, 1, 2, 6, 10, 14 used for formatting, file written to block 18 (18k)
  • block size=2048 bytes; blocks 0, 1, 2, 4, 6, 8 used for formatting, file written to block 10 (20k)
  • block size=4096 bytes; blocks 0, 1, 2, 3, 4, 5 used for formatting, file written to block 6 (24k)

So probably that line in ff.c could be changed to take into account the block size.

@hoihu
Copy link
Sponsor Contributor Author

hoihu commented Nov 25, 2018

Thanks for your reply.

I guess a sensible way (assuming we don't want to have a code change for the often used block size = 512) would be something like 50 / BLOCK_SIZE * 512?

I'm not sure if this issue should be raised upstream actually. However I saw that in your forked repository on https://github.com/micropython/oofatfs/blob/vendor/src/ff.c there is already a separate value for this (128) so I thought perhaps better not.

PS: The size you printed in () is excluding counting block 0 I presume?

@dpgeorge
Copy link
Member

I guess a sensible way would be something like 50 / BLOCK_SIZE * 512?

That's close, but for 4096 byte blocks it will set a minimum of 6 blocks which is not enough, it needs at least 7 to write a file.

I'm not sure if this issue should be raised upstream actually.

If it was it would be a subtly different issue (because they have 128 as the limit). I don't know how responsive they would be to a change, and it would take time to filter down here. So we can just patch it in oofatfs.

The size you printed in () is excluding counting block 0 I presume?

Yes. The size is the "address" of the first block used to write the file.

@hoihu
Copy link
Sponsor Contributor Author

hoihu commented Nov 26, 2018

it will set a minimum of 6 blocks which is not enough, it needs at least 7 to write a file.

maybe I missinterpreted something here so sorry in advance 😉

I thought it would take 6 blocks to hold the FAT table and that was that the if statement was checking (making sure that the FAT table fits during mkfs)

Then the first file would be on block 7 as you have stated.

@hoihu
Copy link
Sponsor Contributor Author

hoihu commented Feb 28, 2019

One option might be to specify the required block numbers based on #ifdefs rather than a formula. The intention would be that the multiplication block size*blocknumber is > 25kB

block size | req. block number to hold FAT
4096 | 7
2048 | 13
1024 | 25
512 | 50

not very scientific but I guess that should moreless work. The calculation could be done at runtime too (as suggested #4551 (comment))

@dpgeorge
Copy link
Member

dpgeorge commented Mar 1, 2019

@hoihu, yes, that formula would be ok, done at runtime.

@dpgeorge
Copy link
Member

dpgeorge commented Mar 2, 2019

It turns out that more blocks are required for a minimum filesystem with sector size of 4096, it needs at least 22. See micropython/oofatfs#3 (comment)

@dpgeorge
Copy link
Member

dpgeorge commented Mar 5, 2019

Fixed in 1a24bac

@dpgeorge dpgeorge closed this as completed Mar 5, 2019
@hoihu
Copy link
Sponsor Contributor Author

hoihu commented Mar 5, 2019 via email

@dpgeorge
Copy link
Member

dpgeorge commented Mar 5, 2019

I wonder though - what is the new block number minimum requirements for a 4k block size?

22 blocks, so 88k bytes. That seems to be a strict limit in FatFS.

@hoihu
Copy link
Sponsor Contributor Author

hoihu commented Mar 5, 2019 via email

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

No branches or pull requests

2 participants