KolibriOS/kolibrios
17
27
Fork 69
Code Issues 157 Pull Requests 48 Actions Projects 2 Releases Wiki Activity

fs/ext: Implement features 64-bit, metadata_csum, and metadata_csum_seed #506

Merged
dunkaist merged 8 commits from Matou1306/kolibrios:csum into main 2026-07-17 10:25:15 +00:00
Conversation 7 Commits 8 Files Changed 1
+442 -95
Matou1306 commented 2026-06-20 09:47:46 +00:00
Contributor
Copy Link
Copy Source

What is implemented: Reading support for all ext4 filesystems built with default features.
Implemented reading filesystems with 64 bit feature for partitions < 16TB (for standard 4kb block size)
Mount filesystems with metadata_csum / metadata_csum_seed and check csum, in case it doesn't match the mount fails.
Created new umka tests 75 and 77 and updated test 61:
t075: tests reading filesystems with 64-bit feature.
t077: tests mounting and validating checksums for partition with csum / csum and csum_seed enabled.
t061/ref.log: (tests reading ext4 directories) updated to show valid output instead of unknown_fs

What is implemented: Reading support for all ext4 filesystems built with default features. Implemented reading filesystems with 64 bit feature for partitions < 16TB (for standard 4kb block size) Mount filesystems with metadata_csum / metadata_csum_seed and check csum, in case it doesn't match the mount fails. Created new umka tests 75 and 77 and updated test 61: t075: tests reading filesystems with 64-bit feature. t077: tests mounting and validating checksums for partition with csum / csum and csum_seed enabled. t061/ref.log: (tests reading ext4 directories) updated to show valid output instead of `unknown_fs`
Matou1306 commented 2026-06-20 09:52:26 +00:00
Author
Contributor
Copy Link
Copy Source

WIP:

  • Write support for those 3 features.
  • Supporting file systems larger than 16tb.

These might take a while (especially the second one) so I would suggest we make sure this works first.

WIP: - Write support for those 3 features. - Supporting file systems larger than 16tb. These might take a while (especially the second one) so I would suggest we make sure this works first.
Matou1306 force-pushed csum from 710b9b077b to a3085caee7 2026-07-03 18:41:07 +00:00 Compare
Matou1306 force-pushed csum from a3085caee7 to 89dd4d157a 2026-07-04 00:56:19 +00:00 Compare
Matou1306 force-pushed csum from 89dd4d157a to c9982366ec 2026-07-04 10:31:48 +00:00 Compare
Matou1306 marked the pull request as ready for review 2026-07-04 10:32:55 +00:00
Matou1306 commented 2026-07-04 11:10:28 +00:00
Author
Contributor
Copy Link
Copy Source

Update: Write support for the 3 features should be done, I removed the WIP label. As for the 16TB support, I added a check that refuses to write to partitions having high block fields. This will read the first 16TB fine given extents are not enabled. Do you suggest we refuse mounting these huge partitions altogether, or add a check for extents that will refuse to mount if they are enabled, and mount read-only to correctly read the first 16TB if they're not?
@dunkaist @Doczom @hidnplayr
@Burer I will take a look at your comments on the other PR, sorry for the delay, was focused on this one for the past days.

Update: Write support for the 3 features should be done, I removed the WIP label. As for the 16TB support, I added a check that refuses to write to partitions having high block fields. This will read the first 16TB fine given extents are not enabled. Do you suggest we refuse mounting these huge partitions altogether, or add a check for extents that will refuse to mount if they are enabled, and mount read-only to correctly read the first 16TB if they're not? @dunkaist @Doczom @hidnplayr @Burer I will take a look at your comments on the other PR, sorry for the delay, was focused on this one for the past days.
❤️ 1
Matou1306 force-pushed csum from c9982366ec to 36621636de 2026-07-05 18:34:48 +00:00 Compare
dunkaist requested changes 2026-07-11 10:52:14 +00:00
Dismissed
kernel/trunk/fs/ext.inc Outdated
@@ -558,4 +733,0 @@
dec [ebx+BGDESCR.inodesFree]
dec [ebp+EXTFS.superblock.inodesFree]
push [ebx+BGDESCR.inodeBitmap]
call extfsWriteDescriptor
dunkaist commented 2026-07-11 00:04:41 +00:00
Owner
Copy Link
Copy Source

Try to replace this

        push    [ebx+BGDESCR.inodeBitmap]
	call    extfsWriteDescriptor

with this
stdcall extfsWriteDescriptor, [ebx+BGDESCR.inodeBitmap]

Try to replace this ``` push [ebx+BGDESCR.inodeBitmap] call extfsWriteDescriptor ``` with this ` stdcall extfsWriteDescriptor, [ebx+BGDESCR.inodeBitmap]`
kernel/trunk/fs/ext.inc Outdated
@@ -381,3 +462,4 @@
.read_only:
or [ebp+EXTFS.mountType], READ_ONLY
@@:
mov ax, 5
dunkaist commented 2026-07-10 22:57:08 +00:00
Owner
Copy Link
Copy Source

Please, replace magic numbers with macros. Here and in other places.

Please, replace magic numbers with macros. Here and in other places.
kernel/trunk/fs/ext.inc Outdated
@@ -384,0 +470,4 @@
mov [ebp+EXTFS.descShift], al
xor edx, edx
bts edx, eax
mov [ebp+EXTFS.descSize], edx
dunkaist commented 2026-07-10 23:42:23 +00:00
Owner
Copy Link
Copy Source

I think the code above can be written a bit shorter and with less bit manipulation instructions, approximately like this:

movi    eax, 32
test    blah
jz      @f
movzx   eax, [SUPERBLOCK.descSize]
@@:
mov     [ebp+EXTFS.descSize], eax
bsf     eax, eax
mov     [ebp+EXTFS.descShift], eax

By the way, is SUPERBLOCK.descSize always a power of two?

I think the code above can be written a bit shorter and with less bit manipulation instructions, approximately like this: ``` movi eax, 32 test blah jz @f movzx eax, [SUPERBLOCK.descSize] @@: mov [ebp+EXTFS.descSize], eax bsf eax, eax mov [ebp+EXTFS.descShift], eax ``` By the way, is SUPERBLOCK.descSize always a power of two?
kernel/trunk/fs/ext.inc Outdated
@@ -387,3 +477,3 @@
div [ebx+SUPERBLOCK.inodesPerGroup]
inc eax
shl eax, 5
mov cl, [ebp+EXTFS.descShift]
dunkaist commented 2026-07-10 23:46:46 +00:00
Owner
Copy Link
Copy Source

movzx ecx, [...] is better than mov cl, [...]
https://stackoverflow.com/questions/47052342/understanding-partial-register-slowdowns-from-mov-instead-of-movzx-instruction

movzx ecx, [...] is better than mov cl, [...] https://stackoverflow.com/questions/47052342/understanding-partial-register-slowdowns-from-mov-instead-of-movzx-instruction
❤️ 1
kernel/trunk/fs/ext.inc Outdated
@@ -430,0 +527,4 @@
jz .success
; Verify superblock checksum
mov eax, -1
dunkaist commented 2026-07-10 23:52:32 +00:00
Owner
Copy Link
Copy Source

Use movi instead of mov

Use movi instead of mov
kernel/trunk/fs/ext.inc Outdated
@@ -430,0 +528,4 @@
; Verify superblock checksum
mov eax, -1
stdcall crc_32, 0x82F63B78, ebx, 1020
dunkaist commented 2026-07-10 23:52:11 +00:00
Owner
Copy Link
Copy Source

Magic numbers

