made `readdir` stateful by jounathaen · Pull Request #1738 · hermit-os/kernel

jounathaen · 2025-06-03T17:02:46Z

This fixes #1589 by saving the read state in the backends of the sys_getdents64 syscall.

Currently, implemented only for RAMFS and FUSE - mostly because I couldn't find an example on how to use the ROMFS.

In general, I'm not a huge fan of this, as we basically reimplement the old POSIX interface instead of designing a better interface ourselves, but this avoids complications and breaking changes in the standard libraries (Rust and Newlib).

Notable Quirks/Issues:

We don't have Inode numbers for RAMFS. All entries have Inode nr. 1. This is unlikely to be a problem because I can't imagine an application utilizing Inodes being seriously executed in an unikernel.
As the RAMFS is currently based on paths, we can't tell the application which file type a direntry has. This is also unlikely to cause a problem, as Linux has introduced this field "recently" (21 years ago) and states that "All applications must properly handle a return of DT_UNKNOWN." (https://linux.die.net/man/2/getdents64)
This would be a great use-case for a unit test, but it is quite tricky to get file system related things working in a unit test. This is because the file system relies on async code, but the async executor also tries to execute other maintenance tasks which are not present in a unit-test environment.

mkroening

Cool! I have a few comments. :D

mkroening · 2025-06-16T10:39:13Z

 }
+impl core::fmt::Debug for Dirent64 {
+	fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+		let cstr = unsafe { CStr::from_ptr((&raw const self.d_name).cast::<c_char>()) };


This is not sound, since the API allows creating the struct without a filename. This would then read out of bounds.

True. I changed it into an unsafe fn in case it will be needed for debugging purposes later. But we can also just throw it out.

We could do it with a debug function similar to this:

kernel/src/arch/x86_64/mm/paging.rs

Lines 398 to 425 in b15388d

impl OffsetPageTableExt for OffsetPageTable<'_> {

fn display(&self) -> MappedPageTableDisplay<'_, PhysOffset> {

MappedPageTableDisplay {

inner: mapped_page_table_iter::offset_page_table_range_iter(self),

}

}

}

pub struct MappedPageTableDisplay<'a, P: PageTableFrameMapping + Clone> {

inner: MappedPageTableRangeInclusiveIter<'a, P>,

}

impl<P: PageTableFrameMapping + Clone> fmt::Display for MappedPageTableDisplay<'_, P> {

fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {

let mut has_fields = false;

for mapped_page_range in self.inner.clone() {

if has_fields {

f.write_char('\n')?;

}

write!(f, "{}", mapped_page_range.display())?;

has_fields = true;

}

Ok(())

}

}

Done!
Still debatable if we want to keep it or not.

github-actions

Benchmark Results

Details

