On Fri, Jan 10, 2025 at 05:00:29PM +1100, Alistair Popple wrote:
> FS DAX requires file systems to call into the DAX layout prior to unlinking
> inodes to ensure there is no ongoing DMA or other remote access to the
> direct mapped page. The fuse file system implements
> fuse_dax_break_layouts() to do this which includes a comment indicating
> that passing dmap_end == 0 leads to unmapping of the whole file.
>
> However this is not true - passing dmap_end == 0 will not unmap anything
> before dmap_start, and further more dax_layout_busy_page_range() will not
> scan any of the range to see if there maybe ongoing DMA access to the
> range. Fix this by passing -1 for dmap_end to fuse_dax_break_layouts()
> which will invalidate the entire file range to
> dax_layout_busy_page_range().
Hi Alistair,
Thanks for fixing DAX related issues for virtiofs. I am wondering how are
you testing DAX with virtiofs. AFAIK, we don't have DAX support in Rust
virtiofsd. C version of virtiofsd used to have out of the tree patches
for DAX. But C version got deprecated long time ago.
Do you have another implementation of virtiofsd somewhere else which
supports DAX and allows for testing DAX related changes?
Thanks
Vivek
>
> Signed-off-by: Alistair Popple <apop...@nvidia.com>
> Co-developed-by: Dan Williams <dan.j.willi...@intel.com>
> Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
> Fixes: 6ae330cad6ef ("virtiofs: serialize truncate/punch_hole and dax fault
> path")
> Cc: Vivek Goyal <vgo...@redhat.com>
>
> ---
>
> Changes for v6:
>
> - Original patch had a misplaced hunk due to a bad rebase.
> - Reworked fix based on Dan's comments.
> ---
> fs/fuse/dax.c | 1 -
> fs/fuse/dir.c | 2 +-
> fs/fuse/file.c | 4 ++--
> 3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
> index 9abbc2f..455c4a1 100644
> --- a/fs/fuse/dax.c
> +++ b/fs/fuse/dax.c
> @@ -681,7 +681,6 @@ static int __fuse_dax_break_layouts(struct inode *inode,
> bool *retry,
> 0, 0, fuse_wait_dax_page(inode));
> }
>
> -/* dmap_end == 0 leads to unmapping of whole file */
> int fuse_dax_break_layouts(struct inode *inode, u64 dmap_start,
> u64 dmap_end)
> {
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 0b2f856..bc6c893 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -1936,7 +1936,7 @@ int fuse_do_setattr(struct mnt_idmap *idmap, struct
> dentry *dentry,
> if (FUSE_IS_DAX(inode) && is_truncate) {
> filemap_invalidate_lock(mapping);
> fault_blocked = true;
> - err = fuse_dax_break_layouts(inode, 0, 0);
> + err = fuse_dax_break_layouts(inode, 0, -1);
> if (err) {
> filemap_invalidate_unlock(mapping);
> return err;
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 082ee37..cef7a8f 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -253,7 +253,7 @@ static int fuse_open(struct inode *inode, struct file
> *file)
>
> if (dax_truncate) {
> filemap_invalidate_lock(inode->i_mapping);
> - err = fuse_dax_break_layouts(inode, 0, 0);
> + err = fuse_dax_break_layouts(inode, 0, -1);
> if (err)
> goto out_inode_unlock;
> }
> @@ -2890,7 +2890,7 @@ static long fuse_file_fallocate(struct file *file, int
> mode, loff_t offset,
> inode_lock(inode);
> if (block_faults) {
> filemap_invalidate_lock(inode->i_mapping);
> - err = fuse_dax_break_layouts(inode, 0, 0);
> + err = fuse_dax_break_layouts(inode, 0, -1);
> if (err)
> goto out;
> }
> --
> git-series 0.9.1
>