Magic numbers
kernel/trunk/fs/ext.inc Outdated
@@ -456,17 +583,54 @@ extfsReadBlock:
@@:
ret
macro calc_bitmap_csum field, count {
dunkaist commented 2026-07-10 23:55:20 +00:00
Owner
Copy Link
Copy Source

Please, make this a function

Please, make this a function
👍 1
kernel/trunk/fs/ext.inc Outdated
@@ -461,0 +611,4 @@
sub eax, [ebp+EXTFS.descriptorTable]
mov ecx, [ebp+EXTFS.descSize]
xor edx, edx
div ecx
dunkaist commented 2026-07-10 23:57:25 +00:00
Owner
Copy Link
Copy Source

When you calculated descShift above, you used BSF instruction. This implies descSize is a power of two. If it is a power of two, then you can use a shift instruction instead of this DIV.

When you calculated descShift above, you used BSF instruction. This implies descSize is a power of two. If it is a power of two, then you can use a shift instruction instead of this DIV.
kernel/trunk/fs/ext.inc Outdated
@@ -467,3 +630,3 @@
shl ebx, 9
add ebx, [ebp+EXTFS.descriptorTable]
call fs_write32_sys
mov ecx, 1
dunkaist commented 2026-07-10 23:59:28 +00:00
Owner
Copy Link
Copy Source

You can use the movi macro when a number fits into a signed one-byte value. In that case movi is a bit shorter than mov.

You can use the movi macro when a number fits into a signed one-byte value. In that case movi is a bit shorter than mov.
kernel/trunk/fs/ext.inc Outdated
@@ -488,2 +656,2 @@
add [ebx+BGDESCR.blocksFree], cx
add [ebp+EXTFS.superblock.blocksFree], ecx
add [ebx+BGDESCR.blocksFree_lo], cx
add [ebp+EXTFS.superblock.blocksFree_lo], ecx
dunkaist commented 2026-07-11 00:01:29 +00:00
Owner
Copy Link
Copy Source

What about .blocksFree_hi?

What about .blocksFree_hi?
Matou1306 commented 2026-07-11 18:45:57 +00:00
Author
Contributor
Copy Link
Copy Source

Those are part of the 16 Tb issue, the whole driver ignores them for now and it's the same the reason we refuse mounting partitions with hi block fields.

Those are part of the 16 Tb issue, the whole driver ignores them for now and it's the same the reason we refuse mounting partitions with hi block fields.
kernel/trunk/fs/ext.inc Outdated
@@ -1576,6 +1818,35 @@ extfsTruncateFile:
.ret:
ret
macro calc_dirblock_csum dir_ino_reg {
dunkaist commented 2026-07-11 00:07:14 +00:00
Owner
Copy Link
Copy Source

Convert this into a function, please

Convert this into a function, please
👍 1
Matou1306 force-pushed csum from 6c6686a916 to 454540724c 2026-07-12 17:54:22 +00:00 Compare
Matou1306 commented 2026-07-12 19:26:27 +00:00
Author
Contributor
Copy Link
Copy Source

@dunkaist All changes should be there now, regarding the macros I replaced them with functions with labels instead of proc because ebp is being constantly used if that's ok?
Also I kept one macro load_bgd_64, this one is currently only used once but it will be very helpful in the future when 64 bit math gets added.
Please let me know about anything else

@dunkaist All changes should be there now, regarding the macros I replaced them with functions with labels instead of `proc` because ebp is being constantly used if that's ok? Also I kept one macro `load_bgd_64`, this one is currently only used once but it will be very helpful in the future when 64 bit math gets added. Please let me know about anything else
dunkaist approved these changes 2026-07-15 13:37:29 +00:00
Matou1306 requested review from Burer 2026-07-15 13:48:06 +00:00
Matou1306 requested review from Doczom 2026-07-15 13:48:06 +00:00
Doczom approved these changes 2026-07-15 19:06:20 +00:00
Dismissed
kernel/trunk/fs/ext.inc Outdated
@@ -580,2 +784,4 @@
add eax, edi
mov ecx, eax
test [ebp+EXTFS.superblock.RO_compatibleFlags], 0x0410
Doczom commented 2026-07-15 16:07:32 +00:00
Owner
Copy Link
Copy Source

is there a way to replace this set of flags with constants?

is there a way to replace this set of flags with constants?
Matou1306 commented 2026-07-15 19:30:13 +00:00
Author
Contributor
Copy Link
Copy Source

@Doczom I could do

CSUM_FLAGS equ INCOMPAT_CSUM_SEED or RO_COMPAT_METADATA_CSUM
test [ebp+EXTFS.superblock.RO_compatibleFlags], CSUM_FLAGS

Please let me know if that's ok.

@Doczom I could do ``` CSUM_FLAGS equ INCOMPAT_CSUM_SEED or RO_COMPAT_METADATA_CSUM test [ebp+EXTFS.superblock.RO_compatibleFlags], CSUM_FLAGS ``` Please let me know if that's ok.
👍 1
Doczom marked this conversation as resolved
Matou1306 force-pushed csum from 795bb16b99 to 0700903437 2026-07-16 22:28:24 +00:00 Compare
Doczom requested review from Doczom 2026-07-16 22:46:02 +00:00
Doczom approved these changes 2026-07-16 22:47:01 +00:00
dunkaist added 8 commits 2026-07-17 10:16:28 +00:00
ext4: 64-bit:add logic to use new 64 bit values for navigation ca5a995841
Update helper functions to support 64-bit 7e1882450f
Hotfix: Support all reading for partitions under 16tb ee9102b141
mount filesystems with incompat_metadata_csum_seed in ro mode cf3993e203 
cache number of directory elements to avoid counting them multiple times for the same directory
read all standard ext4 filesystems and check csum 55ed97f909
metadata_csum + seed write support 0adc2b08a5
Magic numbers + Optimizations ecc5813d40
replace macros with functions
Check kernel codestyle / Check kernel codestyle (pull_request) Successful in 23s
Details
Test PR / Build (ru_RU) (pull_request) Successful in 2m36s
Details
Test PR / Build (es_ES) (pull_request) Successful in 2m39s
Details
Test PR / Build (en_US) (pull_request) Successful in 2m41s
Details
9ae27bdffb 
optimize descshift/descsize calculations

use movi instead of mov where possible
dunkaist force-pushed csum from 0700903437 to 9ae27bdffb 2026-07-17 10:16:28 +00:00 Compare
dunkaist merged commit 42b2a07033 into main 2026-07-17 10:25:15 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
dunkaist
Burer
Doczom
Labels
Clear labels
Influence/Text/TYPO
AI
Eolite
FS
GSoC
This issue or PR in the Google Source of Code program
 Good First PR
The issue is suitable to beginners
 HLL
HardwareTested
IRCC
Influence/Settings
Lang/C
Lang/FASM
Pay for the code
Paid task
 Subsystem/API
Subsystem/Audio
infinity service, audio drivers, midi, speacker, audio programs
 Subsystem/Graphics
vesa, vga, framebuffer, cursors, blitter, and video drivers
 Subsystem/IPC and events
pipes, signals, events, shared memory
 Subsystem/Memory
virt and phys memory allocators, malloc and other
 Subsystem/Network
Subsystem/Services(daemon)
userspace and kernel(for example: serial) services
 Subsystem/Taskmanager
process, threads, run apps, scheduler
 Subsystem/VFS
drivers from filesystem, fs api, blkdev, programs that work with the file system
 Subsystem/Window
windows, skins, buttons, mouse and keyboard code for windows (not the base code)
Category
Applications
Category
Drivers
Category
General
Category
Kernel
Category
Libraries
Kind
Breaking
Breaking change that won't be backward compatible
Kind
Bug
Something is not working
Kind
Build
Kind
Documentation
Documentation changes
Kind
Enhancement
Improve existing functionality
Kind
Feature
New functionality
Kind
Security
This is security issue
Kind
Testing
Issue or pull request related to testing
PR
Ready to merge
Pull request is ready for merge
PR
Conflicts
PR conflicts with main
PR
Dependent
This PR is dependent on another PR
PR
Request changes
Changes requested in pull request
PR
Review required
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Medium
3
The priority is medium
Priority
Low
4
The priority is low
Reviewed
Confirmed
Issue has been confirmed
Reviewed
Duplicate
This issue or pull request already exists
Reviewed
Invalid
Invalid issue
Reviewed
Won't Fix
This issue won't be fixed
Status
Abandoned
Somebody has started to work on this but abandoned work
Status
Blocked
Something is blocking this issue or pull request
Status
Need More Info
Feedback is required to reproduce issue or to continue work
No labels Subsystem/VFS
Category
Kernel
Kind
Feature
PR
Review required
Priority
Medium
3
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
ace-dent (Andrew) b00bl1k (Alexey Ryabov) bad_Dr3dd0x Burer Doczom (Mikhail Frolov) dunkaist (Ivan B) Egor00f (Егор Ярин) hidnplayr IgorA Leency (Kiril Lipatov) mxlgv (Max Logaev) ramenu rgimad sdongles (Serhii Sakhno) Sweetbread (Gleb Zaharov)
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: KolibriOS/kolibrios#506