Benchmark	Current: `4d17269`	Previous: `4a84d25`	Performance Ratio
startup_benchmark Build Time	`94.25` s	`87.39` s	`1.08`
startup_benchmark File Size	`0.88` MB	`0.88` MB	`1.00`
Startup Time - 1 core	`0.99` s (`±0.02` s)	`0.93` s (`±0.02` s)	`1.07`
Startup Time - 2 cores	`0.98` s (`±0.02` s)	`0.93` s (`±0.01` s)	`1.05`
Startup Time - 4 cores	`0.98` s (`±0.02` s)	`0.93` s (`±0.01` s)	`1.05`
multithreaded_benchmark Build Time	`90.95` s	`86.04` s	`1.06`
multithreaded_benchmark File Size	`0.98` MB	`0.98` MB	`1.00`
Multithreaded Pi Efficiency - 2 Threads	`86.61` % (`±8.81` %)	`88.29` % (`±8.59` %)	`0.98`
Multithreaded Pi Efficiency - 4 Threads	`43.48` % (`±3.29` %)	`44.10` % (`±3.40` %)	`0.99`
Multithreaded Pi Efficiency - 8 Threads	`25.16` % (`±2.00` %)	`25.37` % (`±1.74` %)	`0.99`
micro_benchmarks Build Time	`81.13` s	`85.38` s	`0.95`
micro_benchmarks File Size	`0.99` MB	`0.98` MB	`1.00`
Scheduling time - 1 thread	`55.07` ticks (`±0.95` ticks)	`59.70` ticks (`±2.64` ticks)	`0.92`
Scheduling time - 2 threads	`32.50` ticks (`±5.25` ticks)	`35.16` ticks (`±4.53` ticks)	`0.92`
Micro - Time for syscall (getpid)	`13.55` ticks (`±1.57` ticks)	`14.31` ticks (`±1.67` ticks)	`0.95`
Memcpy speed - (built_in) block size 4096	`89659.19` MByte/s (`±61686.48` MByte/s)	`83116.66` MByte/s (`±57474.29` MByte/s)	`1.08`
Memcpy speed - (built_in) block size 1048576	`44165.62` MByte/s (`±30520.96` MByte/s)	`43185.37` MByte/s (`±29862.04` MByte/s)	`1.02`
Memcpy speed - (built_in) block size 16777216	`30028.70` MByte/s (`±24650.53` MByte/s)	`29129.50` MByte/s (`±23947.84` MByte/s)	`1.03`
Memset speed - (built_in) block size 4096	`90056.91` MByte/s (`±61923.87` MByte/s)	`83351.99` MByte/s (`±57637.36` MByte/s)	`1.08`
Memset speed - (built_in) block size 1048576	`44379.81` MByte/s (`±30665.69` MByte/s)	`43401.64` MByte/s (`±30010.43` MByte/s)	`1.02`
Memset speed - (built_in) block size 16777216	`30801.39` MByte/s (`±25094.75` MByte/s)	`29891.52` MByte/s (`±24393.18` MByte/s)	`1.03`
Memcpy speed - (rust) block size 4096	`79417.07` MByte/s (`±55203.81` MByte/s)	`72228.32` MByte/s (`±50333.11` MByte/s)	`1.10`
Memcpy speed - (rust) block size 1048576	`44152.04` MByte/s (`±30519.87` MByte/s)	`42874.06` MByte/s (`±29664.74` MByte/s)	`1.03`
Memcpy speed - (rust) block size 16777216	`30062.54` MByte/s (`±24687.43` MByte/s)	`29267.80` MByte/s (`±24080.22` MByte/s)	`1.03`
Memset speed - (rust) block size 4096	`79884.28` MByte/s (`±55542.61` MByte/s)	`72617.93` MByte/s (`±50623.99` MByte/s)	`1.10`
Memset speed - (rust) block size 1048576	`44364.92` MByte/s (`±30664.74` MByte/s)	`43100.09` MByte/s (`±29821.73` MByte/s)	`1.03`
Memset speed - (rust) block size 16777216	`30825.56` MByte/s (`±25120.51` MByte/s)	`30032.03` MByte/s (`±24522.76` MByte/s)	`1.03`
alloc_benchmarks Build Time	`79.12` s	`83.17` s	`0.95`
alloc_benchmarks File Size	`0.95` MB	`0.94` MB	`1.00`
Allocations - Allocation success	`100.00` %	`100.00` %	`1`
Allocations - Deallocation success	`69.99` % (`±0.26` %)	`69.97` % (`±0.24` %)	`1.00`
Allocations - Pre-fail Allocations	`100.00` %	`100.00` %	`1`
Allocations - Average Allocation time	`9359.41` Ticks (`±158.52` Ticks)	`10122.44` Ticks (`±753.43` Ticks)	`0.92`
Allocations - Average Allocation time (no fail)	`9359.41` Ticks (`±158.52` Ticks)	`10122.44` Ticks (`±753.43` Ticks)	`0.92`
Allocations - Average Deallocation time	`615.25` Ticks (`±9.24` Ticks)	`670.17` Ticks (`±33.53` Ticks)	`0.92`
mutex_benchmark Build Time	`78.77` s	`84.34` s	`0.93`
mutex_benchmark File Size	`0.99` MB	`0.98` MB	`1.00`
Mutex Stress Test Average Time per Iteration - 1 Threads	`11.16` ns (`±0.37` ns)	`12.00` ns (`±0.49` ns)	`0.93`
Mutex Stress Test Average Time per Iteration - 2 Threads	`12.98` ns (`±0.55` ns)	`13.44` ns (`±1.10` ns)	`0.97`

This comment was automatically generated by workflow using github-action-benchmark.

github-actions

Benchmark Results

_{^{This comment was automatically generated by github-action-benchmark.}}

Misc

Benchmark	Current: `ca2e818`	Previous: `260c073`	Performance Ratio
micro_benchmarks Build Time	`88.47` s	`77.28` s	`1.14`
micro_benchmarks File Size	`0.97` MB	`0.96` MB	`1.00`
Scheduling time - 1 thread	`65.10` ticks (`3.32`)	`65.78` ticks (`3.09`)	`0.99`
Scheduling time - 2 threads	`34.13` ticks (`1.99`)	`34.06` ticks (`1.60`)	`1.00`
Micro - Time for syscall (getpid)	`15.36` ticks (`1.23`)	`15.63` ticks (`1.12`)	`0.98`
Memcpy speed - (built_in) block size 4096	`74438.86` MByte/s (`51551.07`)	`74071.55` MByte/s (`51298.47`)	`1.00`
Memcpy speed - (built_in) block size 1048576	`42010.17` MByte/s (`29140.51`)	`41671.22` MByte/s (`28953.97`)	`1.01`
Memcpy speed - (built_in) block size 16777216	`27178.02` MByte/s (`22510.03`)	`28183.83` MByte/s (`23127.34`)	`0.96`
Memset speed - (built_in) block size 4096	`74304.93` MByte/s (`51461.59`)	`73377.06` MByte/s (`50853.04`)	`1.01`
Memset speed - (built_in) block size 1048576	`42249.71` MByte/s (`29306.15`)	`41909.14` MByte/s (`29114.60`)	`1.01`
Memset speed - (built_in) block size 16777216	`27863.38` MByte/s (`22898.57`)	`28925.26` MByte/s (`23564.17`)	`0.96`
Memcpy speed - (rust) block size 4096	`65423.46` MByte/s (`45770.23`)	`66148.09` MByte/s (`46097.41`)	`0.99`
Memcpy speed - (rust) block size 1048576	`41854.18` MByte/s (`29046.84`)	`41089.86` MByte/s (`28495.44`)	`1.02`
Memcpy speed - (rust) block size 16777216	`27945.12` MByte/s (`23115.64`)	`28178.78` MByte/s (`23125.08`)	`0.99`
Memset speed - (rust) block size 4096	`66325.61` MByte/s (`46387.78`)	`66625.45` MByte/s (`46410.51`)	`1.00`
Memset speed - (rust) block size 1048576	`42031.00` MByte/s (`29170.08`)	`41334.74` MByte/s (`28668.15`)	`1.02`
Memset speed - (rust) block size 16777216	`28690.87` MByte/s (`23552.65`)	`28886.49` MByte/s (`23531.12`)	`0.99`
alloc_benchmarks Build Time	`87.81` s	`73.24` s	`1.20`
alloc_benchmarks File Size	`0.92` MB	`0.92` MB	`1.00`
Allocations - Allocation success	`100.00` %	`100.00` %	`1`
Allocations - Deallocation success	`70.03` % (`0.28`)	`70.07` % (`0.26`)	`1.00`
Allocations - Pre-fail Allocations	`100.00` %	`100.00` %	`1`
Allocations - Average Allocation time	`13108.29` Ticks (`294.68`)	`10973.10` Ticks (`133.18`)	`1.19`
Allocations - Average Allocation time (no fail)	`13108.29` Ticks (`294.68`)	`10973.10` Ticks (`133.18`)	`1.19`
Allocations - Average Deallocation time	`810.79` Ticks (`82.54`)	`804.59` Ticks (`13.39`)	`1.01`
mutex_benchmark Build Time	`87.90` s	`72.57` s	`1.21`
mutex_benchmark File Size	`0.97` MB	`0.96` MB	`1.00`
Mutex Stress Test Average Time per Iteration - 1 Threads	`13.90` ns (`0.57`)	`14.02` ns (`0.55`)	`0.99`
Mutex Stress Test Average Time per Iteration - 2 Threads	`27.42` ns (`20.81`)	`16.30` ns (`1.12`)	`1.68`

