From c160b06bf7d7237389006b4a198ff957d5f56efc Mon Sep 17 00:00:00 2001 From: Kurt McMillan Date: Wed, 5 Nov 2025 23:01:29 +0000 Subject: [PATCH 01/10] rocfile: Introduce fastpath backend --- rocfile/src/CMakeLists.txt | 1 + rocfile/src/backend/fastpath.cpp | 142 +++++++++++++ rocfile/src/backend/fastpath.h | 24 +++ rocfile/src/rocfile.cpp | 5 + rocfile/src/state.cpp | 14 +- rocfile/src/state.h | 2 +- rocfile/test/CMakeLists.txt | 1 + rocfile/test/fastpath.cpp | 349 +++++++++++++++++++++++++++++++ rocfile/test/io.cpp | 5 + 9 files changed, 541 insertions(+), 2 deletions(-) create mode 100644 rocfile/src/backend/fastpath.cpp create mode 100644 rocfile/src/backend/fastpath.h create mode 100644 rocfile/test/fastpath.cpp diff --git a/rocfile/src/CMakeLists.txt b/rocfile/src/CMakeLists.txt index 3bde410a..782f2dd7 100644 --- a/rocfile/src/CMakeLists.txt +++ b/rocfile/src/CMakeLists.txt @@ -6,6 +6,7 @@ set(ROCFILE_SOURCES async.cpp backend/asyncop-fallback.cpp backend/fallback.cpp + backend/fastpath.cpp batch/batch.cpp buffer.cpp context.cpp diff --git a/rocfile/src/backend/fastpath.cpp b/rocfile/src/backend/fastpath.cpp new file mode 100644 index 00000000..6e9da027 --- /dev/null +++ b/rocfile/src/backend/fastpath.cpp @@ -0,0 +1,142 @@ +/* Copyright (c) Advanced Micro Devices, Inc. All rights reserved. + * + * SPDX-License-Identifier: MIT + */ + +#include "context.h" +#include "fastpath.h" +#include "rocfile.h" + +#include +#include + +using namespace rocFile; +using namespace std; + +/* The fastpath backend is used when: + * - IO is alligned (size, file_offset, buffer_offset) + * - The buffer is of type hipMemoryTypeDevice + * - The file has been opened with the O_DIRECT flag + * - The file is either a block device or a regular file + * - The file resides on a an xfs or ext4 (journaling mode: ordered) filesystem + * + * When using the fastpath the IO flows through the following layers + * - rocfile hip wrapper (userspace) + * - repository: hipFile + * - file: rocfile/hip.cpp + * - methods: Hip::hipAmdFileRead(...), Hip::hipAmdFileWrite(...) + * - Throws std::system_error if status is non-zero (set by kfd) + * - Throws Hip::runtime_error if hip runtime does not return hipSuccess or + * if rocFile was unable to find hipAmdFileRead/hipAmdFileWrite + * - hip runtime (userspace) + * - repository: rocm-systems + * - file: projects/clr/hipamd/src/hip_storage.cpp + * - functions: hipAmdFileRead(...), hipAmdFileWrite(...) + * - hipAmdFileRead & hipAmdFileWrite are not exposed through a public + * header so hipGetProcAddress() is used to lookup each function's function + * pointer + * - **Currently returns hipSuccess if size is zero (Need to fix)** + * - returns hipErrorInvalidDevice if unable to lookup current device + * - returns hipErrorUnknown if rocr runtime does not return true (success) + * - rocr runtime (userspace) + * - repository: rocm-systems + * - file: projects/clr/rocclr/device/rocm/rocdevice.cpp + * - methods: Device::amdFileRead(...), Device::amdFileWrite(...) + * - Does not perform any input validation + * - Logs on every IO failure + * - size_copied and status are never modified + * - returns false (error) if hsa does not return HSA_STATUS_SUCCESS + * - hsa (userspace) + * - repository: rocm-systems + * - file: projects/rocr-runtime/runtime/hsa-runtime/core/runtime/hsa_ext_amd.cpp + * - hsa_amd_ais_file_read(...), hsa_amd_ais_file_write(...) + * - ** Currently returns HSA_STATUS_ERROR_INVALID_ARGUMENT if size is zero + * or device (buffer) pointer is zero ** + * - status and size_coppied are left untouched + * - if devicePtr is NULL or size is zero, HSA_STATUS_ERROR_INVALID_ARGUMENT is returned + * - if hsaKmtAisReadWriteFile(...) does not return success, HSA_STATUS_ERROR is returned + * - thunk (userspace) + * - repository: rocm-systems + * - file: projects/rocr-runtime/libhsakmt/src/ais.c + * - functions: hsaKmtAisReadWriteFile(...), + * - If the buffer pointer, and io_size combination are invalid, return HSAKMT_STATUS_INVALID_PARAMETER + * - converts buffer pointer + io_size to a buffer handle + * - Does not check for alignment + * - size and status are untouched and return HSAKMT_STATUS_INVALID_PARAMETER if + * - the buffer pointer + size combination are invalid + * - if AisFlags (read/write) are invalid + * - If the kfd ioctl does not return zero, return HSAKMT_STATUS_ERROR + * - kfd (chardev) (kernel) + * - repository: ROCm + * - file: amdgpu/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c + * - function: kfd_ioctl_ais(...) + * - returns -EINVAL if op != READ or WRITE + * - returns -EINVAL if fd < 0 + * - returns -EINVAL if unable to lookup device + * - returns -NODEV if AIS is not initialized on the device + * - returns -ESRCH if unable to bind the process ot the device + * - returns -EINVAL if the buffer's domain is not AMDGPU_GEM_DOMAIN_VRAM + * - returns an error if it is unable to pin the buffer object + * - returns the result of the call to kfd_ais_rw_file(...) + * - the value returned by kfd_ioctl_ais() is also stored in the status + * variable provided in the initial call to + * hipAmdFileRead/hipAmdFileWrite + * - kfd (kernel) + * - repository: ROCm + * - file: amdgpu/drivers/gpu/drm/amd/amdkfd/kfd_ais.c + * - function: kfd_ais_rw_file(...) + * - returns -EINVAL if file_offset or size are not page aligned (4K) + * - returns -EBADF unable to lookup the file object + * - returns -ENODEV if unable to lookup the storage device + * - returns -ENODEV if pcie_p2pdma_distance < 0 + * - returns -EINVAL if buffer object domain is not AMDGPU_GEM_DOMAIN_VRAM + * - returns an error if unable to create a sg_table for the buffer object + * - returns -ENOMEM if unbale to init a bvec + * - ** Appears to return -EIO on short reads ** + * - ** Calls dev_dbg on every IO that doesn't return an error ** + * - ** Should not retry on short read/writes ** + */ + +int +Fastpath::score(shared_ptr file, shared_ptr buffer, size_t size, off_t file_offset, + off_t buffer_offset) const +{ + bool accept_io{true}; + + accept_io &= !!(file->getStatusFlags() & O_DIRECT); + + accept_io &= buffer->getType() == hipMemoryTypeDevice; + + accept_io &= 0 <= file_offset; + accept_io &= 0 <= buffer_offset; + + accept_io &= !(size & 0xFFF); + accept_io &= !(file_offset & 0xFFF); + accept_io &= !(buffer_offset & 0xFFF); + + return accept_io ? 100 : -1; +} + +ssize_t +Fastpath::io(IoType type, shared_ptr file, shared_ptr buffer, size_t size, off_t file_offset, + off_t buffer_offset) +{ + void *devptr{reinterpret_cast(reinterpret_cast(buffer->getBuffer()) + buffer_offset)}; + hipAmdFileHandle_t handle{}; + size_t nbytes{}; + + handle.fd = file->getFd(); + + switch (type) { + case IoType::Read: + nbytes = Context::get()->hipAmdFileRead(handle, devptr, size, file_offset); + break; + case IoType::Write: + nbytes = Context::get()->hipAmdFileWrite(handle, devptr, size, file_offset); + break; + default: + throw std::runtime_error("Invalid IoType"); + } + + return static_cast(nbytes); +} diff --git a/rocfile/src/backend/fastpath.h b/rocfile/src/backend/fastpath.h new file mode 100644 index 00000000..c4af215f --- /dev/null +++ b/rocfile/src/backend/fastpath.h @@ -0,0 +1,24 @@ +/* Copyright (c) Advanced Micro Devices, Inc. All rights reserved. + * + * SPDX-License-Identifier: MIT + */ + +#pragma once + +#include "backend.h" +#include "file.h" +#include "io.h" + +namespace rocFile { + +struct Fastpath : public Backend { + virtual ~Fastpath() override = default; + + int score(std::shared_ptr file, std::shared_ptr buffer, size_t size, off_t file_offset, + off_t buffer_offset) const override; + + ssize_t io(IoType type, std::shared_ptr file, std::shared_ptr buffer, size_t size, + off_t file_offset, off_t buffer_offset) override; +}; + +} diff --git a/rocfile/src/rocfile.cpp b/rocfile/src/rocfile.cpp index 10f2b60d..0064b304 100644 --- a/rocfile/src/rocfile.cpp +++ b/rocfile/src/rocfile.cpp @@ -21,6 +21,7 @@ #include #include #include +#include using namespace rocFile; @@ -276,6 +277,10 @@ catch (const Sys::RuntimeError &e) { errno = e.error; return -1; } +catch (const std::system_error &e) { + errno = e.code().value(); + return -1; +} catch (...) { return -rocFileInternalError; } diff --git a/rocfile/src/state.cpp b/rocfile/src/state.cpp index f7806179..69b4201c 100644 --- a/rocfile/src/state.cpp +++ b/rocfile/src/state.cpp @@ -4,6 +4,7 @@ */ #include "backend/fallback.h" +#include "backend/fastpath.h" #include "batch/batch.h" #include "buffer.h" #include "file.h" @@ -26,7 +27,7 @@ struct Backend; namespace rocFile { -DriverState::DriverState() : ref_count{0}, backends{std::shared_ptr(new Fallback{})} +DriverState::DriverState() : ref_count{0} { this->file_map = std::make_unique(); this->batch_map = std::make_unique(); @@ -230,6 +231,17 @@ DriverState::incrRefCount() unique_lock ulock{state_mutex}; ref_count++; + + // If backends are added within DriverState's constructor we cannot test the + // behavior of getHipAmdFileReadPtr() and getHipAmdFileWritePtr() as the + // default DriverState is constructed before main in context.cpp as part of + // RocFileInit's constructor. + if (ref_count == 1 && backends.empty()) { + if (getHipAmdFileReadPtr() && getHipAmdFileWritePtr()) { + backends.emplace_back(new Fastpath{}); + } + backends.emplace_back(new Fallback{}); + } } void diff --git a/rocfile/src/state.h b/rocfile/src/state.h index 68e42141..fb2d8769 100644 --- a/rocfile/src/state.h +++ b/rocfile/src/state.h @@ -245,7 +245,7 @@ class DriverState { std::unique_ptr stream_map; // Backends available to service IO requests - const std::vector> backends; + std::vector> backends; /// Mutex to protect the state mutable std::shared_mutex state_mutex; diff --git a/rocfile/test/CMakeLists.txt b/rocfile/test/CMakeLists.txt index a3bf1f36..99ac06f4 100644 --- a/rocfile/test/CMakeLists.txt +++ b/rocfile/test/CMakeLists.txt @@ -21,6 +21,7 @@ set(TEST_SOURCE_FILES driver.cpp handle.cpp hip.cpp + fastpath.cpp io.cpp misc.cpp mountinfo.cpp diff --git a/rocfile/test/fastpath.cpp b/rocfile/test/fastpath.cpp new file mode 100644 index 00000000..6791b7cc --- /dev/null +++ b/rocfile/test/fastpath.cpp @@ -0,0 +1,349 @@ +/* Copyright (c) Advanced Micro Devices, Inc. All rights reserved. + * + * SPDX-License-Identifier: MIT + */ + +#include "mbuffer.h" +#include "mfile.h" +#include "mhip.h" +#include "rocfile-test.h" + +#include "backend/fastpath.h" +#include "mountinfo.h" +#include "buffer.h" +#include "file.h" +#include "hip.h" +#include "io.h" + +#include + +using namespace rocFile; +using namespace testing; +using namespace std; + +HIPFILE_WARN_NO_GLOBAL_CTOR_OFF + +static int SCORE_ACCEPT{100}; +static const int SCORE_REJECT{-1}; + +namespace rocFile { +inline bool +operator==(const hipAmdFileHandle_t &lhs, const hipAmdFileHandle_t &rhs) +{ + return lhs.fd == rhs.fd; +} +} + +// Provide defaut values for variables used in fastpath tests +struct FastpathTestBase { + const size_t DEFAULT_IO_SIZE{1024 * 1024}; + void *const DEFAULT_BUFFER_ADDR{reinterpret_cast(0xCAFE000)}; + const off_t DEFAULT_BUFFER_OFFSET{4096}; + const hipMemoryType DEFAULT_BUFFER_TYPE{hipMemoryTypeDevice}; + const int DEFAULT_FILE_DESCRIPTOR{7}; + const int DEFAULT_FILE_FLAGS{O_DIRECT}; + const off_t DEFAULT_FILE_OFFSET{8192}; + + // Buffer and file mocks used to setup expectations + shared_ptr> mfile{make_shared>()}; + shared_ptr> mbuffer{make_shared>()}; +}; + +struct FastpathTest : public FastpathTestBase, public Test {}; + +TEST_F(FastpathTest, DefaultExpecations) +{ + EXPECT_CALL(*mfile, getStatusFlags).WillOnce(Return(DEFAULT_FILE_FLAGS)); + EXPECT_CALL(*mbuffer, getType).WillOnce(Return(DEFAULT_BUFFER_TYPE)); + + ASSERT_EQ(Fastpath().score(mfile, mbuffer, DEFAULT_IO_SIZE, DEFAULT_FILE_OFFSET, DEFAULT_BUFFER_OFFSET), + SCORE_ACCEPT); +} + +TEST_F(FastpathTest, BufferedFile) +{ + EXPECT_CALL(*mfile, getStatusFlags).WillOnce(Return(DEFAULT_FILE_FLAGS & ~O_DIRECT)); + EXPECT_CALL(*mbuffer, getType).WillOnce(Return(DEFAULT_BUFFER_TYPE)); + + ASSERT_EQ(Fastpath().score(mfile, mbuffer, DEFAULT_IO_SIZE, DEFAULT_FILE_OFFSET, DEFAULT_BUFFER_OFFSET), + SCORE_REJECT); +} + +struct FastpathSupportedHipMemoryParam : public FastpathTestBase, public TestWithParam {}; + +TEST_P(FastpathSupportedHipMemoryParam, Score) +{ + EXPECT_CALL(*mfile, getStatusFlags).WillOnce(Return(DEFAULT_FILE_FLAGS)); + EXPECT_CALL(*mbuffer, getType).WillOnce(Return(GetParam())); + + ASSERT_EQ(Fastpath().score(mfile, mbuffer, DEFAULT_IO_SIZE, DEFAULT_FILE_OFFSET, DEFAULT_BUFFER_OFFSET), + SCORE_ACCEPT); +} + +INSTANTIATE_TEST_SUITE_P(FastpathTest, FastpathSupportedHipMemoryParam, ValuesIn(SupportedHipMemoryTypes)); + +struct FastpathUnsupportedHipMemoryParam : public FastpathTestBase, public TestWithParam {}; + +TEST_P(FastpathUnsupportedHipMemoryParam, Score) +{ + EXPECT_CALL(*mfile, getStatusFlags).WillOnce(Return(DEFAULT_FILE_FLAGS)); + EXPECT_CALL(*mbuffer, getType).WillOnce(Return(GetParam())); + + ASSERT_EQ(Fastpath().score(mfile, mbuffer, DEFAULT_IO_SIZE, DEFAULT_FILE_OFFSET, DEFAULT_BUFFER_OFFSET), + SCORE_REJECT); +} + +INSTANTIATE_TEST_SUITE_P(FastpathTest, FastpathUnsupportedHipMemoryParam, + ValuesIn(UnsupportedHipMemoryTypes)); + +struct FastpathAlignedIoSizesParam : public FastpathTestBase, public TestWithParam {}; + +TEST_P(FastpathAlignedIoSizesParam, Score) +{ + EXPECT_CALL(*mfile, getStatusFlags).WillOnce(Return(DEFAULT_FILE_FLAGS)); + EXPECT_CALL(*mbuffer, getType).WillOnce(Return(DEFAULT_BUFFER_TYPE)); + + ASSERT_EQ(Fastpath().score(mfile, mbuffer, GetParam(), DEFAULT_FILE_OFFSET, DEFAULT_BUFFER_OFFSET), + SCORE_ACCEPT); +} + +INSTANTIATE_TEST_SUITE_P(FastpathTest, FastpathAlignedIoSizesParam, + Values(0, 4096, 1024 * 1024, 1024 * 1024 * 1024)); + +struct FastpathUnalignedIoSizesParam : public FastpathTestBase, public TestWithParam {}; + +TEST_P(FastpathUnalignedIoSizesParam, Score) +{ + EXPECT_CALL(*mfile, getStatusFlags).WillOnce(Return(DEFAULT_FILE_FLAGS)); + EXPECT_CALL(*mbuffer, getType).WillOnce(Return(DEFAULT_BUFFER_TYPE)); + + ASSERT_EQ(Fastpath().score(mfile, mbuffer, GetParam(), DEFAULT_FILE_OFFSET, DEFAULT_BUFFER_OFFSET), + SCORE_REJECT); +} + +INSTANTIATE_TEST_SUITE_P(FastpathTest, FastpathUnalignedIoSizesParam, + Values(1, 4096 - 1, 1024 * 1024 + 1, 1024 * 1024 * 1024 - 1)); + +struct FastpathAlignedFileOffsetsParam : public FastpathTestBase, public TestWithParam {}; + +TEST_P(FastpathAlignedFileOffsetsParam, Score) +{ + EXPECT_CALL(*mfile, getStatusFlags).WillOnce(Return(DEFAULT_FILE_FLAGS)); + EXPECT_CALL(*mbuffer, getType).WillOnce(Return(DEFAULT_BUFFER_TYPE)); + + ASSERT_EQ(Fastpath().score(mfile, mbuffer, DEFAULT_IO_SIZE, GetParam(), DEFAULT_BUFFER_OFFSET), + SCORE_ACCEPT); +} + +static array aligned_offsets{0, 4096, 1024 * 1024, 1024 * 1024 * 1024}; +INSTANTIATE_TEST_SUITE_P(FastpathTest, FastpathAlignedFileOffsetsParam, ValuesIn(aligned_offsets)); + +/// @brief Tests negative and unaligned file offsets +struct FastpathInvalidFileOffsetsParam : public FastpathTestBase, public TestWithParam {}; + +TEST_P(FastpathInvalidFileOffsetsParam, Score) +{ + EXPECT_CALL(*mfile, getStatusFlags).WillOnce(Return(DEFAULT_FILE_FLAGS)); + EXPECT_CALL(*mbuffer, getType).WillOnce(Return(DEFAULT_BUFFER_TYPE)); + + ASSERT_EQ(Fastpath().score(mfile, mbuffer, DEFAULT_IO_SIZE, GetParam(), DEFAULT_BUFFER_OFFSET), + SCORE_REJECT); +} + +static array invalid_offsets{-1024 * 1024 * 1024, -1024 * 1024, -4096, 1, 4096 - 1, + 1024 * 1024 + 1, 1024 * 1024 * 1024 - 1}; +INSTANTIATE_TEST_SUITE_P(FastpathTest, FastpathInvalidFileOffsetsParam, ValuesIn(invalid_offsets)); + +struct FastpathAlignedBufferOffsetsParam : public FastpathTestBase, public TestWithParam {}; + +TEST_P(FastpathAlignedBufferOffsetsParam, Score) +{ + EXPECT_CALL(*mfile, getStatusFlags).WillOnce(Return(DEFAULT_FILE_FLAGS)); + EXPECT_CALL(*mbuffer, getType).WillOnce(Return(DEFAULT_BUFFER_TYPE)); + + ASSERT_EQ(Fastpath().score(mfile, mbuffer, DEFAULT_IO_SIZE, DEFAULT_BUFFER_OFFSET, GetParam()), + SCORE_ACCEPT); +} + +INSTANTIATE_TEST_SUITE_P(FastpathTest, FastpathAlignedBufferOffsetsParam, ValuesIn(aligned_offsets)); + +/// @brief Tests negative and unaligned buffer offsets +struct FastpathInvalidBufferOffsetsParam : public FastpathTestBase, public TestWithParam {}; + +TEST_P(FastpathInvalidBufferOffsetsParam, Score) +{ + EXPECT_CALL(*mfile, getStatusFlags).WillOnce(Return(DEFAULT_FILE_FLAGS)); + EXPECT_CALL(*mbuffer, getType).WillOnce(Return(DEFAULT_BUFFER_TYPE)); + + ASSERT_EQ(Fastpath().score(mfile, mbuffer, DEFAULT_IO_SIZE, DEFAULT_FILE_OFFSET, GetParam()), + SCORE_REJECT); +} + +INSTANTIATE_TEST_SUITE_P(FastpathTest, FastpathInvalidBufferOffsetsParam, ValuesIn(invalid_offsets)); + +struct FastpathIoParam : public FastpathTestBase, public TestWithParam {}; + +TEST_P(FastpathIoParam, IoConfiguresHandle) +{ + StrictMock mhip; + + hipAmdFileHandle_t handle{}; + handle.fd = DEFAULT_FILE_DESCRIPTOR; + + EXPECT_CALL(*mbuffer, getBuffer).WillOnce(Return(DEFAULT_BUFFER_ADDR)); + EXPECT_CALL(*mfile, getFd).WillOnce(Return(DEFAULT_FILE_DESCRIPTOR)); + switch (GetParam()) { + case IoType::Read: + EXPECT_CALL(mhip, hipAmdFileRead(Eq(handle), _, _, _)); + break; + case IoType::Write: + EXPECT_CALL(mhip, hipAmdFileWrite(Eq(handle), _, _, _)); + break; + default: + FAIL() << "Invalid IoType"; + } + + Fastpath().io(GetParam(), mfile, mbuffer, DEFAULT_IO_SIZE, DEFAULT_FILE_OFFSET, DEFAULT_BUFFER_OFFSET); +} + +TEST_P(FastpathIoParam, IoCalculatesCorrectDevicePointer) +{ + StrictMock mhip; + + off_t buffer_offset{0x1000}; + void *buffer_addr{reinterpret_cast(0x20000)}; + void *expected_device_ptr{reinterpret_cast(0x21000)}; + + EXPECT_CALL(*mbuffer, getBuffer).WillOnce(Return(buffer_addr)); + EXPECT_CALL(*mfile, getFd).WillOnce(Return(DEFAULT_FILE_DESCRIPTOR)); + switch (GetParam()) { + case IoType::Read: + EXPECT_CALL(mhip, hipAmdFileRead(_, expected_device_ptr, _, _)); + break; + case IoType::Write: + EXPECT_CALL(mhip, hipAmdFileWrite(_, expected_device_ptr, _, _)); + break; + default: + FAIL() << "Invalid IoType"; + } + + Fastpath().io(GetParam(), mfile, mbuffer, DEFAULT_IO_SIZE, DEFAULT_FILE_OFFSET, buffer_offset); +} + +TEST_P(FastpathIoParam, IoPassesThroughSizeAndFileOffset) +{ + StrictMock mhip; + + EXPECT_CALL(*mbuffer, getBuffer).WillOnce(Return(DEFAULT_BUFFER_ADDR)); + EXPECT_CALL(*mfile, getFd).WillOnce(Return(DEFAULT_FILE_DESCRIPTOR)); + switch (GetParam()) { + case IoType::Read: + EXPECT_CALL(mhip, hipAmdFileRead(_, _, Eq(DEFAULT_IO_SIZE), Eq(DEFAULT_FILE_OFFSET))); + break; + case IoType::Write: + EXPECT_CALL(mhip, hipAmdFileWrite(_, _, Eq(DEFAULT_IO_SIZE), Eq(DEFAULT_FILE_OFFSET))); + break; + default: + FAIL() << "Invalid IoType"; + } + + Fastpath().io(GetParam(), mfile, mbuffer, DEFAULT_IO_SIZE, DEFAULT_FILE_OFFSET, DEFAULT_BUFFER_OFFSET); +} + +TEST_P(FastpathIoParam, IoReturnsBytesTransferred) +{ + StrictMock mhip; + + EXPECT_CALL(*mbuffer, getBuffer).WillOnce(Return(DEFAULT_BUFFER_ADDR)); + EXPECT_CALL(*mfile, getFd).WillOnce(Return(DEFAULT_FILE_DESCRIPTOR)); + switch (GetParam()) { + case IoType::Read: + EXPECT_CALL(mhip, hipAmdFileRead(_, _, _, _)).WillOnce(Return(DEFAULT_IO_SIZE)); + break; + case IoType::Write: + EXPECT_CALL(mhip, hipAmdFileWrite(_, _, _, _)).WillOnce(Return(DEFAULT_IO_SIZE)); + break; + default: + FAIL() << "Invalid IoType"; + } + + ASSERT_EQ(Fastpath().io(GetParam(), mfile, mbuffer, DEFAULT_IO_SIZE, DEFAULT_FILE_OFFSET, + DEFAULT_BUFFER_OFFSET), + DEFAULT_IO_SIZE); +} + +TEST_P(FastpathIoParam, IoReturnsBytesTransferredShort) +{ + StrictMock mhip; + + size_t nbytes{4096}; + + ASSERT_LT(nbytes, DEFAULT_IO_SIZE); + + EXPECT_CALL(*mbuffer, getBuffer).WillOnce(Return(DEFAULT_BUFFER_ADDR)); + EXPECT_CALL(*mfile, getFd).WillOnce(Return(DEFAULT_FILE_DESCRIPTOR)); + switch (GetParam()) { + case IoType::Read: + EXPECT_CALL(mhip, hipAmdFileRead(_, _, _, _)).WillOnce(Return(nbytes)); + break; + case IoType::Write: + EXPECT_CALL(mhip, hipAmdFileWrite(_, _, _, _)).WillOnce(Return(nbytes)); + break; + default: + FAIL() << "Invalid IoType"; + } + + ASSERT_EQ(Fastpath().io(GetParam(), mfile, mbuffer, DEFAULT_IO_SIZE, DEFAULT_FILE_OFFSET, + DEFAULT_BUFFER_OFFSET), + nbytes); +} + +// Ensure hip errors thrown by Hip::hipAmdFileRead/Hip::hipAmdFileWtite are not masked +TEST_P(FastpathIoParam, IoDoesNotMaskHipRuntimeError) +{ + StrictMock mhip; + + EXPECT_CALL(*mbuffer, getBuffer).WillOnce(Return(DEFAULT_BUFFER_ADDR)); + EXPECT_CALL(*mfile, getFd).WillOnce(Return(DEFAULT_FILE_DESCRIPTOR)); + switch (GetParam()) { + case IoType::Read: + EXPECT_CALL(mhip, hipAmdFileRead).WillOnce(Throw(Hip::RuntimeError(hipErrorUnknown))); + break; + case IoType::Write: + EXPECT_CALL(mhip, hipAmdFileWrite).WillOnce(Throw(Hip::RuntimeError(hipErrorUnknown))); + break; + default: + FAIL() << "Invalid IoType"; + } + + EXPECT_THROW(Fastpath().io(GetParam(), mfile, mbuffer, DEFAULT_IO_SIZE, DEFAULT_FILE_OFFSET, + DEFAULT_BUFFER_OFFSET), + Hip::RuntimeError); +} + +// Ensure hip errors thrown by Hip::hipAmdFileRead/Hip::hipAmdFileWtite are not masked +TEST_P(FastpathIoParam, IoDoesNotMaskSystemError) +{ + StrictMock mhip; + + EXPECT_CALL(*mbuffer, getBuffer).WillOnce(Return(DEFAULT_BUFFER_ADDR)); + EXPECT_CALL(*mfile, getFd).WillOnce(Return(DEFAULT_FILE_DESCRIPTOR)); + switch (GetParam()) { + case IoType::Read: + EXPECT_CALL(mhip, hipAmdFileRead).WillOnce(Throw(system_error(ENODEV, generic_category()))); + break; + case IoType::Write: + EXPECT_CALL(mhip, hipAmdFileWrite).WillOnce(Throw(system_error(EINVAL, generic_category()))); + break; + default: + FAIL() << "Invalid IoType"; + } + + EXPECT_THROW(Fastpath().io(GetParam(), mfile, mbuffer, DEFAULT_IO_SIZE, DEFAULT_FILE_OFFSET, + DEFAULT_BUFFER_OFFSET), + std::system_error); +} + +INSTANTIATE_TEST_SUITE_P(FastpathTest, FastpathIoParam, Values(IoType::Read, IoType::Write)); + +HIPFILE_WARN_NO_GLOBAL_CTOR_ON diff --git a/rocfile/test/io.cpp b/rocfile/test/io.cpp index 9d3cc82a..a95baf29 100644 --- a/rocfile/test/io.cpp +++ b/rocfile/test/io.cpp @@ -181,6 +181,11 @@ struct RocFileFallbackValidation : ::testing::TestWithParam { StrictMock mhip; StrictMock msys; + // Fail the lookup for hipAmdFileRead & hipAmdFileWrite when the + // driver's reference count is bumped. + EXPECT_CALL(mhip, hipRuntimeGetVersion); + EXPECT_CALL(mhip, hipGetProcAddress(StrEq("hipAmdFileRead"), _, _, _)); + assert(rocFileDriverOpen() == ROCFILE_SUCCESS); expect_buffer_registration(mhip, hipMemoryTypeDevice); From 0ff715c7fe5802241825a68ff37b4addac020f3c Mon Sep 17 00:00:00 2001 From: Kurt McMillan Date: Thu, 6 Nov 2025 23:13:39 +0000 Subject: [PATCH 02/10] rocfile: Update fastpath notes --- rocfile/src/backend/fastpath.cpp | 112 +++++++++++++++++-------------- rocfile/test/fastpath.cpp | 2 +- 2 files changed, 62 insertions(+), 52 deletions(-) diff --git a/rocfile/src/backend/fastpath.cpp b/rocfile/src/backend/fastpath.cpp index 6e9da027..a9737955 100644 --- a/rocfile/src/backend/fastpath.cpp +++ b/rocfile/src/backend/fastpath.cpp @@ -14,87 +14,97 @@ using namespace rocFile; using namespace std; /* The fastpath backend is used when: - * - IO is alligned (size, file_offset, buffer_offset) - * - The buffer is of type hipMemoryTypeDevice * - The file has been opened with the O_DIRECT flag - * - The file is either a block device or a regular file - * - The file resides on a an xfs or ext4 (journaling mode: ordered) filesystem + * - IO is 4K aligned (size, file_offset, buffer_offset) + * - This requirement will be relaxed in the future + * - The buffer type is hipMemoryTypeDevice * - * When using the fastpath the IO flows through the following layers - * - rocfile hip wrapper (userspace) + * When using the fastpath the IO flows through the following + * - rocFile HIP wrapper (userspace) * - repository: hipFile * - file: rocfile/hip.cpp * - methods: Hip::hipAmdFileRead(...), Hip::hipAmdFileWrite(...) - * - Throws std::system_error if status is non-zero (set by kfd) - * - Throws Hip::runtime_error if hip runtime does not return hipSuccess or - * if rocFile was unable to find hipAmdFileRead/hipAmdFileWrite - * - hip runtime (userspace) + * - throws + * - std::system_error if status is non-zero (set by kfd) + * - Hip::runtime_error if hip runtime does not return hipSuccess or + * if rocFile was unable to find hipAmdFileRead/hipAmdFileWrite + * - HIP Runtime (userspace) * - repository: rocm-systems * - file: projects/clr/hipamd/src/hip_storage.cpp * - functions: hipAmdFileRead(...), hipAmdFileWrite(...) * - hipAmdFileRead & hipAmdFileWrite are not exposed through a public * header so hipGetProcAddress() is used to lookup each function's function * pointer - * - **Currently returns hipSuccess if size is zero (Need to fix)** - * - returns hipErrorInvalidDevice if unable to lookup current device - * - returns hipErrorUnknown if rocr runtime does not return true (success) - * - rocr runtime (userspace) + * - returns + * - hipSuccess if size is zero + * - This check should be removed + * - hipErrorInvalidDevice if unable to lookup current device + * - hipErrorUnknown if rocr runtime does not return success + * - ROCR Runtime (userspace) * - repository: rocm-systems * - file: projects/clr/rocclr/device/rocm/rocdevice.cpp * - methods: Device::amdFileRead(...), Device::amdFileWrite(...) * - Does not perform any input validation * - Logs on every IO failure * - size_copied and status are never modified - * - returns false (error) if hsa does not return HSA_STATUS_SUCCESS - * - hsa (userspace) + * - returns error if HSA does not return HSA_STATUS_SUCCESS + * - HSA (userspace) * - repository: rocm-systems * - file: projects/rocr-runtime/runtime/hsa-runtime/core/runtime/hsa_ext_amd.cpp * - hsa_amd_ais_file_read(...), hsa_amd_ais_file_write(...) - * - ** Currently returns HSA_STATUS_ERROR_INVALID_ARGUMENT if size is zero - * or device (buffer) pointer is zero ** - * - status and size_coppied are left untouched - * - if devicePtr is NULL or size is zero, HSA_STATUS_ERROR_INVALID_ARGUMENT is returned - * - if hsaKmtAisReadWriteFile(...) does not return success, HSA_STATUS_ERROR is returned - * - thunk (userspace) + * - status and size_coppied untouched + * - returns + * - HSA_STATUS_ERROR_INVALID_ARGUMENT if size is zero + * - This check should be be removed + * - HSA_STATUS_ERROR_INVALID_ARGUMENT if devicePtr is NULL + * - This check should be be removed + * - HSA_STATUS_ERROR if hsaKmtAisReadWriteFile(...) does not return success + * - Thunk (userspace) * - repository: rocm-systems * - file: projects/rocr-runtime/libhsakmt/src/ais.c * - functions: hsaKmtAisReadWriteFile(...), * - If the buffer pointer, and io_size combination are invalid, return HSAKMT_STATUS_INVALID_PARAMETER - * - converts buffer pointer + io_size to a buffer handle - * - Does not check for alignment - * - size and status are untouched and return HSAKMT_STATUS_INVALID_PARAMETER if - * - the buffer pointer + size combination are invalid - * - if AisFlags (read/write) are invalid - * - If the kfd ioctl does not return zero, return HSAKMT_STATUS_ERROR - * - kfd (chardev) (kernel) + * - converts buffer pointer + io_size to a buffer handle (no alignment check) + * - size_copied and status are untouched + * - returns + * - HSAKMT_STATUS_INVALID_PARAMETER if + * - the buffer pointer + size combination are invalid + * - if AisFlags (read/write) are invalid + * - HSAKMT_STATUS_ERROR if the kfd ioctl does not return success + * - KFD (chardev) (kernel) * - repository: ROCm * - file: amdgpu/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c * - function: kfd_ioctl_ais(...) - * - returns -EINVAL if op != READ or WRITE - * - returns -EINVAL if fd < 0 - * - returns -EINVAL if unable to lookup device - * - returns -NODEV if AIS is not initialized on the device - * - returns -ESRCH if unable to bind the process ot the device - * - returns -EINVAL if the buffer's domain is not AMDGPU_GEM_DOMAIN_VRAM - * - returns an error if it is unable to pin the buffer object - * - returns the result of the call to kfd_ais_rw_file(...) - * - the value returned by kfd_ioctl_ais() is also stored in the status - * variable provided in the initial call to - * hipAmdFileRead/hipAmdFileWrite - * - kfd (kernel) + * - returns + * - -EINVAL if op != READ or WRITE + * - -EINVAL if fd < 0 + * - -EINVAL if unable to lookup device + * - -NODEV if AIS is not initialized on the device + * - -ESRCH if unable to bind the process to the device + * - -EINVAL if the buffer's domain is not AMDGPU_GEM_DOMAIN_VRAM + * - an error if it is unable to pin the buffer object + * - error is from amdgpu_amdkfd_gpuvm_pin_bo(...) + * - the result of the call to kfd_ais_rw_file(...) + * - the value returned is also written to the status variable + * - KFD (kernel) * - repository: ROCm * - file: amdgpu/drivers/gpu/drm/amd/amdkfd/kfd_ais.c * - function: kfd_ais_rw_file(...) - * - returns -EINVAL if file_offset or size are not page aligned (4K) - * - returns -EBADF unable to lookup the file object - * - returns -ENODEV if unable to lookup the storage device - * - returns -ENODEV if pcie_p2pdma_distance < 0 - * - returns -EINVAL if buffer object domain is not AMDGPU_GEM_DOMAIN_VRAM - * - returns an error if unable to create a sg_table for the buffer object - * - returns -ENOMEM if unbale to init a bvec - * - ** Appears to return -EIO on short reads ** - * - ** Calls dev_dbg on every IO that doesn't return an error ** - * - ** Should not retry on short read/writes ** + * - Calls dev_dbg on every successful IO + * - May want to remove this + * - returns + * - -EINVAL if file_offset or size are not page aligned (4K) + * - This check should be removed /if/ the IO stack checks for alignment. + * - -EBADF unable to lookup the file object + * - -ENODEV if unable to lookup the storage device + * - -ENODEV if pcie_p2pdma_distance < 0 + * - This should be changed to return -EREMOTE + * - -EINVAL if buffer object domain is not AMDGPU_GEM_DOMAIN_VRAM + * - an error if unable to create a sg_table for the buffer object + * - error is from amdgpu_amdkfd_gpuvm_get_sg_table(...) + * - -ENOMEM if unable to init a bvec + * - -EIO on short reads + * - Need to confirm & fix */ int diff --git a/rocfile/test/fastpath.cpp b/rocfile/test/fastpath.cpp index 6791b7cc..d4166dc0 100644 --- a/rocfile/test/fastpath.cpp +++ b/rocfile/test/fastpath.cpp @@ -34,7 +34,7 @@ operator==(const hipAmdFileHandle_t &lhs, const hipAmdFileHandle_t &rhs) } } -// Provide defaut values for variables used in fastpath tests +// Provide default values for variables used in fastpath tests struct FastpathTestBase { const size_t DEFAULT_IO_SIZE{1024 * 1024}; void *const DEFAULT_BUFFER_ADDR{reinterpret_cast(0xCAFE000)}; From 8242e2f0230c84adb15bbcfc887d2ec310339fc3 Mon Sep 17 00:00:00 2001 From: Kurt McMillan Date: Thu, 6 Nov 2025 23:22:32 +0000 Subject: [PATCH 03/10] codespell: Add HSA to the ignore list --- .codespellrc | 1 + 1 file changed, 1 insertion(+) diff --git a/.codespellrc b/.codespellrc index 892f73de..9ab80c80 100644 --- a/.codespellrc +++ b/.codespellrc @@ -3,3 +3,4 @@ [codespell] check-hidden = true skip = ./.git +ignore-words-list = hsa, From b0086a48cd6086421f8c4bd278b2e8252e3d3c5e Mon Sep 17 00:00:00 2001 From: Kurt McMillan Date: Fri, 7 Nov 2025 16:37:25 +0000 Subject: [PATCH 04/10] review: IWYU fixes after rebase --- rocfile/src/backend/fastpath.cpp | 11 ++++++++--- rocfile/src/backend/fastpath.h | 15 +++++++++++++-- rocfile/src/state.cpp | 1 + rocfile/test/fastpath.cpp | 21 +++++++++++++-------- 4 files changed, 35 insertions(+), 13 deletions(-) diff --git a/rocfile/src/backend/fastpath.cpp b/rocfile/src/backend/fastpath.cpp index a9737955..9569a185 100644 --- a/rocfile/src/backend/fastpath.cpp +++ b/rocfile/src/backend/fastpath.cpp @@ -3,12 +3,17 @@ * SPDX-License-Identifier: MIT */ +#include "buffer.h" #include "context.h" #include "fastpath.h" -#include "rocfile.h" +#include "file.h" +#include "hip.h" +#include "io.h" -#include -#include +#include +#include +#include +#include using namespace rocFile; using namespace std; diff --git a/rocfile/src/backend/fastpath.h b/rocfile/src/backend/fastpath.h index c4af215f..c06848ff 100644 --- a/rocfile/src/backend/fastpath.h +++ b/rocfile/src/backend/fastpath.h @@ -6,8 +6,19 @@ #pragma once #include "backend.h" -#include "file.h" -#include "io.h" + +#include +#include + +namespace rocFile { +class IBuffer; +} +namespace rocFile { +class IFile; +} +namespace rocFile { +enum class IoType; +} namespace rocFile { diff --git a/rocfile/src/state.cpp b/rocfile/src/state.cpp index 69b4201c..eaf4d3f9 100644 --- a/rocfile/src/state.cpp +++ b/rocfile/src/state.cpp @@ -8,6 +8,7 @@ #include "batch/batch.h" #include "buffer.h" #include "file.h" +#include "hip.h" #include "state.h" #include "stream.h" diff --git a/rocfile/test/fastpath.cpp b/rocfile/test/fastpath.cpp index d4166dc0..ee62270b 100644 --- a/rocfile/test/fastpath.cpp +++ b/rocfile/test/fastpath.cpp @@ -3,19 +3,24 @@ * SPDX-License-Identifier: MIT */ +#include "backend/fastpath.h" +#include "hip.h" +#include "hipfile-warnings.h" +#include "io.h" #include "mbuffer.h" #include "mfile.h" #include "mhip.h" #include "rocfile-test.h" -#include "backend/fastpath.h" -#include "mountinfo.h" -#include "buffer.h" -#include "file.h" -#include "hip.h" -#include "io.h" - -#include +#include +#include +#include +#include +#include +#include +#include +#include +#include using namespace rocFile; using namespace testing; From 7a36d2e6ecb086616979749bc23900a0452e54b2 Mon Sep 17 00:00:00 2001 From: Kurt McMillan Date: Fri, 7 Nov 2025 18:16:43 +0000 Subject: [PATCH 05/10] review: Fix backend initialization Moving backend initialization out of the DriverState constructor and into DriverState::incRefCount() resulted in the backends getting initialized after rocFileIo() cached an empty backend vector from DriverState::getBackends(). This change relies on compilers supporting thread-stafe static initialization. DriverState's backend vector is now initialized the first time DriverState::getBackends() is called. --- rocfile/src/state.cpp | 20 +++++++++----------- rocfile/src/state.h | 2 +- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/rocfile/src/state.cpp b/rocfile/src/state.cpp index eaf4d3f9..93e99cca 100644 --- a/rocfile/src/state.cpp +++ b/rocfile/src/state.cpp @@ -232,17 +232,6 @@ DriverState::incrRefCount() unique_lock ulock{state_mutex}; ref_count++; - - // If backends are added within DriverState's constructor we cannot test the - // behavior of getHipAmdFileReadPtr() and getHipAmdFileWritePtr() as the - // default DriverState is constructed before main in context.cpp as part of - // RocFileInit's constructor. - if (ref_count == 1 && backends.empty()) { - if (getHipAmdFileReadPtr() && getHipAmdFileWritePtr()) { - backends.emplace_back(new Fastpath{}); - } - backends.emplace_back(new Fallback{}); - } } void @@ -288,6 +277,15 @@ DriverState::ensureInitialized() std::vector> DriverState::getBackends() const { + static bool once = [&]() { + if (getHipAmdFileReadPtr() && getHipAmdFileWritePtr()) { + backends.emplace_back(new Fastpath{}); + } + backends.emplace_back(new Fallback{}); + return true; + }(); + (void)once; + return backends; } } diff --git a/rocfile/src/state.h b/rocfile/src/state.h index fb2d8769..287d3a60 100644 --- a/rocfile/src/state.h +++ b/rocfile/src/state.h @@ -245,7 +245,7 @@ class DriverState { std::unique_ptr stream_map; // Backends available to service IO requests - std::vector> backends; + mutable std::vector> backends; /// Mutex to protect the state mutable std::shared_mutex state_mutex; From 6e60f4907c013f2365a467ad8945725f3e5c4d7e Mon Sep 17 00:00:00 2001 From: Kurt McMillan Date: Fri, 7 Nov 2025 18:21:36 +0000 Subject: [PATCH 06/10] hipfile/examples/aiscp: Improve error messages --- hipfile/examples/aiscp/aiscp.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/hipfile/examples/aiscp/aiscp.cpp b/hipfile/examples/aiscp/aiscp.cpp index 14e1715a..ac670965 100644 --- a/hipfile/examples/aiscp/aiscp.cpp +++ b/hipfile/examples/aiscp/aiscp.cpp @@ -122,14 +122,16 @@ main(int argc, char *argv[]) } nbytes = hipFileRead(src_handle, devbuf, file_size, 0, 0); - if (-1 == nbytes || file_size != static_cast(nbytes)) { - fprintf(stderr, "Could not read from %s (%zd) (%s)\n", src_path, nbytes, strerror(errno)); + if (nbytes < 0 || file_size != static_cast(nbytes)) { + fprintf(stderr, "Could not read from %s (%zd) (%s)\n", src_path, nbytes, + IS_HIPFILE_ERR(nbytes) ? HIPFILE_ERRSTR(nbytes) : strerror(errno)); goto free_devbuf; } nbytes = hipFileWrite(dst_handle, devbuf, file_size, 0, 0); - if (-1 == nbytes || file_size != static_cast(nbytes)) { - fprintf(stderr, "Could not write to %s (%zd) (%s)\n", dst_path, nbytes, strerror(errno)); + if (nbytes < 0 || file_size != static_cast(nbytes)) { + fprintf(stderr, "Could not write to %s (%zd) (%s)\n", src_path, nbytes, + IS_HIPFILE_ERR(nbytes) ? HIPFILE_ERRSTR(nbytes) : strerror(errno)); goto free_devbuf; } From 3a117b794d6046b0983ada983ab021f296aae6a1 Mon Sep 17 00:00:00 2001 From: Kurt McMillan Date: Fri, 7 Nov 2025 21:22:00 +0000 Subject: [PATCH 07/10] review: Fix broken unit tests --- rocfile/test/io.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rocfile/test/io.cpp b/rocfile/test/io.cpp index a95baf29..e89aba89 100644 --- a/rocfile/test/io.cpp +++ b/rocfile/test/io.cpp @@ -121,6 +121,11 @@ struct RocFileIO : public RocFileOpened { StrictMock mhip; StrictMock msys; + // Initialize DriverState::backends + EXPECT_CALL(mhip, hipRuntimeGetVersion); + EXPECT_CALL(mhip, hipGetProcAddress(StrEq("hipAmdFileRead"), _, _, _)); + Context::get()->getBackends(); + expect_buffer_registration(mhip, hipMemoryTypeDevice); Context::get()->registerBuffer(buffer_data.data(), buffer_data.size(), 0); buffer = Context::get()->getBuffer(buffer_data.data()); @@ -181,11 +186,6 @@ struct RocFileFallbackValidation : ::testing::TestWithParam { StrictMock mhip; StrictMock msys; - // Fail the lookup for hipAmdFileRead & hipAmdFileWrite when the - // driver's reference count is bumped. - EXPECT_CALL(mhip, hipRuntimeGetVersion); - EXPECT_CALL(mhip, hipGetProcAddress(StrEq("hipAmdFileRead"), _, _, _)); - assert(rocFileDriverOpen() == ROCFILE_SUCCESS); expect_buffer_registration(mhip, hipMemoryTypeDevice); From aad3ff27cb02ed814a68e605d9e32eadfef82edc Mon Sep 17 00:00:00 2001 From: Kurt McMillan Date: Mon, 10 Nov 2025 18:10:28 +0000 Subject: [PATCH 08/10] review: Address review comments --- rocfile/src/backend/fastpath.cpp | 15 ++++++++------- rocfile/src/backend/fastpath.h | 8 ++++---- rocfile/test/fastpath.cpp | 6 +++--- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/rocfile/src/backend/fastpath.cpp b/rocfile/src/backend/fastpath.cpp index 9569a185..c7cf12f3 100644 --- a/rocfile/src/backend/fastpath.cpp +++ b/rocfile/src/backend/fastpath.cpp @@ -10,18 +10,19 @@ #include "hip.h" #include "io.h" +#include #include #include #include -#include using namespace rocFile; using namespace std; /* The fastpath backend is used when: * - The file has been opened with the O_DIRECT flag - * - IO is 4K aligned (size, file_offset, buffer_offset) - * - This requirement will be relaxed in the future + * - file_offset is 4KiB aligned + * - buffer_offset is 4KiB aligned + * - size if a multiple of 4KiB * - The buffer type is hipMemoryTypeDevice * * When using the fastpath the IO flows through the following @@ -113,8 +114,8 @@ using namespace std; */ int -Fastpath::score(shared_ptr file, shared_ptr buffer, size_t size, off_t file_offset, - off_t buffer_offset) const +Fastpath::score(shared_ptr file, shared_ptr buffer, size_t size, hoff_t file_offset, + hoff_t buffer_offset) const { bool accept_io{true}; @@ -133,8 +134,8 @@ Fastpath::score(shared_ptr file, shared_ptr buffer, size_t size, } ssize_t -Fastpath::io(IoType type, shared_ptr file, shared_ptr buffer, size_t size, off_t file_offset, - off_t buffer_offset) +Fastpath::io(IoType type, shared_ptr file, shared_ptr buffer, size_t size, hoff_t file_offset, + hoff_t buffer_offset) { void *devptr{reinterpret_cast(reinterpret_cast(buffer->getBuffer()) + buffer_offset)}; hipAmdFileHandle_t handle{}; diff --git a/rocfile/src/backend/fastpath.h b/rocfile/src/backend/fastpath.h index c06848ff..37fe3671 100644 --- a/rocfile/src/backend/fastpath.h +++ b/rocfile/src/backend/fastpath.h @@ -7,8 +7,8 @@ #include "backend.h" -#include #include +#include namespace rocFile { class IBuffer; @@ -25,11 +25,11 @@ namespace rocFile { struct Fastpath : public Backend { virtual ~Fastpath() override = default; - int score(std::shared_ptr file, std::shared_ptr buffer, size_t size, off_t file_offset, - off_t buffer_offset) const override; + int score(std::shared_ptr file, std::shared_ptr buffer, size_t size, hoff_t file_offset, + hoff_t buffer_offset) const override; ssize_t io(IoType type, std::shared_ptr file, std::shared_ptr buffer, size_t size, - off_t file_offset, off_t buffer_offset) override; + hoff_t file_offset, hoff_t buffer_offset) override; }; } diff --git a/rocfile/test/fastpath.cpp b/rocfile/test/fastpath.cpp index ee62270b..e74f7b6e 100644 --- a/rocfile/test/fastpath.cpp +++ b/rocfile/test/fastpath.cpp @@ -13,7 +13,7 @@ #include "rocfile-test.h" #include -#include +#include #include #include #include @@ -303,7 +303,7 @@ TEST_P(FastpathIoParam, IoReturnsBytesTransferredShort) nbytes); } -// Ensure hip errors thrown by Hip::hipAmdFileRead/Hip::hipAmdFileWtite are not masked +// Ensure hip errors thrown by Hip::hipAmdFileRead/Hip::hipAmdFileWrite are not masked TEST_P(FastpathIoParam, IoDoesNotMaskHipRuntimeError) { StrictMock mhip; @@ -326,7 +326,7 @@ TEST_P(FastpathIoParam, IoDoesNotMaskHipRuntimeError) Hip::RuntimeError); } -// Ensure hip errors thrown by Hip::hipAmdFileRead/Hip::hipAmdFileWtite are not masked +// Ensure hip errors thrown by Hip::hipAmdFileRead/Hip::hipAmdFileWrite are not masked TEST_P(FastpathIoParam, IoDoesNotMaskSystemError) { StrictMock mhip; From ec2454400abd27c4a9f77fbcad01aa01e3d26667 Mon Sep 17 00:00:00 2001 From: Kurt McMillan Date: Mon, 10 Nov 2025 18:21:17 +0000 Subject: [PATCH 09/10] review: IWYU fix --- rocfile/src/backend/fastpath.h | 1 + 1 file changed, 1 insertion(+) diff --git a/rocfile/src/backend/fastpath.h b/rocfile/src/backend/fastpath.h index 37fe3671..91b9863f 100644 --- a/rocfile/src/backend/fastpath.h +++ b/rocfile/src/backend/fastpath.h @@ -6,6 +6,7 @@ #pragma once #include "backend.h" +#include "hipfile-types.h" #include #include From ea0381a7b24fcb9a632fc9a119b20a09c35e5a63 Mon Sep 17 00:00:00 2001 From: Kurt McMillan Date: Mon, 10 Nov 2025 18:30:39 +0000 Subject: [PATCH 10/10] review: Fix if --> is --- rocfile/src/backend/fastpath.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rocfile/src/backend/fastpath.cpp b/rocfile/src/backend/fastpath.cpp index c7cf12f3..74c5dd80 100644 --- a/rocfile/src/backend/fastpath.cpp +++ b/rocfile/src/backend/fastpath.cpp @@ -22,7 +22,7 @@ using namespace std; * - The file has been opened with the O_DIRECT flag * - file_offset is 4KiB aligned * - buffer_offset is 4KiB aligned - * - size if a multiple of 4KiB + * - size is a multiple of 4KiB * - The buffer type is hipMemoryTypeDevice * * When using the fastpath the IO flows through the following