Message ID | 20231213150703.6262-1-bschubert@ddn.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | fuse: Fix VM_MAYSHARE and direct_io_allow_mmap | expand |
On Wed, Dec 13, 2023 at 5:07 PM Bernd Schubert <bschubert@ddn.com> wrote: > > There were multiple issues with direct_io_allow_mmap: > - fuse_link_write_file() was missing, resulting in warnings in > fuse_write_file_get() and EIO from msync() > - "vma->vm_ops = &fuse_file_vm_ops" was not set, but especially > fuse_page_mkwrite is needed. > > The semantics of invalidate_inode_pages2() is so far not clearly defined > in fuse_file_mmap. It dates back to > commit 3121bfe76311 ("fuse: fix "direct_io" private mmap") > Though, as direct_io_allow_mmap is a new feature, that was for MAP_PRIVATE > only. As invalidate_inode_pages2() is calling into fuse_launder_folio() > and writes out dirty pages, it should be save to call typo: safe to call... > invalidate_inode_pages2 for MAP_PRIVATE and MAP_SHARED as well. > > Cc: Hao Xu <howeyxu@tencent.com> > Cc: Miklos Szeredi <miklos@szeredi.hu> > Cc: Dharmendra Singh <dsingh@ddn.com> > Cc: Amir Goldstein <amir73il@gmail.com> > Cc: linux-fsdevel@vger.kernel.org > Cc: stable@vger.kernel.org > Fixes: e78662e818f9 ("fuse: add a new fuse init flag to relax restrictions in no cache mode") > Signed-off-by: Bernd Schubert <bschubert@ddn.com> > --- Reviewed-by: Amir Goldstein <amir73il@gmail.com> Thanks, Amir. > fs/fuse/file.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c > index a660f1f21540..174aa16407c4 100644 > --- a/fs/fuse/file.c > +++ b/fs/fuse/file.c > @@ -2475,7 +2475,10 @@ static int fuse_file_mmap(struct file *file, struct vm_area_struct *vma) > > invalidate_inode_pages2(file->f_mapping); > > - return generic_file_mmap(file, vma); > + if (!(vma->vm_flags & VM_MAYSHARE)) { > + /* MAP_PRIVATE */ > + return generic_file_mmap(file, vma); > + } > } > > if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE)) > -- > 2.40.1 >
diff --git a/fs/fuse/file.c b/fs/fuse/file.c index a660f1f21540..174aa16407c4 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -2475,7 +2475,10 @@ static int fuse_file_mmap(struct file *file, struct vm_area_struct *vma) invalidate_inode_pages2(file->f_mapping); - return generic_file_mmap(file, vma); + if (!(vma->vm_flags & VM_MAYSHARE)) { + /* MAP_PRIVATE */ + return generic_file_mmap(file, vma); + } } if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE))
There were multiple issues with direct_io_allow_mmap: - fuse_link_write_file() was missing, resulting in warnings in fuse_write_file_get() and EIO from msync() - "vma->vm_ops = &fuse_file_vm_ops" was not set, but especially fuse_page_mkwrite is needed. The semantics of invalidate_inode_pages2() is so far not clearly defined in fuse_file_mmap. It dates back to commit 3121bfe76311 ("fuse: fix "direct_io" private mmap") Though, as direct_io_allow_mmap is a new feature, that was for MAP_PRIVATE only. As invalidate_inode_pages2() is calling into fuse_launder_folio() and writes out dirty pages, it should be save to call invalidate_inode_pages2 for MAP_PRIVATE and MAP_SHARED as well. Cc: Hao Xu <howeyxu@tencent.com> Cc: Miklos Szeredi <miklos@szeredi.hu> Cc: Dharmendra Singh <dsingh@ddn.com> Cc: Amir Goldstein <amir73il@gmail.com> Cc: linux-fsdevel@vger.kernel.org Cc: stable@vger.kernel.org Fixes: e78662e818f9 ("fuse: add a new fuse init flag to relax restrictions in no cache mode") Signed-off-by: Bernd Schubert <bschubert@ddn.com> --- fs/fuse/file.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)