Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
* `AIS_USE_SANITIZERS` now also enables the following sanitizers: integer, float-divide-by-zero, local-bounds, vptr, nullability (in addition to address, leak, and undefined). Sanitizers should also now emit usable stack trace info.
* Added check in the Fastpath/AIS backend to ensure the HIP Runtime is initialized. This avoids causing a segfault in the HIP Runtime.
* The default CMake build type was changed from `Debug` to `RelWithDebInfo`
* Added `HIPFILE_UNSUPPORTED_FILE_SYSTEMS` environment variable to allow fastpath operations on unsupported file systems (defaults to false)
* Added validation in Fastpath::score() to ensure the file is a regular file (not a directory, symlink, character device, etc.)
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

The changelog claims Fastpath::score() now validates the file is a regular file, but there is no regular-file check in the current implementation (and mount-info availability is not equivalent, since directories can also have mount info). Please update/remove this entry to reflect the actual behavior implemented in this PR.

Suggested change
* Added validation in Fastpath::score() to ensure the file is a regular file (not a directory, symlink, character device, etc.)

Copilot uses AI. Check for mistakes.
* Added file system validation in Fastpath::score() to accept only ext4 file systems with ordered journaling mode and XFS file systems. Other file systems can be explicitly allowed via the `HIPFILE_UNSUPPORTED_FILE_SYSTEMS` environment variable.

### Removed
* The rocFile library has been completely removed and the code is now a part of hipFile.
Expand Down
12 changes: 12 additions & 0 deletions src/amd_detail/backend/fastpath.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "hip.h"
#include "hipfile.h"
#include "io.h"
#include "mountinfo.h"
#include "stats.h"

#include <cstdint>
Expand Down Expand Up @@ -138,6 +139,17 @@ Fastpath::score(shared_ptr<IFile> file, shared_ptr<IBuffer> buffer, size_t size,
accept_io &= 0 <= file_offset;
accept_io &= 0 <= buffer_offset;

bool unsupported_file_systems{Context<Configuration>::get()->unsupportedFileSystems()};
bool ext4_ordered{false};
bool xfs{false};
auto mount_info{file->getMountInfo()};
if (mount_info.has_value()) {
ext4_ordered = mount_info->type == FilesystemType::ext4 &&
mount_info->options.ext4.journaling_mode == ExtJournalingMode::ordered;
xfs = mount_info->type == FilesystemType::xfs;
}
accept_io &= unsupported_file_systems || ext4_ordered || xfs;

uint64_t mem_align_mask{4096 - 1};
uint64_t offset_align_mask{4096 - 1};

Expand Down
7 changes: 7 additions & 0 deletions src/amd_detail/configuration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,10 @@ Configuration::statsLevel() const noexcept
HIPFILE_STATIC unsigned int stats_level_env{Environment::stats_level().value_or(0)};
return stats_level_env;
}

bool
Configuration::unsupportedFileSystems() const noexcept
{
HIPFILE_STATIC bool unsupported_file_systems_env{Environment::unsupported_file_systems().value_or(false)};
return unsupported_file_systems_env;
}
5 changes: 5 additions & 0 deletions src/amd_detail/configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ class Configuration {
/// @brief Shows the level of detail for stats collection
/// @return 0 if stats collection disabled, higher levels of detail as value increases
virtual unsigned int statsLevel() const noexcept;

/// @brief Checks if unsupported file systems are allowed in the fastpath backend
/// @return true if unsupported file systems are allowed, false if only supported file systems are
/// permitted (default)
virtual bool unsupportedFileSystems() const noexcept;
};

}
6 changes: 6 additions & 0 deletions src/amd_detail/environment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ Environment::force_compat_mode()
return Environment::get<bool>(Environment::FORCE_COMPAT_MODE);
}

optional<bool>
Environment::unsupported_file_systems()
{
return Environment::get<bool>(Environment::UNSUPPORTED_FILE_SYSTEMS);
}

optional<unsigned int>
Environment::stats_level()
{
Expand Down
13 changes: 13 additions & 0 deletions src/amd_detail/environment.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,19 @@ class Environment {
static constexpr const char *const STATS_LEVEL{"HIPFILE_STATS_LEVEL"};

static std::optional<unsigned int> stats_level();

/// @brief Allow unsupported file systems in the fastpath backend
///
/// If enabled, the fastpath backend will allow I/O on file systems other than
/// ext4 (with ordered journaling) and xfs. If disabled (default), only supported
/// file systems are permitted.
static constexpr const char *const UNSUPPORTED_FILE_SYSTEMS{"HIPFILE_UNSUPPORTED_FILE_SYSTEMS"};

/// @brief Get the value of HIPFILE_UNSUPPORTED_FILE_SYSTEMS from the environment
/// @return An optional boolean value if HIPFILE_UNSUPPORTED_FILE_SYSTEMS was set,
/// nullopt if HIPFILE_UNSUPPORTED_FILE_SYSTEMS was unset or had a value other than
/// true or false.
static std::optional<bool> unsupported_file_systems();
Comment on lines +51 to +62
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

The PR title/description refer to HIPFILE_UNSUPPORTED_FILESYSTEMS, but the code introduces HIPFILE_UNSUPPORTED_FILE_SYSTEMS (with _FILE_). To avoid user confusion and misconfiguration, please make the external documentation/PR description match the implemented environment variable name (or rename the constant if the other name is intended).

Copilot uses AI. Check for mistakes.
};

}
30 changes: 30 additions & 0 deletions test/amd_detail/configuration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ struct HipFileConfiguration : public Test {
EXPECT_CALL(msys, getenv(StrEq(hipFile::Environment::STATS_LEVEL)))
.WillOnce(Return(const_cast<char *>(hipfile_stats_level)));
}

void expect_configuration_unsupported_file_systems(const char *hipfile_unsupported_file_systems)
{
EXPECT_CALL(msys, getenv(StrEq(hipFile::Environment::UNSUPPORTED_FILE_SYSTEMS)))
.WillOnce(Return(const_cast<char *>(hipfile_unsupported_file_systems)));
}
};

TEST_F(HipFileConfiguration, FastpathEnabledIfForceCompatModeEnvironmentVariableIsNotSet)
Expand Down Expand Up @@ -194,4 +200,28 @@ TEST_F(HipFileConfiguration, StatsLevelEnvironmentVariableIsSet)
ASSERT_EQ(1, Configuration().statsLevel());
}

TEST_F(HipFileConfiguration, UnsupportedFilesystemsDisabledIfEnvironmentVariableIsNotSet)
{
expect_configuration_unsupported_file_systems(nullptr);
ASSERT_FALSE(Configuration().unsupportedFileSystems());
}

TEST_F(HipFileConfiguration, UnsupportedFilesystemsDisabledIfEnvironmentVariableIsInvalid)
{
expect_configuration_unsupported_file_systems("not-a-bool");
ASSERT_FALSE(Configuration().unsupportedFileSystems());
}

TEST_F(HipFileConfiguration, UnsupportedFilesystemsEnabledIfEnvironmentVariableIsTrue)
{
expect_configuration_unsupported_file_systems("true");
ASSERT_TRUE(Configuration().unsupportedFileSystems());
}

TEST_F(HipFileConfiguration, UnsupportedFilesystemsDisabledIfEnvironmentVariableIsFalse)
{
expect_configuration_unsupported_file_systems("false");
ASSERT_FALSE(Configuration().unsupportedFileSystems());
}

HIPFILE_WARN_NO_GLOBAL_CTOR_ON
Loading
Loading