github-actions

Benchmark Results

_{^{This comment was automatically generated by github-action-benchmark.}}

General

Benchmark	Current: `ca2e818`	Previous: `260c073`	Performance Ratio
startup_benchmark Build Time	`69.17` s	`76.85` s	`0.90`
startup_benchmark File Size	`0.85` MB	`0.85` MB	`1.00`
Startup Time - 1 core	`0.92` s (`0.04`)	`0.99` s (`0.03`)	`0.93`
Startup Time - 2 cores	`0.93` s (`0.03`)	`1.00` s (`0.03`)	`0.93`
Startup Time - 4 cores	`0.92` s (`0.03`)	`1.02` s (`0.04`)	`0.91`
multithreaded_benchmark Build Time	`68.21` s	`73.93` s	`0.92`
multithreaded_benchmark File Size	`0.96` MB	`0.96` MB	`1.00`
Multithreaded Pi Efficiency - 2 Threads	`92.02` % (`11.60`)	`91.24` % (`10.29`)	`1.01`
Multithreaded Pi Efficiency - 4 Threads	`62.54` % (`9.19`)	`62.31` % (`8.62`)	`1.00`
Multithreaded Pi Efficiency - 8 Threads	`43.36` % (`6.04`)	`43.82` % (`5.66`)	`0.99`

Co-authored-by: Martin Kröning <martin.kroening@eonerc.rwth-aachen.de>

mkroening

Thanks! :)

Hermit: fix `readdir()` On Hermit, the `sys_getdents` implementation was flawed, as it wasn't stateful. So the caller had to provide a buffer large enough to hold all `dirents` of the directory in question. Else it returns `EINVAL`. The current workaround used in Rust is to grow the buffer until it is sufficiently large. hermit-os/kernel#1738 fixes the behavior of `sys_getdents`. Hermit now keeps track of the directory position, and each new call to the `sys_getdents` provides the next entries. But this results in the current implementation in Rust Std being flawed: Because Hermit now returns a proper _readcount_, `read_dir` will only ever iterate over the first 512 bytes of directory entries, even if not all `dirent64`s are read. This PR fixes this with a proper implementation of the `getdents64` algorithm. Remark: This breaks compatibility with Hermit versions 0.11 and previous. We have discussed this in the Hermit team and have come to the conclusion that this is ok. We have released two newer "major" versions since, and Hermit is still in the development phase where we don't guarantee any stability and backwards compatibility in the kernel itself.

Rollup merge of #158649 - hermit-os:hermit-readdir, r=joboet Hermit: fix `readdir()` On Hermit, the `sys_getdents` implementation was flawed, as it wasn't stateful. So the caller had to provide a buffer large enough to hold all `dirents` of the directory in question. Else it returns `EINVAL`. The current workaround used in Rust is to grow the buffer until it is sufficiently large. hermit-os/kernel#1738 fixes the behavior of `sys_getdents`. Hermit now keeps track of the directory position, and each new call to the `sys_getdents` provides the next entries. But this results in the current implementation in Rust Std being flawed: Because Hermit now returns a proper _readcount_, `read_dir` will only ever iterate over the first 512 bytes of directory entries, even if not all `dirent64`s are read. This PR fixes this with a proper implementation of the `getdents64` algorithm. Remark: This breaks compatibility with Hermit versions 0.11 and previous. We have discussed this in the Hermit team and have come to the conclusion that this is ok. We have released two newer "major" versions since, and Hermit is still in the development phase where we don't guarantee any stability and backwards compatibility in the kernel itself.

