Skip to content

fsimpl/gofer/tmpfs: Support statx btime#13038

Open
tanyifeng wants to merge 1 commit into
google:masterfrom
tanyifeng:fsimpl-statx-btime
Open

fsimpl/gofer/tmpfs: Support statx btime#13038
tanyifeng wants to merge 1 commit into
google:masterfrom
tanyifeng:fsimpl-statx-btime

Conversation

@tanyifeng
Copy link
Copy Markdown
Contributor

@tanyifeng tanyifeng commented Apr 29, 2026

gVisor did not report valid statx btime: gofer/directfs used fstat(2), which has no btime, and tmpfs did not track creation time.

Use host statx(AT_EMPTY_PATH) for gofer/directfs metadata. Restrict host statx in gofer/sandbox seccomp filters to FD-only calls using a fixed empty path and AT_EMPTY_PATH.

Track optional STATX_BTIME availability so filesystems without btime support do not expose zero btime. For tmpfs, initialize btime at inode creation and report STATX_BTIME, matching Linux >= 5.18.

Reference: fs/stat.c:vfs_statx(), mm/shmem.c:shmem_getattr()

@tanyifeng tanyifeng force-pushed the fsimpl-statx-btime branch from 608a874 to 55435a4 Compare April 29, 2026 06:53
@ayushr2
Copy link
Copy Markdown
Collaborator

ayushr2 commented Apr 29, 2026

Curious, which workload is this impacting?

@tanyifeng
Copy link
Copy Markdown
Contributor Author

Curious, which workload is this impacting?

The dependency is from tokio-rs/tracing:

Specifically, test_max_log_files uses max_log_files = 2, writes three hourly log files, and expects the oldest one to be pruned.

The pruning code depends on file creation time here:

https://github.com/tokio-rs/tracing/blob/908cc432a5994f6e17c8f36e13c217dc40085704/tracing-appender/src/rolling.rs#L605

let created = metadata.created().ok()?;

On Linux, Rust's Metadata::created() uses statx(STATX_BTIME). Before this fix, gVisor did not expose a valid STATX_BTIME, so metadata.created() failed, the old log file was skipped from pruning, and the test hit:

https://github.com/tokio-rs/tracing/blob/908cc432a5994f6e17c8f36e13c217dc40085704/tracing-appender/src/rolling.rs#L1097

panic!("this file should have been pruned already!");

Comment thread runsc/fsgofer/filter/config.go Outdated
Comment thread pkg/sentry/fsimpl/gofer/directfs_inode.go Outdated
Comment thread pkg/sentry/fsimpl/gofer/directfs_inode.go Outdated
@tanyifeng tanyifeng force-pushed the fsimpl-statx-btime branch 2 times, most recently from 0aaf7b2 to 1a06b7b Compare May 8, 2026 06:45
Copy link
Copy Markdown
Collaborator

@ayushr2 ayushr2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM, thanks!

Comment thread runsc/fsgofer/filter/config.go Outdated
Comment thread pkg/sentry/fsimpl/gofer/gofer.go Outdated
Comment thread pkg/sentry/fsimpl/gofer/directfs_inode.go Outdated
Comment thread pkg/fsutil/fsutil_unsafe.go Outdated
Comment thread runsc/boot/filter/config/config_main.go Outdated
Comment thread runsc/boot/filter/config/config_precompiled.go Outdated
gVisor did not report valid statx btime: gofer/directfs used fstat(2),
which has no btime, and tmpfs did not track creation time.

Use host statx(AT_EMPTY_PATH) for gofer/directfs metadata. Restrict host
statx in gofer/sandbox seccomp filters to FD-only calls using a fixed
empty path and AT_EMPTY_PATH.

Track optional STATX_BTIME availability so filesystems without btime
support do not expose zero btime. For tmpfs, initialize btime at inode
creation and report STATX_BTIME, matching Linux >= 5.18.

Reference: fs/stat.c:vfs_statx(), mm/shmem.c:shmem_getattr()
Signed-off-by: Tan Yifeng <yiftan@tencent.com>
@tanyifeng tanyifeng force-pushed the fsimpl-statx-btime branch from 1a06b7b to 4c56998 Compare May 23, 2026 02:15
Copy link
Copy Markdown
Collaborator

@ayushr2 ayushr2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI I imported this and fixed some things:

  • directfs was missing btime update on revalidation and restore paths (lisafs was updating it correctly)
  • changed directfsInode.updateMetadataFromStatLocked() => updateMetadataFromStatxLocked() and updated it to be same as lisafsInode.updateMetadataFromStatxLocked() implementation.
  • Deleted dentryTimestampFromStatx and just updated dentryTimestampFromUnix to use unix.Statx_t and updated all callsites.
  • Fixed newDirectfsDentry() to check unix.Statx_t.Mask field and accordingly initialize the dentry fields, analogous to newLisafsDentry().
  • Some minor cosmetic changes.

Will submit that change and retain your authorship on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants