On 09/01/2015 10:06 AM, Christoph Hellwig wrote:
On Tue, Sep 01, 2015 at 09:38:03AM +1000, Dave Chinner wrote:
> On Mon, Aug 31, 2015 at 12:59:44PM -0600, Ross Zwisler wrote:
>> For DAX msync we just need to flush the given range using
>> wb_cache_pmem(), which is now a public part of the PMEM API.
>
> This is wrong, because it still leaves fsync() broken on dax.
>
> Flushing dirty data to stable storage is the responsibility of the
> writeback infrastructure, not the VMA/mm infrasrtucture. For non-dax
> configurations, msync defers all that to vfs_fsync_range(), because
> it has to be implemented there for fsync() to work.
>
> Even for DAX, msync has to call vfs_fsync_range() for the filesystem to commit
> the backing store allocations to stable storage, so there's not
> getting around the fact msync is the wrong place to be flushing
> DAX mappings to persistent storage.
DAX does call ->fsync before and after this patch. And with all
the recent fixes we take care to ensure data is written though the
cache for everything but mmap-access. With this patch from Ross
we ensure msync writes back the cache before calling ->fsync so that
the filesystem can then do it's work like converting unwritten extents.
The only downside is that previously on Linux you could always use
fsync as a replaement for msymc, which isn't true anymore for DAX.
Hi Christoph
So the approach we took was a bit different to exactly solve these
problem, and to also not over flush too much. here is what we did.
* At vm_operations_struct we also override the .close vector (say call it dax_vm_close)
* At dax_vm_close() on writable files call ->fsync(,vma->vm_start, vma->vm_end,)
(We have an inode flag if the file was actually dirtied, but even if not, that will
not be that bad, so a file was opened for write, mmapped, but actually never
modified. Not a lot of these, and the do nothing cl_flushing is very fast)
* At ->fsync() do the actual cl_flush for all cases but only iff
if (mapping_mapped(inode->i_mapping) == 0)
return 0;
This is because data written not through mmap is already persistent and we
do not need the cl_flushing
Apps expect all these to work:
1. open mmap m-write msync ... close
2. open mmap m-write fsync ... close
3. open mmap m-write unmap ... fsync close
4. open mmap m-write sync ...
The first 3 are supported with above, because what happens is that at [3]
the fsync actually happens on unmap and fsync is redundant in that case.
The only broken scenario is [3]. We do not have a list of "dax-dirty" inodes
per sb to iterate on and call inode-sync on. This cause problems mostly in
freeze because with actual [3] scenario the file will be eventually closed
and persistent, but after the call to sync returns.
Its on my TODO to fix [3] based on instructions from Dave.
The mmap call will put the inode on the list and the dax_vm_close will
remove it. One of the regular dirty list should be used as suggested by
Dave.
But given that we need the virtual address to write back the cache
I can't see how to do this differently given that clwb() needs the
user virtual address to flush the cache.
On Intel or any systems that have physical-based caching this is not
a problem you just iterate on all get_block() of the range and flush
the Kernel's virt_addr of the block, this is easy.
With ARCHs with per VM caching you need to go through the i_mapping VMAs list
and flush like that. I guess there is a way to schedule yourself as a process VMA
somehow.
I'm not sure how to solve this split, perhaps two generic functions, that
are selected through the ARCH.
Just my $0.017
Boaz