From 2f4232816402f7856d9e5fcd266b74fef94bcc69 Mon Sep 17 00:00:00 2001 From: Samuel Le Meur-Diebolt Date: Wed, 30 Sep 2026 13:51:17 +0100 Subject: [PATCH 1/6] fix(storage): fall back when the kernel does not report STATX_DIOALIGN queryAlignment() threw when statx came back without STATX_DIOALIGN, so storage could not open a file on a pre-6.1 kernel or a filesystem that does not implement the query (BTRFS, among others). It now falls back to stx_blksize, or the page size when the kernel did not report that either, and warns. The page size is a multiple of every sector size such a filesystem uses, so a buffer aligned to it satisfies any of them. Moving the statx struct and the resolution into linux_statx.h lets linux_alignment_check drive the mask directly, since a modern kernel cannot produce the one this fix is about. Writing that check turned up a division by zero: a filesystem that reports the query but zeroes dio_offset_align (tmpfs) had stx_blksize % stx_dio_offset_align evaluated with a zero divisor whenever stx_blksize was larger. The zero case is now passed through for the caller to reject, as before. --- echoframe/cpp/libs/Storage/CMakeLists.txt | 11 +- echoframe/cpp/libs/Storage/README.md | 5 +- .../Storage/src/storage/Linux/LinuxFileIO.h | 15 +- .../src/storage/Linux/LinuxFileIO.t.hpp | 90 +++-------- .../Storage/src/storage/Linux/linux_statx.h | 126 +++++++++++++++ .../Storage/tests/linux_alignment_check.cpp | 146 ++++++++++++++++++ 6 files changed, 313 insertions(+), 80 deletions(-) create mode 100644 echoframe/cpp/libs/Storage/src/storage/Linux/linux_statx.h create mode 100644 echoframe/cpp/libs/Storage/tests/linux_alignment_check.cpp diff --git a/echoframe/cpp/libs/Storage/CMakeLists.txt b/echoframe/cpp/libs/Storage/CMakeLists.txt index 2a32a22..c30b8f9 100644 --- a/echoframe/cpp/libs/Storage/CMakeLists.txt +++ b/echoframe/cpp/libs/Storage/CMakeLists.txt @@ -49,8 +49,11 @@ endif () # with and without the deferred-write knob. Drives a real # Handler through a real file, which is where the counting # lives, so this one needs elevation on Windows. +# linux_alignment_check how a statx(STATX_DIOALIGN) result becomes the O_DIRECT +# offset/length and buffer-address alignments, including the +# no-STATX_DIOALIGN fallback. Linux only; no file I/O. # -# Both exit non-zero on failure. +# All exit non-zero on failure. include(CTest) foreach (check write_stats_check write_count_check) @@ -58,3 +61,9 @@ foreach (check write_stats_check write_count_check) target_link_libraries(${check} PRIVATE echoframe_storage) add_test(NAME ${check} COMMAND ${check}) endforeach () + +if (NOT WIN32) + add_executable(linux_alignment_check tests/linux_alignment_check.cpp) + target_link_libraries(linux_alignment_check PRIVATE echoframe_storage) + add_test(NAME linux_alignment_check COMMAND linux_alignment_check) +endif () diff --git a/echoframe/cpp/libs/Storage/README.md b/echoframe/cpp/libs/Storage/README.md index a90ecb7..fbebfa7 100644 --- a/echoframe/cpp/libs/Storage/README.md +++ b/echoframe/cpp/libs/Storage/README.md @@ -27,7 +27,10 @@ than plain synchronous writes: buffers and the header are sector-size aligned (padding is added when a buffer is not a multiple of the sector size, and recorded in the header). The alignment is queried at open time — `queryAlignment()` — from the physical - sector size on Windows and from `statx(STATX_DIOALIGN)` on Linux. + sector size on Windows and from `statx(STATX_DIOALIGN)` on Linux. A Linux + kernel that does not report `STATX_DIOALIGN` (pre-6.1, or a filesystem that + does not implement the query) falls back to a conservative page-sized + alignment and logs a warning. - **Preallocation + privileged extension** — with `preallocateFullFile` the file is sized up front. On Windows extension uses `SE_MANAGE_VOLUME_NAME` (`SeManageVolumePrivilege`) to grow files without zero-filling; on Linux it diff --git a/echoframe/cpp/libs/Storage/src/storage/Linux/LinuxFileIO.h b/echoframe/cpp/libs/Storage/src/storage/Linux/LinuxFileIO.h index 09b1d40..0e3a10a 100644 --- a/echoframe/cpp/libs/Storage/src/storage/Linux/LinuxFileIO.h +++ b/echoframe/cpp/libs/Storage/src/storage/Linux/LinuxFileIO.h @@ -4,9 +4,10 @@ // // The sector size used for offset/length rounding (adjustToSectorSize()) and // the buffer-address alignment O_DIRECT requires are queried at runtime via -// statx(STATX_DIOALIGN) in queryAlignment(); the constructor throws if the -// kernel/filesystem can't report a real value. The file header -// (writeToHeader()) is allocated against these queried values. +// statx(STATX_DIOALIGN) in queryAlignment(); a kernel that does not report it +// falls back to a conservative page-sized alignment, and the constructor throws +// only if the filesystem reports the query but does not enforce alignment. The +// file header (writeToHeader()) is allocated against these queried values. // #ifndef CUBE_STORAGE_LINUXFILEIO_H @@ -44,7 +45,9 @@ class LinuxFileIO { /** * @brief Query the file's actual O_DIRECT offset/length alignment * (mSectorSize) and buffer-address alignment (mMemAlign) via - * statx(STATX_DIOALIGN). Throws if either value can't be determined. + * statx(STATX_DIOALIGN). Falls back to the page size for both when the + * kernel does not report the query. Throws if the filesystem reports it but + * does not enforce alignment. */ void queryAlignment(); @@ -80,8 +83,8 @@ class LinuxFileIO { std::vector mBlockOffset; // element offset into buffer for each block // O_DIRECT alignment requirements. Set by queryAlignment(), called - // early in the constructor (which throws if either can't be - // determined) -- never read at their zero-initialized value. + // early in the constructor, which either resolves them or throws -- + // never read at their zero-initialized value. uint64_t mSectorSize{0}; // required offset/length alignment uint64_t mMemAlign{0}; // required buffer-address alignment diff --git a/echoframe/cpp/libs/Storage/src/storage/Linux/LinuxFileIO.t.hpp b/echoframe/cpp/libs/Storage/src/storage/Linux/LinuxFileIO.t.hpp index 889c018..1e0c94c 100644 --- a/echoframe/cpp/libs/Storage/src/storage/Linux/LinuxFileIO.t.hpp +++ b/echoframe/cpp/libs/Storage/src/storage/Linux/LinuxFileIO.t.hpp @@ -24,58 +24,10 @@ #include #include "../error.h" +#include "linux_statx.h" namespace Storage { -// The kernel's statx layout, transcribed. glibc's struct statx and -// cannot be included together (same tag, different members), -// and neither reliably declares stx_dio_mem_align/stx_dio_offset_align. The -// raw syscall is used rather than glibc's wrapper, which is typed against -// glibc's own struct. -namespace { - -struct EfStatxTimestamp { - int64_t tv_sec; - uint32_t tv_nsec; - int32_t __reserved; -}; - -struct EfStatx { - uint32_t stx_mask; - uint32_t stx_blksize; - uint64_t stx_attributes; - uint32_t stx_nlink; - uint32_t stx_uid; - uint32_t stx_gid; - uint16_t stx_mode; - uint16_t __spare0[1]; - uint64_t stx_ino; - uint64_t stx_size; - uint64_t stx_blocks; - uint64_t stx_attributes_mask; - EfStatxTimestamp stx_atime; - EfStatxTimestamp stx_btime; - EfStatxTimestamp stx_ctime; - EfStatxTimestamp stx_mtime; - uint32_t stx_rdev_major; - uint32_t stx_rdev_minor; - uint32_t stx_dev_major; - uint32_t stx_dev_minor; - uint64_t stx_mnt_id; - uint32_t stx_dio_mem_align; - uint32_t stx_dio_offset_align; - uint64_t __spare3[12]; // reserved space for fields added after these -}; - -// statx(2) takes no buffer-length argument: the kernel always writes a fixed -// 256-byte struct. A smaller struct here would be overflowed by the syscall. -static_assert(sizeof(EfStatx) == 256, "EfStatx must match the kernel's fixed statx(2) ABI size"); - -constexpr uint32_t kEfStatxDioalign = 0x00002000U; // STATX_DIOALIGN -constexpr uint32_t kEfStatxBasicStats = 0x000007ffU; // STATX_BASIC_STATS (covers stx_blksize) - -} // namespace - // --------------------------------------------------------------------------- // Constructor // --------------------------------------------------------------------------- @@ -315,41 +267,35 @@ void LinuxFileIO::extendFile(int nBuffers) { // Block layout // --------------------------------------------------------------------------- -// Query the file's O_DIRECT alignment via statx(STATX_DIOALIGN). Offset/length -// alignment (stx_dio_offset_align) and buffer-address alignment -// (stx_dio_mem_align) are reported separately and are not guaranteed equal. -// Throws if either is unavailable -- a kernel without STATX_DIOALIGN, or a -// filesystem that accepts O_DIRECT without enforcing alignment (e.g. tmpfs). +// Query the file's O_DIRECT alignment via statx(STATX_DIOALIGN) and store it. +// A kernel that does not report the query falls back to a conservative +// page-sized alignment (resolveDioAlignment) and warns. Throws if the +// filesystem reports the query but zeroes either value, i.e. accepts O_DIRECT +// without enforcing alignment (e.g. tmpfs). template void LinuxFileIO::queryAlignment() { - EfStatx stx {}; + detail::EfStatx stx {}; if (syscall(SYS_statx, mFd, "", AT_EMPTY_PATH, - kEfStatxDioalign | kEfStatxBasicStats, &stx) != 0) + detail::kEfStatxDioalign | detail::kEfStatxBasicStats, + &stx) != 0) throw storageException( std::string("queryAlignment: statx failed: ") + strerror(errno) + "\n"); - if (!(stx.stx_mask & kEfStatxDioalign)) - throw storageException( - "queryAlignment: kernel does not report STATX_DIOALIGN " - "(requires Linux 6.1+)\n"); - if (stx.stx_dio_offset_align == 0 || stx.stx_dio_mem_align == 0) + + const detail::DioAlignment align = detail::resolveDioAlignment(stx); + if (!align.reported && logAtLeast(kLogNormal)) + std::cerr << "queryAlignment: kernel does not report STATX_DIOALIGN; " + "using conservative " + << align.sectorSize << "-byte O_DIRECT alignment.\n"; + if (align.sectorSize == 0 || align.memAlign == 0) throw storageException( "queryAlignment: filesystem does not support/enforce O_DIRECT " "alignment (dio_offset_align/dio_mem_align reported as 0 -- " "direct I/O is not actually honored on this filesystem, e.g. " "tmpfs)\n"); - // dio_offset_align is only the required minimum: on a 512e drive (512 B - // logical, 4096 B physical) it is 512, and writing at that alignment forces - // a device-side read-modify-write per block. Prefer stx_blksize (typically - // the physical sector) when it is a whole multiple of dio_offset_align, so - // writes land on physical-sector boundaries; fall back to dio_offset_align. - mSectorSize = stx.stx_dio_offset_align; - if ((stx.stx_mask & kEfStatxBasicStats) && stx.stx_blksize > mSectorSize && - stx.stx_blksize % stx.stx_dio_offset_align == 0) { - mSectorSize = stx.stx_blksize; - } - mMemAlign = stx.stx_dio_mem_align; + mSectorSize = align.sectorSize; + mMemAlign = align.memAlign; mHeaderSize = computeHeaderSize(); } diff --git a/echoframe/cpp/libs/Storage/src/storage/Linux/linux_statx.h b/echoframe/cpp/libs/Storage/src/storage/Linux/linux_statx.h new file mode 100644 index 0000000..6217056 --- /dev/null +++ b/echoframe/cpp/libs/Storage/src/storage/Linux/linux_statx.h @@ -0,0 +1,126 @@ +// +// O_DIRECT alignment reported by statx(STATX_DIOALIGN), and how to resolve it +// into the two alignments EchoFrame needs. +// +// Split out of LinuxFileIO.t.hpp so the resolution can be exercised directly +// from tests/linux_alignment_check.cpp. It is header-only and safe to include +// from more than one translation unit. +// + +#ifndef CUBE_STORAGE_LINUX_STATX_H +#define CUBE_STORAGE_LINUX_STATX_H + +#include + +#include + +namespace Storage { +namespace detail { + +// The kernel's statx layout, transcribed. glibc's struct statx and +// cannot be included together (same tag, different members), +// and neither reliably declares stx_dio_mem_align/stx_dio_offset_align. The +// raw syscall is used rather than glibc's wrapper, which is typed against +// glibc's own struct. +struct EfStatxTimestamp { + int64_t tv_sec; + uint32_t tv_nsec; + int32_t __reserved; +}; + +struct EfStatx { + uint32_t stx_mask; + uint32_t stx_blksize; + uint64_t stx_attributes; + uint32_t stx_nlink; + uint32_t stx_uid; + uint32_t stx_gid; + uint16_t stx_mode; + uint16_t __spare0[1]; + uint64_t stx_ino; + uint64_t stx_size; + uint64_t stx_blocks; + uint64_t stx_attributes_mask; + EfStatxTimestamp stx_atime; + EfStatxTimestamp stx_btime; + EfStatxTimestamp stx_ctime; + EfStatxTimestamp stx_mtime; + uint32_t stx_rdev_major; + uint32_t stx_rdev_minor; + uint32_t stx_dev_major; + uint32_t stx_dev_minor; + uint64_t stx_mnt_id; + uint32_t stx_dio_mem_align; + uint32_t stx_dio_offset_align; + uint64_t __spare3[12]; // reserved space for fields added after these +}; + +// statx(2) takes no buffer-length argument: the kernel always writes a fixed +// 256-byte struct. A smaller struct here would be overflowed by the syscall. +static_assert(sizeof(EfStatx) == 256, + "EfStatx must match the kernel's fixed statx(2) ABI size"); + +constexpr uint32_t kEfStatxDioalign = 0x00002000U; // STATX_DIOALIGN +constexpr uint32_t kEfStatxBasicStats = + 0x000007ffU; // STATX_BASIC_STATS (covers stx_blksize) + +/// @return The runtime page size, which bounds any O_DIRECT buffer-address +/// alignment. Falls back to 4096 if the query fails, which is the smallest +/// page size Linux allows on the platforms EchoFrame targets. +inline uint64_t pageSize() { + const long ps = sysconf(_SC_PAGESIZE); + return ps > 0 ? static_cast(ps) : 4096; +} + +/// The two alignments an O_DIRECT write has to satisfy, and whether the +/// filesystem reported them. +struct DioAlignment { + uint64_t sectorSize; // required offset/length alignment + uint64_t memAlign; // required buffer-address alignment + bool reported; // false when the kernel did not report STATX_DIOALIGN +}; + +/// Resolve a statx result into the O_DIRECT offset/length and buffer-address +/// alignments. +/// +/// Offset/length alignment (stx_dio_offset_align) and buffer-address alignment +/// (stx_dio_mem_align) are reported separately and are not guaranteed equal. +/// Both come back as 0 from a filesystem that accepts O_DIRECT without +/// enforcing it (e.g. tmpfs); the caller is expected to reject that, so they +/// are passed through rather than replaced by the conservative fallback. +/// +/// A kernel that does not report STATX_DIOALIGN at all -- before Linux 6.1, or +/// a filesystem that does not implement the query -- yields `reported == false` +/// and a conservative alignment: stx_blksize as the sector size when the +/// kernel reported it, else the page size, and the page size for the buffer +/// address. The page size is a multiple of every sector size such a +/// filesystem uses, so a buffer aligned to it satisfies any of them. +inline DioAlignment resolveDioAlignment(const EfStatx &stx) { + if (!(stx.stx_mask & kEfStatxDioalign)) { + const uint64_t fallback = + (stx.stx_mask & kEfStatxBasicStats) && stx.stx_blksize > 0 + ? stx.stx_blksize + : pageSize(); + return {fallback, pageSize(), false}; + } + + if (stx.stx_dio_offset_align == 0 || stx.stx_dio_mem_align == 0) + return {0, 0, true}; + + // dio_offset_align is only the required minimum: on a 512e drive (512 B + // logical, 4096 B physical) it is 512, and writing at that alignment forces + // a device-side read-modify-write per block. Prefer stx_blksize (typically + // the physical sector) when it is a whole multiple of dio_offset_align, so + // writes land on physical-sector boundaries; fall back to dio_offset_align. + uint64_t sectorSize = stx.stx_dio_offset_align; + if ((stx.stx_mask & kEfStatxBasicStats) && stx.stx_blksize > sectorSize && + stx.stx_blksize % stx.stx_dio_offset_align == 0) { + sectorSize = stx.stx_blksize; + } + return {sectorSize, stx.stx_dio_mem_align, true}; +} + +} // namespace detail +} // namespace Storage + +#endif // CUBE_STORAGE_LINUX_STATX_H diff --git a/echoframe/cpp/libs/Storage/tests/linux_alignment_check.cpp b/echoframe/cpp/libs/Storage/tests/linux_alignment_check.cpp new file mode 100644 index 0000000..67e4df9 --- /dev/null +++ b/echoframe/cpp/libs/Storage/tests/linux_alignment_check.cpp @@ -0,0 +1,146 @@ +// Checks how a statx(STATX_DIOALIGN) result is resolved into the O_DIRECT +// offset/length and buffer-address alignments. +// +// The case that matters is a mask without STATX_DIOALIGN, which a pre-6.1 +// kernel or a filesystem that does not implement the query returns: storage +// used to throw there, so it could not write at all on such a host. That mask +// cannot be produced by a real filesystem on a modern kernel, hence driving +// resolveDioAlignment() directly. +// +// c++ -std=c++17 -I ..\src linux_alignment_check.cpp +// +// Linux only: it includes the Linux backend's statx header. No MATLAB, no GPU, +// no file I/O. Exits non-zero on failure. + +#include +#include + +#include "storage/Linux/linux_statx.h" + +static int failures = 0; + +static void expect(const char *what, uint64_t got, uint64_t want) { + if (got != want) { + std::printf(" FAIL %-46s got %llu, want %llu\n", what, + static_cast(got), + static_cast(want)); + ++failures; + } else { + std::printf(" ok %-46s %llu\n", what, + static_cast(got)); + } +} + +static void expectTrue(const char *what, bool got) { + if (!got) { + std::printf(" FAIL %-46s false, want true\n", what); + ++failures; + } else { + std::printf(" ok %-46s true\n", what); + } +} + +using Storage::detail::EfStatx; +using Storage::detail::kEfStatxBasicStats; +using Storage::detail::kEfStatxDioalign; +using Storage::detail::resolveDioAlignment; + +int main() { + const uint64_t page = Storage::detail::pageSize(); + expectTrue("page size is a power of two", (page & (page - 1)) == 0); + expectTrue("page size is at least 4096", page >= 4096); + + // A 512e drive: 512 B minimum offset/length alignment, but the block size + // is the 4096 B physical sector, which is where writes should land. + { + EfStatx stx{}; + stx.stx_mask = kEfStatxDioalign | kEfStatxBasicStats; + stx.stx_blksize = 4096; + stx.stx_dio_offset_align = 512; + stx.stx_dio_mem_align = 512; + const auto a = resolveDioAlignment(stx); + expectTrue("512e: reported", a.reported); + expect("512e: sector size prefers the physical sector", a.sectorSize, + 4096); + expect("512e: mem align is taken verbatim", a.memAlign, 512); + } + + // A block size that is not a multiple of the minimum alignment is not a + // usable sector size, so the minimum stands. + { + EfStatx stx{}; + stx.stx_mask = kEfStatxDioalign | kEfStatxBasicStats; + stx.stx_blksize = 3000; + stx.stx_dio_offset_align = 512; + stx.stx_dio_mem_align = 512; + const auto a = resolveDioAlignment(stx); + expect("non-multiple block size keeps the minimum", a.sectorSize, 512); + } + + // STATX_BASIC_STABS absent, so stx_blksize cannot be trusted. + { + EfStatx stx{}; + stx.stx_mask = kEfStatxDioalign; + stx.stx_blksize = 4096; // must be ignored + stx.stx_dio_offset_align = 512; + stx.stx_dio_mem_align = 4096; + const auto a = resolveDioAlignment(stx); + expect("no basic stats: sector size is the minimum", a.sectorSize, 512); + } + + // tmpfs: reports the query but does not enforce it. The caller rejects + // these on the zero, which is why they must come through as 0 rather than + // as the conservative fallback. + { + EfStatx stx{}; + stx.stx_mask = kEfStatxDioalign | kEfStatxBasicStats; + stx.stx_blksize = 4096; + stx.stx_dio_offset_align = 0; + stx.stx_dio_mem_align = 0; + const auto a = resolveDioAlignment(stx); + expectTrue("tmpfs: reported", a.reported); + expect("tmpfs: sector size is 0", a.sectorSize, 0); + expect("tmpfs: mem align is 0", a.memAlign, 0); + } + + // The regression: no STATX_DIOALIGN, but a block size is available. + { + EfStatx stx{}; + stx.stx_mask = kEfStatxBasicStats; + stx.stx_blksize = 512; + const auto a = resolveDioAlignment(stx); + expectTrue("no STATX_DIOALIGN: not reported", !a.reported); + expect("no STATX_DIOALIGN: sector size is the block size", a.sectorSize, + 512); + expectTrue("no STATX_DIOALIGN: mem align covers the block size", + a.memAlign >= a.sectorSize); + } + + // The regression with nothing to go on at all: the page size, which is a + // multiple of every sector size these filesystems use. + { + EfStatx stx{}; + stx.stx_mask = kEfStatxBasicStats; + stx.stx_blksize = 0; // must be ignored + const auto a = resolveDioAlignment(stx); + expectTrue("empty mask: not reported", !a.reported); + expect("empty mask: sector size is the page size", a.sectorSize, page); + expect("empty mask: mem align is the page size", a.memAlign, page); + } + + { + EfStatx stx{}; + stx.stx_mask = 0; + const auto a = resolveDioAlignment(stx); + expect("no mask at all: sector size is the page size", a.sectorSize, + page); + expect("no mask at all: mem align is the page size", a.memAlign, page); + } + + if (failures) { + std::printf("\n%d check(s) failed\n", failures); + return 1; + } + std::printf("\nall checks passed\n"); + return 0; +} From 9c0102ea5d1da12e0b1cb2e1d503aecc4cdea60d Mon Sep 17 00:00:00 2001 From: Samuel Le Meur-Diebolt Date: Wed, 30 Sep 2026 16:55:13 +0100 Subject: [PATCH 2/6] fix(storage): use page-sized DIOALIGN fallback --- .../libs/Storage/src/storage/Linux/linux_statx.h | 13 ++++--------- .../libs/Storage/tests/linux_alignment_check.cpp | 14 +++++++------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/echoframe/cpp/libs/Storage/src/storage/Linux/linux_statx.h b/echoframe/cpp/libs/Storage/src/storage/Linux/linux_statx.h index 6217056..40acb5b 100644 --- a/echoframe/cpp/libs/Storage/src/storage/Linux/linux_statx.h +++ b/echoframe/cpp/libs/Storage/src/storage/Linux/linux_statx.h @@ -91,17 +91,12 @@ struct DioAlignment { /// /// A kernel that does not report STATX_DIOALIGN at all -- before Linux 6.1, or /// a filesystem that does not implement the query -- yields `reported == false` -/// and a conservative alignment: stx_blksize as the sector size when the -/// kernel reported it, else the page size, and the page size for the buffer -/// address. The page size is a multiple of every sector size such a -/// filesystem uses, so a buffer aligned to it satisfies any of them. +/// and a conservative page-sized alignment for both the file offset/length and +/// the buffer address. inline DioAlignment resolveDioAlignment(const EfStatx &stx) { if (!(stx.stx_mask & kEfStatxDioalign)) { - const uint64_t fallback = - (stx.stx_mask & kEfStatxBasicStats) && stx.stx_blksize > 0 - ? stx.stx_blksize - : pageSize(); - return {fallback, pageSize(), false}; + const uint64_t fallback = pageSize(); + return {fallback, fallback, false}; } if (stx.stx_dio_offset_align == 0 || stx.stx_dio_mem_align == 0) diff --git a/echoframe/cpp/libs/Storage/tests/linux_alignment_check.cpp b/echoframe/cpp/libs/Storage/tests/linux_alignment_check.cpp index 67e4df9..ed82b4f 100644 --- a/echoframe/cpp/libs/Storage/tests/linux_alignment_check.cpp +++ b/echoframe/cpp/libs/Storage/tests/linux_alignment_check.cpp @@ -103,21 +103,21 @@ int main() { expect("tmpfs: mem align is 0", a.memAlign, 0); } - // The regression: no STATX_DIOALIGN, but a block size is available. + // The regression: no STATX_DIOALIGN falls back to the page size even when + // a smaller block size is available. { EfStatx stx{}; stx.stx_mask = kEfStatxBasicStats; stx.stx_blksize = 512; const auto a = resolveDioAlignment(stx); expectTrue("no STATX_DIOALIGN: not reported", !a.reported); - expect("no STATX_DIOALIGN: sector size is the block size", a.sectorSize, - 512); - expectTrue("no STATX_DIOALIGN: mem align covers the block size", - a.memAlign >= a.sectorSize); + expect("no STATX_DIOALIGN: sector size is the page size", a.sectorSize, + page); + expect("no STATX_DIOALIGN: mem align is the page size", a.memAlign, + page); } - // The regression with nothing to go on at all: the page size, which is a - // multiple of every sector size these filesystems use. + // The regression with nothing to go on at all: the page size is still used. { EfStatx stx{}; stx.stx_mask = kEfStatxBasicStats; From d482af4a400ed3fef9b03dfa6b4e985b0ef240ff Mon Sep 17 00:00:00 2001 From: Samuel Le Meur-Diebolt Date: Wed, 7 Oct 2026 18:16:33 +0100 Subject: [PATCH 3/6] fix(matlab): correct BS67BW Fourier frequencies --- .../core/imaging/echoframe_setup_fourier.m | 3 +- .../core/verasonics/get_system_parameters.m | 3 +- echoframe/matlab/tests/verify_core_headless.m | 54 +++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/echoframe/matlab/core/imaging/echoframe_setup_fourier.m b/echoframe/matlab/core/imaging/echoframe_setup_fourier.m index f460fd3..03f3551 100644 --- a/echoframe/matlab/core/imaging/echoframe_setup_fourier.m +++ b/echoframe/matlab/core/imaging/echoframe_setup_fourier.m @@ -134,7 +134,8 @@ case 'BS67BW' Nz = single(ReceiveSpec.nSamples * 2); freq_scale = 2; - lower = Nz/2; + % The negative-frequency half starts after the positive half in FFT order. + lower = Nz/2 + 1; freq_window = lower:(lower + Nz/4 - 1); freq_mapping = 1:Nz/4; freq_axis_shift = false; diff --git a/echoframe/matlab/core/verasonics/get_system_parameters.m b/echoframe/matlab/core/verasonics/get_system_parameters.m index 3c1f12d..64566e9 100644 --- a/echoframe/matlab/core/verasonics/get_system_parameters.m +++ b/echoframe/matlab/core/verasonics/get_system_parameters.m @@ -14,7 +14,7 @@ % % nChannels 128 or 256, from Resource.Parameters.numRcvChannels % and Trans.numelements -% Fs Trans.frequency * 4 (Hz) +% Fs pre-quadrature sample rate: 4*Fc, or 4*Fc/3 for BS67BW (Hz) % samplesPerWavelength from samplingMode: 1 (BS50BW), 4/3 (BS67BW), % 2 (BS100BW), 4 (NS200BW) % @@ -52,6 +52,7 @@ case 'BS50BW' ReceiveSpec.samplesPerWavelength = 1; case 'BS67BW' + ReceiveSpec.Fs = Trans.frequency * (4/3) * 1e6; ReceiveSpec.samplesPerWavelength = 4/3; case 'BS100BW' ReceiveSpec.samplesPerWavelength = 2; diff --git a/echoframe/matlab/tests/verify_core_headless.m b/echoframe/matlab/tests/verify_core_headless.m index f2210ce..cbaeaf0 100644 --- a/echoframe/matlab/tests/verify_core_headless.m +++ b/echoframe/matlab/tests/verify_core_headless.m @@ -156,6 +156,8 @@ assert(threw, 'validate_structs accepted a ReceiveSpec missing samplingMode'); fprintf(' missing required field rejected OK\n'); +local_verify_fourier_sampling(); + %% ------------------------------------------------------- 5. cropped frames fprintf('\n=== stored_frame_size (cropped storage geometry) ===\n'); @@ -490,3 +492,55 @@ PDISpec.cropPDI = logical(false); PDISpec.svdMethod = 'Covariance'; end + +function local_verify_fourier_sampling() +% A known transmit delay must be cancelled at the dominant receive-tone bin. +modes = {'BS50BW', 'BS67BW', 'BS100BW', 'NS200BW'}; +rateFactors = [4, 4/3, 4, 4]; +P = struct('pitch', 300e-6, 'nElements', 8); +T = struct('c0', 1540, 'steer', 0, 'apodization', ones(8,1)); +Resource = struct('Parameters', struct('numRcvChannels', 128)); +for fc = [1e6, 31.25e6] + Trans = struct('frequency', fc/1e6, 'numelements', 8); + for n = [128, 512] + rec = struct('c0', 1540, 'nx', 128, 'nz', n/2); + for m = 1:numel(modes) + r = struct('samplingMode', modes{m}, 'nBuffers', 1, ... + 'nSamples', n, 'nSamplesIQ', n/2, 'nTransmissions', 1); + [~, ~, ~, r] = get_system_parameters(Resource, Trans, T, r); + assert(abs(r.Fs/fc - rateFactors(m)) < 1e-6, ... + 'Wrong pre-quadrature sample rate for %s', modes{m}); + switch modes{m} + case 'BS50BW' + starts = 2*(0:n/2-1)/fc; + times = reshape([starts; starts + 1/(4*fc)], [], 1); + case 'BS100BW' + starts = (0:n/2-1)/fc; + times = reshape([starts; starts + 1/(4*fc)], [], 1); + case 'BS67BW' + times = (0:n-1)'*3/(4*fc); + case 'NS200BW' + times = (0:n-1)'/(4*fc); + end + for shift = [0, 1/(8*fc)] + T.transmitDelays = shift * ones(8,1); + s = echoframe_setup_fourier(P, T, r, rec); + assert(numel(s.frequencyAxis) == n/2); + assert(all(s.delayIndices(:) >= 0 & s.delayIndices(:) < n/2)); + for tone = fc*[0.875, 1, 1.125] + a = int16(round(16000*cos(2*pi*tone*times))); + b = int16(round(16000*cos(2*pi*tone*(times-shift)))); + a = fft(complex(single(a(1:2:end)), -single(a(2:2:end)))); + b = fft(complex(single(b(1:2:end)), -single(b(2:2:end)))); + [~, k] = max(abs(a)); + rotation = exp(1i*double(s.frequencyAxis(k))*double(s.planewaveDelays(1,2))); + assert(abs(double(b(k))/double(a(k))*rotation - 1) < 2e-4, ... + 'Fourier delay correction failed: %s, Fc=%g, n=%d, tone=%g', ... + modes{m}, fc, n, tone); + end + end + end + end +end +fprintf(' all four sampling modes cancel known tone delays OK\n'); +end From ec2e99fe3408f97da6bbeb1e32b9e5262fa667ba Mon Sep 17 00:00:00 2001 From: Arber Demi Date: Wed, 11 Nov 2026 15:01:19 +0100 Subject: [PATCH 4/6] fix(beamformer): insert Fourier zero-padding at the spectrum wrap point The pad kernel split the spectrum at half the padding instead of half the data length, so extraVoxelsZ/X other than 0 or the data size gave striped images in every sampling mode. --- .../fourier_imaging/fourier_imaging_kernels.cu | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/echoframe/cpp/src/beamformer/fourier_imaging/fourier_imaging_kernels.cu b/echoframe/cpp/src/beamformer/fourier_imaging/fourier_imaging_kernels.cu index 5f34c54..dbf7335 100644 --- a/echoframe/cpp/src/beamformer/fourier_imaging/fourier_imaging_kernels.cu +++ b/echoframe/cpp/src/beamformer/fourier_imaging/fourier_imaging_kernels.cu @@ -166,12 +166,13 @@ __global__ void interpFastTimeCompoundAndPad_kernel( ind2subV2(sub, idx, stridesInds, nDim); // padding - // put the zero-padding in the middle of the frequency field - // this is needed for the IFFT later - int xBounds = (nz - receiveSpec.nFastTimeSamples) / 2; - int x = (sub[0] < xBounds) ? sub[0] : sub[0] + xBounds * 2; - int zBounds = (nx - receiveSpec.nChannels) / 2; - int z = (sub[1] < zBounds) ? sub[1] : sub[1] + zBounds * 2; + // put the zero-padding at the wrap point of the (DC-first) frequency + // field, after the non-negative half, so the IFFT interpolates. + int zSplit = (receiveSpec.nFastTimeSamples + 1) / 2; + int x = (sub[0] < zSplit) ? sub[0] + : sub[0] + (nz - receiveSpec.nFastTimeSamples); + int xSplit = (receiveSpec.nChannels + 1) / 2; + int z = (sub[1] < xSplit) ? sub[1] : sub[1] + (nx - receiveSpec.nChannels); float2 compoundSample{0, 0}; for (gsl::index i = 0; i < receiveSpec.nTX; i++) { From 79eba4725cc3a4b3cd6fc203cfed42f476972297 Mon Sep 17 00:00:00 2001 From: Arber Demi Date: Wed, 11 Nov 2026 15:34:48 +0100 Subject: [PATCH 5/6] ci: pin runners for MATLAB R2024a and docs, and retry apt downloads --- .github/workflows/docs.yml | 6 +++--- .github/workflows/matlab-tests.yml | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 4539b2d..e35e8b1 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -21,7 +21,7 @@ concurrency: jobs: sphinx: - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v4 @@ -32,8 +32,8 @@ jobs: # Breathe renders Doxygen's XML, so Doxygen has to run first. - name: Install Doxygen run: | - sudo apt-get update - sudo apt-get install --no-install-recommends -y doxygen + sudo apt-get -o Acquire::Retries=5 update + sudo apt-get -o Acquire::Retries=5 install --no-install-recommends -y doxygen - name: Install doc requirements run: python -m pip install --upgrade pip -r documentation/requirements.txt diff --git a/.github/workflows/matlab-tests.yml b/.github/workflows/matlab-tests.yml index b1d3bf1..ba73129 100644 --- a/.github/workflows/matlab-tests.yml +++ b/.github/workflows/matlab-tests.yml @@ -30,7 +30,8 @@ concurrency: jobs: headless: - runs-on: ubuntu-latest + # Pinned: MATLAB R2024a supports Ubuntu up to 22.04, and setup-matlab refuses newer. + runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v4 From a941dd96148a96ac79be40559cc8ff450a81b668 Mon Sep 17 00:00:00 2001 From: Arber Demi Date: Wed, 11 Nov 2026 16:08:01 +0100 Subject: [PATCH 6/6] ci: install Doxygen and MATLAB without the Ubuntu apt mirrors --- .github/workflows/docs.yml | 9 ++++++--- .github/workflows/matlab-tests.yml | 3 +++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index e35e8b1..ae955dd 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -29,11 +29,14 @@ jobs: with: python-version: "3.12" - # Breathe renders Doxygen's XML, so Doxygen has to run first. + # Breathe renders Doxygen's XML, so Doxygen has to run first. The release + # binary comes from GitHub, so the step does not depend on the Ubuntu mirrors. - name: Install Doxygen run: | - sudo apt-get -o Acquire::Retries=5 update - sudo apt-get -o Acquire::Retries=5 install --no-install-recommends -y doxygen + curl -fsSL --retry 5 -o "$RUNNER_TEMP/doxygen.tar.gz" \ + https://github.com/doxygen/doxygen/releases/download/Release_1_12_0/doxygen-1.12.0.linux.bin.tar.gz + tar -xzf "$RUNNER_TEMP/doxygen.tar.gz" -C "$RUNNER_TEMP" + echo "$RUNNER_TEMP/doxygen-1.12.0/bin" >> "$GITHUB_PATH" - name: Install doc requirements run: python -m pip install --upgrade pip -r documentation/requirements.txt diff --git a/.github/workflows/matlab-tests.yml b/.github/workflows/matlab-tests.yml index ba73129..6573f61 100644 --- a/.github/workflows/matlab-tests.yml +++ b/.github/workflows/matlab-tests.yml @@ -39,6 +39,9 @@ jobs: uses: matlab-actions/setup-matlab@v2 with: release: R2024a + # No apt step: the runner image already has the libraries MATLAB + # needs to run -batch, and the Ubuntu mirrors are not always reachable. + install-system-dependencies: false # verify_core_headless reaches initialize_image_reconstruction, which # builds tgcVector with gausswin, and echoframe_setup_fourier, which # apodises k-space with tukeywin.