daxfs_mem_sync() (daxfs/dax_mem.c:218) is a store barrier and nothing else:
void daxfs_mem_sync(struct daxfs_info *info, void *ptr, size_t size)
{
...
smp_wmb();
}
The comment is explicit that this is incomplete:
On platforms with ADR/eADR, this is sufficient for persistence. On platforms without hardware persistence guarantees, explicit cache line flushes (clflush/clwb) would be needed per cache line - not yet implemented.
That is correct for the dma-buf and shared-DRAM backings in use today, where the memory is volatile anyway and the barrier is all that is meaningful. It is not correct for a persistent backing, and the README describes daxfs as operating on "persistent memory, CXL memory, or DMA buffers", so fsync() currently promises more than it delivers on the first of those.
Separately, daxfs_fsync() (daxfs/file.c:803) ignores its file argument and syncs the entire overlay region:
daxfs_mem_sync(info, daxfs_mem_ptr(info, ovl_offset), ovl_size);
Harmless while the sync is a barrier, but it becomes O(overlay size) per fsync the moment real cache-line flushing is added. A multi-gigabyte overlay would make every fsync flush the whole thing.
Suggested: implement clwb/clflushopt per cache line behind a mount option or a backing-type check, and make daxfs_fsync() flush only the pages belonging to the file being synced. Until then, consider documenting that persistence relies on ADR/eADR.
Found during the review in #14.
daxfs_mem_sync()(daxfs/dax_mem.c:218) is a store barrier and nothing else:The comment is explicit that this is incomplete:
That is correct for the dma-buf and shared-DRAM backings in use today, where the memory is volatile anyway and the barrier is all that is meaningful. It is not correct for a persistent backing, and the README describes daxfs as operating on "persistent memory, CXL memory, or DMA buffers", so
fsync()currently promises more than it delivers on the first of those.Separately,
daxfs_fsync()(daxfs/file.c:803) ignores its file argument and syncs the entire overlay region:Harmless while the sync is a barrier, but it becomes O(overlay size) per fsync the moment real cache-line flushing is added. A multi-gigabyte overlay would make every fsync flush the whole thing.
Suggested: implement
clwb/clflushoptper cache line behind a mount option or a backing-type check, and makedaxfs_fsync()flush only the pages belonging to the file being synced. Until then, consider documenting that persistence relies on ADR/eADR.Found during the review in #14.