jounathaen changed the title ~~Readdir~~ made readdir stateful Jun 3, 2025

jounathaen force-pushed the readdir branch 2 times, most recently from 0d0a2d1 to 691484e Compare June 3, 2025 17:15

jounathaen marked this pull request as ready for review June 3, 2025 17:16

mkroening self-requested a review June 3, 2025 17:17

mkroening self-assigned this Jun 3, 2025

jounathaen force-pushed the readdir branch from 691484e to 95b9950 Compare June 6, 2025 13:34

jounathaen force-pushed the readdir branch from 95b9950 to 2cdd832 Compare June 13, 2025 14:53

mkroening requested changes Jun 16, 2025

View reviewed changes

mkroening mentioned this pull request Jun 25, 2025

feat(fs): add readdir test hermit-os/hermit-c#18

Merged

jounathaen force-pushed the readdir branch 6 times, most recently from 2fe6b59 to ca2e818 Compare July 4, 2025 13:30

mkroening self-requested a review July 4, 2025 13:31

github-actions Bot reviewed Jul 4, 2025

View reviewed changes

Fuse readdir: Changed manual min functionality to min fn

3236c32

jounathaen force-pushed the readdir branch 2 times, most recently from 16fba36 to 73946da Compare July 17, 2025 12:39

made getdents64 stateful

4d17269

Co-authored-by: Martin Kröning <martin.kroening@eonerc.rwth-aachen.de>

jounathaen force-pushed the readdir branch from 73946da to 4d17269 Compare July 17, 2025 12:41

mkroening approved these changes Jul 21, 2025

View reviewed changes

mkroening added this pull request to the merge queue Jul 21, 2025

Merged via the queue into main with commit e77df50 Jul 21, 2025
15 checks passed

mkroening deleted the readdir branch August 29, 2025 14:17

This was referenced Jun 26, 2026

Hermit: Fix readdir for hermit >= v0.12 hermit-os/rust#8

Closed

Hermit: fix readdir() rust-lang/rust#158649

Merged

	impl OffsetPageTableExt for OffsetPageTable<'_> {
	fn display(&self) -> MappedPageTableDisplay<'_, PhysOffset> {
	MappedPageTableDisplay {
	inner: mapped_page_table_iter::offset_page_table_range_iter(self),
	}
	}
	}

	pub struct MappedPageTableDisplay<'a, P: PageTableFrameMapping + Clone> {
	inner: MappedPageTableRangeInclusiveIter<'a, P>,
	}

	impl<P: PageTableFrameMapping + Clone> fmt::Display for MappedPageTableDisplay<'_, P> {
	fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
	let mut has_fields = false;

	for mapped_page_range in self.inner.clone() {
	if has_fields {
	f.write_char('\n')?;
	}
	write!(f, "{}", mapped_page_range.display())?;

	has_fields = true;
	}

	Ok(())
	}
	}

Uh oh!

Conversation

jounathaen commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mkroening left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mkroening Jun 16, 2025

Choose a reason for hiding this comment

Uh oh!

jounathaen Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

mkroening Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jounathaen Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Benchmark Results

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Benchmark Results

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Benchmark Results

Uh oh!

mkroening left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jounathaen commented Jun 3, 2025 •

edited

Loading

mkroening Jul 8, 2025 •

edited

Loading

github-actions Bot left a comment •

edited

Loading