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! :)

jounathaen changed the title ~~Readdir~~ 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

Open

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

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

made `readdir` stateful#1738

made `readdir` stateful#1738
mkroening merged 2 commits into
mainfrom
readdir

jounathaen commented Jun 3, 2025 •

edited

Loading

mkroening left a comment

Uh oh!

mkroening Jun 16, 2025

jounathaen Jul 2, 2025

mkroening Jul 8, 2025 •

edited

Loading

jounathaen Jul 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot left a comment •

edited

Loading

github-actions Bot left a comment

github-actions Bot left a comment

mkroening left a comment

Uh oh!

Labels

2 participants

	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!

mkroening left a comment

Choose a reason for hiding this comment

Uh oh!

mkroening Jun 16, 2025

Choose a reason for hiding this comment

jounathaen Jul 2, 2025

Choose a reason for hiding this comment

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

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!

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

github-actions Bot left a comment

Choose a reason for hiding this comment

Benchmark Results

github-actions Bot left a comment

Choose a reason for hiding this comment

Benchmark Results

mkroening left a comment

Choose a reason for hiding this comment

Uh oh!

Labels

2 participants

jounathaen commented Jun 3, 2025 •

edited

Loading

mkroening Jul 8, 2025 •

edited

Loading

github-actions Bot left a comment •

edited

Loading