Re: [PATCH V12 06/12] famfs: Introduce mmap and VM fault handling

On 26/08/05 10:16PM, Darrick J. Wong wrote:
> On Mon, Aug 03, 2026 at 02:29:16AM +0000, John Groves wrote:
> > From: John Groves <[email protected]>
> > 
> > This commit adds vm_operations, plus famfs_mmap() and fault handlers.
> > It is still missing iomap_ops, iomap mapping resolution, and
> > famfs_ioctl() for setting up file-to-memory mappings.
> > 
> > Signed-off-by: John Groves <[email protected]>
> > ---
> >  fs/famfs/famfs_file.c | 101 +++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 100 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
> > index e192b573c51f..678f2035fd5f 100644
> > --- a/fs/famfs/famfs_file.c
> > +++ b/fs/famfs/famfs_file.c
> > @@ -16,6 +16,75 @@
> >  
> >  #include "famfs_internal.h"
> >  
> > +/*********************************************************************
> > + * vm_operations
> > + */
> > +static vm_fault_t
> > +__famfs_filemap_fault(struct vm_fault *vmf, unsigned int order,
> > +		      bool write_fault)
> > +{
> > +	struct inode *inode = file_inode(vmf->vma->vm_file);
> > +	struct super_block *sb = inode->i_sb;
> > +	struct famfs_fs_info *fsi = sb->s_fs_info;
> > +	vm_fault_t ret;
> > +	unsigned long pfn;
> > +
> > +	if (fsi->deverror)
> > +		return VM_FAULT_SIGBUS;
> > +
> > +	if (!IS_DAX(file_inode(vmf->vma->vm_file))) {
> > +		pr_err("%s: file not marked IS_DAX!!\n", __func__);
> > +		return VM_FAULT_SIGBUS;
> > +	}
> > +
> > +	if (write_fault) {
> > +		sb_start_pagefault(inode->i_sb);
> > +		file_update_time(vmf->vma->vm_file);
> > +	}
> > +
> > +	ret = dax_iomap_fault(vmf, order, &pfn, NULL, NULL /*&famfs_iomap_ops */);
> > +	if (ret & VM_FAULT_NEEDDSYNC)
> > +		ret = dax_finish_sync_fault(vmf, order, pfn);
> > +
> > +	if (write_fault)
> > +		sb_end_pagefault(inode->i_sb);
> > +
> > +	return ret;
> > +}
> > +
> > +static inline bool
> > +famfs_is_write_fault(struct vm_fault *vmf)
> > +{
> > +	return (vmf->flags & FAULT_FLAG_WRITE) &&
> > +	       (vmf->vma->vm_flags & VM_SHARED);
> > +}
> > +
> > +static vm_fault_t
> > +famfs_filemap_fault(struct vm_fault *vmf)
> > +{
> > +	return __famfs_filemap_fault(vmf, 0, famfs_is_write_fault(vmf));
> > +}
> > +
> > +static vm_fault_t
> > +famfs_filemap_huge_fault(struct vm_fault *vmf, unsigned int order)
> > +{
> > +	return __famfs_filemap_fault(vmf, order, famfs_is_write_fault(vmf));
> > +}
> > +
> > +static vm_fault_t
> > +famfs_filemap_mkwrite(struct vm_fault *vmf)
> > +{
> > +	return __famfs_filemap_fault(vmf, 0, true);
> 
> Do neither of the conditions in famfs_is_write_fault apply here?

I'm confident this is correct. This is directly copied from xfs on the 
dax path:

	if (IS_DAX(inode))
		ret = xfs_dax_fault_locked(vmf, order, true);

Famfs never has to do COW or allocate a page or any of that sort of
sfuff; It ends up just looking at the fmap to resolve to a page,
which always exists unless out or range or bugs.

> 
> The rest of the patch looks ok to me.
> 
> --D

Thank you!

John

<snip>