On Sun, Feb 07, 2016 at 10:15:12AM +1100, Dave Chinner wrote: > On Thu, Feb 04, 2016 at 10:15:58AM +0100, Jan Kara wrote: > > On Wed 03-02-16 13:13:28, Ross Zwisler wrote: > > > Here is the comment from Dave Chinner that had me move to having the calls to > > > dax_writeback_mapping_range() into the generic mm code: > > > > > > > Lastly, this flushing really needs to be inside > > > > filemap_write_and_wait_range(), because we call the writeback code > > > > from many more places than just fsync to ensure ordering of various > > > > operations such that files are in known state before proceeding > > > > (e.g. hole punch). > > > https://lkml.org/lkml/2015/11/16/847 > ..... > > > > So revisiting the decision I see two options: > > > > > > > > 1) Move the DAX flushing code from filemap_write_and_wait() into > > > > ->writepages() fs callback. There the filesystem can provide all the > > > > information it needs including bdev, get_block callback, or whatever. > > > > > > This seems fine as long as we add it to ->fsync as well since ->writepages is > > > never called in that path, and as long as we are okay with skipping DAX > > > writebacks on hole punch, truncate, and block relocation. > > > > Look at ext4_sync_file() -> filemap_write_and_wait_range() -> > > __filemap_fdatawrite_range() -> do_writepages(). Except those nrpages > 0 > > checks which would need to be changed. > > Just to be clear: this is pretty much what I was implying was > necessary when I said that the DAX flushing needed to be "inside > filemap_write_and_wait_range". And that's what I thought Ross was > planning on doing after that round discussion. i.e. Ross said: > > "If the race described above isn't an issue then I agree moving this > call out of the filesystems and down into the generic page writeback > code is probably the right thing to do." > > https://lkml.org/lkml/2015/11/17/718 > > Realistically, what Jan is saying in this thread is exactly what I > said we needed to do way back when I first pointed out that fsync > was broken and dirty tracking in the mapping radix tree was still > needed for fsync to work effectively. Here, let me try and quickly summarize what is going on. 1) The DAX fsync set was merged into v4.5-rc1, it does use the radix tree for tracking dirty PTE and PMD pages, and we do currently call into the DAX sync code via filemap_write_and_wait_range() as you initially suggested. 2) During testing of raw block devices + DAX I noticed that the struct block_device that we were using for DAX operations was incorrect. For the fault handlers, etc. we can just get the correct bdev via get_block(), which is passed in as a function pointer, but for the flushing code we don't have access to get_block(). This is also an issue for XFS real-time devices, whenever we get those working. In short, somehow we need to get dax_writeback_mapping_range() a valid bdev. Right now it is called via filemap_write_and_wait_range(), which can't provide either the bdev nor a get_block() function pointer. So, our options seem to be: a) Move the calls to dax_writeback_mapping_range() into the filesystems (what Jan is suggesting, i.e. ->writepages()) b) Keep the calls to dax_writeback_mapping_range() in the mm code, and provide a generic way to ask a filesystem for an inode's bdev. I did a version of this using a superblock operation here: https://lkml.org/lkml/2016/2/2/941 3) During the review and discussion for the above problems, Jan noticed that the flushing code wasn't being called for sync() and syncfs(). Clearly from your other response (https://lkml.org/lkml/2016/2/6/168) you think this is incorrect. Regardless, the above issue remains - dax_writeback_mapping_range() needs a bdev. Do we move the calls into the filesystem so the fs can provide a bdev, or do we we create a generic method for DAX to ask the fs for the correct bdev for an inode? -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
