On Sat, Jun 17, 2017 at 2:52 PM, Dan Williams <dan.j.williams@xxxxxxxxx> wrote: > On Sat, Jun 17, 2017 at 9:25 AM, Andy Lutomirski <luto@xxxxxxxxxx> wrote: >> >> Can you remind those of us who haven't played with DAX in a while what >> the problem is with mmapping a DAX file without this patchset? If >> there's some bookkkeeping needed to make sure that the filesystem will >> invalidate all the mappings if it decides to move the file, maybe that >> should be the default rather than needing a new syscall. > > The bookkeeping to invalidate mappings when the filesystem moves a > block is already there. > > Without this patchset an application needs to call fsync/msync after > any write to a DAX mapping otherwise there is no guarantee the > filesystem has written the metadata to find the updated block after a > crash or power loss event. Even if the sync operation is reduced to a > minimal cmpxchg in userspace to check if the filesystem-metadata is > dirty, that mechanism doesn't translate to a virtualized environment, > as requiring guests to trigger host fsync()s is not feasible. It's a > half-step solution when you can instead just ask the filesystem to > never move blocks, as Dave proposed many months back. > > We stepped back from that proposal when it looked like a significant > amount of per-filesystem work to introduce the capability and it was > not clear that application developers would tolerate the side effects > of this 'immutable' semantic. However, the implementation is dead > simple since ext4 and xfs already need to make > block-allocation-immutable semantics available for swapfiles. We also > have application developers telling us they are ok with the semantics, > especially because it catches Linux up to other operating environments > that are already on board with allowing this type of access to pmem > through a filesystem. This patchset gives pmem application developers > what they want without any additional burden on filesystem > implementations. I see. I have a couple of minor-ish issues with the current proposal, then. One is that I think the terminology should be changed to still make sense if filesystems or VFS improves to make this approach unnecessary. Rather than saying "this file is now static", perhaps users should set a flag with the explicit semantics that "mmaps of this file are guaranteed not to lose data due to the kernel's activities", IOW that mmaps will be at least as durable as a direct mapping of DAX memory. Then the kernel has the flexibility to add a future implementation in which, instead of pinning the file, the filesystem just knows to keep metadata synced before allowing page_mkwrite to re-enable writes to an mmapped DAX file. My other objection is that the syscall intentionally leaks a reference to the file. This means it needs overflow protection and it probably shouldn't ever be allowed to use it without privilege. Why can't the underlying issue be easily fixed, though? Could .page_mkwrite just make sure that metadata is synced when the FS uses DAX? On a DAX fs, syncing metadata should be extremely fast. This could be conditioned on an madvise or mmap flag if performance might be an issue. As far as I know, this change alone should be sufficient. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
