Skip to content

fastpath: Only allow IO to approved file types and file systems#238

Open
kurtmcmillan wants to merge 28 commits intodevelopfrom
kumcmill/add-ext4-xfs-check-to-fastpath-scoring
Open

fastpath: Only allow IO to approved file types and file systems#238
kurtmcmillan wants to merge 28 commits intodevelopfrom
kumcmill/add-ext4-xfs-check-to-fastpath-scoring

Conversation

@kurtmcmillan
Copy link
Copy Markdown
Collaborator

@kurtmcmillan kurtmcmillan commented Mar 25, 2026

Motivation

Currently, hipFile has only been tested with block devices and regular files on ext4(ordered) or xfs file systems. This change updates Fastpath::score() to accept IO operations that target block devices or files backed by ext4(ordered) or xfs.

Technical Details

When a file is registered, hipFile will cache file type and mount information. Fastpath::score() will use the cached information to determine if it will accept an IO.

If HIPFILE_UNSUPPORTED_FILE_SYSTEMS=true is set in the environment, then Fastpath::score will allow IO to regular files on unsupported filesystems.

AIHIPFILE-114

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the fastpath backend’s eligibility logic to only select fastpath I/O for regular files on ext4 (data=ordered) or xfs by default, with an environment-variable override to permit other filesystems.

Changes:

  • Add HIPFILE_UNSUPPORTED_FILESYSTEMS environment plumbing via Environment and Configuration.
  • Update Fastpath::score() to require a regular file and a supported filesystem (unless overridden).
  • Extend unit tests to cover the new configuration flag and fastpath filesystem/type gating behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/amd_detail/backend/fastpath.cpp Enforces regular-file + supported-filesystem constraints in fastpath scoring.
src/amd_detail/environment.h Adds the new env var constant and accessor declaration.
src/amd_detail/environment.cpp Implements Environment::unsupported_filesystems() via existing bool parser.
src/amd_detail/configuration.h Adds Configuration::unsupportedFilesystems() API.
src/amd_detail/configuration.cpp Reads/caches HIPFILE_UNSUPPORTED_FILESYSTEMS into configuration.
test/amd_detail/mconfiguration.h Extends configuration mock with unsupportedFilesystems().
test/amd_detail/configuration.cpp Adds tests verifying env var parsing/default behavior for unsupported filesystems flag.
test/amd_detail/fastpath.cpp Updates/extends fastpath tests for mount info + regular file checks and override behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@kurtmcmillan kurtmcmillan force-pushed the kumcmill/add-ext4-xfs-check-to-fastpath-scoring branch 2 times, most recently from e927ed3 to abbeeb1 Compare March 26, 2026 16:12
@kurtmcmillan kurtmcmillan requested a review from Copilot March 26, 2026 16:16
@kurtmcmillan kurtmcmillan changed the title fastpath: Restrict IO to ext4 (data=ordered) or xfs unless HIPFILE_UNSUPPORTED_FILESYSTEMS=true fastpath: Restrict IO to ext4 (data=ordered) or xfs unless HIPFILE_UNSUPPORTED_FILE_SYSTEMS=true Mar 26, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@kurtmcmillan kurtmcmillan force-pushed the kumcmill/add-ext4-xfs-check-to-fastpath-scoring branch from abbeeb1 to a9ea835 Compare March 30, 2026 19:21
@kurtmcmillan kurtmcmillan requested a review from Copilot March 30, 2026 19:24
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@kurtmcmillan kurtmcmillan changed the title fastpath: Restrict IO to ext4 (data=ordered) or xfs unless HIPFILE_UNSUPPORTED_FILE_SYSTEMS=true fastpath: Only allow IO to approved file types and file systems Mar 30, 2026
@kurtmcmillan kurtmcmillan force-pushed the kumcmill/add-ext4-xfs-check-to-fastpath-scoring branch from a9ea835 to ad9b516 Compare March 30, 2026 20:32
@kurtmcmillan kurtmcmillan marked this pull request as ready for review March 30, 2026 20:45
Copilot AI review requested due to automatic review settings March 30, 2026 20:45
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

The mount information the fastpath needs can now be obtained from File::onXfs()
and File::onExt4Ordered().
At one time this was used to determine if O_DIRECT was set on File's file
descriptor. File now holds buffered (~O_DIRECT) and unbuffered (O_DIRECT) file
descriptors. Clients can use the file descriptor that best suits their purpose.
…::score expectations

This will reduce the blast radius of the following changes.
This will be used by the to determine if IO to unsupported file systems should
be permitted to use the fastpath backend.
… checks in Fastpath::score

If HIPFILE_UNSUPPORTED_FILE_SYSTEMS=true, Fastpath::score will accept IO to
regular files on any filesystem.
@kurtmcmillan kurtmcmillan force-pushed the kumcmill/add-ext4-xfs-check-to-fastpath-scoring branch from ad9b516 to 192215b Compare March 31, 2026 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants