vfs: increase FATFS_VFS_FILE_BUFFER_SIZE (again)

[fabian18]

Contribution description

With _FATFS_FILE_CACHE _FATFS_FILE_SEEK_PTR _FATFS_FILE_EXFAT the problem of #20297 is still present :/, so I increased FATFS_VFS_FILE_BUFFER_SIZE once again.

Testing procedure

CPU_MODEL = samd51j20a

In file included from /home/[email protected]//ext/RIOT/sys/include/fs/fatfs.h:28,
                 from /home/[email protected]//ext/RIOT/pkg/fatfs/fatfs_vfs/fatfs_vfs.c:29:
/home/[email protected]//ext/RIOT/sys/include/vfs.h:147:9: note: '#pragma message: VFS_NAME_MAX is 41'
  147 | #pragma message "VFS_NAME_MAX is "XTSTR(VFS_NAME_MAX)
      |         ^~~~~~~
/home/[email protected]//ext/RIOT/sys/include/vfs.h:148:9: note: '#pragma message: _FATFS_FILE_CACHE is 512'
  148 | #pragma message "_FATFS_FILE_CACHE is "XTSTR(_FATFS_FILE_CACHE)
      |         ^~~~~~~
/home/[email protected]//ext/RIOT/sys/include/vfs.h:149:9: note: '#pragma message: _FATFS_FILE_SEEK_PTR is (4)'
  149 | #pragma message "_FATFS_FILE_SEEK_PTR is "XTSTR(_FATFS_FILE_SEEK_PTR)
      |         ^~~~~~~
/home/[email protected]//ext/RIOT/sys/include/vfs.h:150:9: note: '#pragma message: _FATFS_FILE_EXFAT is (44)'
  150 | #pragma message "_FATFS_FILE_EXFAT is "XTSTR(_FATFS_FILE_EXFAT)
      |         ^~~~~~~
In file included from /home/[email protected]//ext/RIOT/pkg/fatfs/fatfs_vfs/fatfs_vfs.c:21:
/home/[email protected]//ext/RIOT/pkg/fatfs/fatfs_vfs/fatfs_vfs.c: In function '_mount':
/home/[email protected]//ext/RIOT/core/lib/include/assert.h:148:28: error: static assertion failed: "fatfs_file_desc_t must fit into VFS_FILE_BUFFER_SIZE"
  148 | #define static_assert(...) _Static_assert(__VA_ARGS__)
      |                            ^~~~~~~~~~~~~~
/home/[email protected]//ext/RIOT/pkg/fatfs/fatfs_vfs/fatfs_vfs.c:123:5: note: in expansion of macro 'static_assert'
  123 |     static_assert(VFS_FILE_BUFFER_SIZE >= sizeof(fatfs_file_desc_t),

Issues/PRs references

#20297

[riot-ci]

Murdock results

❌ FAILED

bf4bceb vfs: increase _FATFS_FILE_EXFAT

Success	Failures	Total	Runtime
2713	0	9351	03m:27s

Artifacts

[benpicco]

Ah I think I know what's going on - the struct is of course going to be padded.
How about instead

--- a/sys/include/vfs.h
+++ b/sys/include/vfs.h
@@ -66,9 +66,11 @@
 #include <sys/types.h> /* for off_t etc. */
 #include <sys/statvfs.h> /* for struct statvfs */
 
+#include "architecture.h"
 #include "sched.h"
 #include "clist.h"
 #include "iolist.h"
+#include "macros/math.h"
 #include "mtd.h"
 #include "xfa.h"
 
@@ -279,12 +281,12 @@ extern "C" {
  * @attention Put the check in the public header file (.h), do not put the check in the
  * implementation (.c) file.
  */
-#define VFS_FILE_BUFFER_SIZE MAX5(FATFS_VFS_FILE_BUFFER_SIZE,    \
+#define VFS_FILE_BUFFER_SIZE MATH_ALIGN(MAX5(FATFS_VFS_FILE_BUFFER_SIZE, \
                                   LITTLEFS_VFS_FILE_BUFFER_SIZE, \
                                   LITTLEFS2_VFS_FILE_BUFFER_SIZE,\
                                   SPIFFS_VFS_FILE_BUFFER_SIZE,   \
                                   LWEXT4_VFS_FILE_BUFFER_SIZE    \
-                                 )
+                                 ), ARCHITECTURE_WORD_BYTES)
 #endif
 
 #ifndef VFS_NAME_MAX

[fabian18]

That's good yes! It is the right direction, but for CFLAGS += -DVFS_NAME_MAX=48 it fails and for 49 it is passing again.

[fabian18]

Maybe MATH_ALIGN(1 + MAX5(...))

[benpicco]

Weird, does this only happen with exFAT support enabled? Then it might be that the 4 bytes need to be added to _FATFS_FILE_EXFAT

[fabian18]

Yes, with CFLAGS += -DFATFS_FFCONF_OPT_FS_EXFAT=0 it is passing with your provided patch. (Tested up to 52).
That means I will have to try again with _FATFS_FILE_EXFAT = 48? + your patch

[fabian18]

And VFS_DIR_BUFFER_SIZE also needs the alignment I suppose

[mguetschow]

@benpicco @fabian18 what's the status here after #20542 had been merged for the release?