Re: [PATCH 5/6] xfs: always take XFS_MMAPLOCK shared in xfs_dax_fault

Darrick J. Wong
2024-06-20T18:56:28Z

On Wed, Jun 19, 2024 at 01:53:55PM +0200, Christoph Hellwig wrote:
> After the previous refactoring, xfs_dax_fault is now never used for write
> faults, so don't bother with the xfs_ilock_for_write_fault logic to
> protect against writes when remapping is in progress.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Makes sense to me, all we need to do here is make sure that nobody can
invalidate the mappings -- there's no need to take MMAPLOCK_EXCL if a
reflink is cloning from this file's data.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
> fs/xfs/xfs_file.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 51e50afd935895..62a69ed796f2fd 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1279,12 +1279,11 @@ xfs_dax_fault(
> unsigned int order)
> {
> struct xfs_inode *ip = XFS_I(file_inode(vmf->vma->vm_file));
> - unsigned int lock_mode;
> vm_fault_t ret;
>
> - lock_mode = xfs_ilock_for_write_fault(ip);
> + xfs_ilock(ip, XFS_MMAPLOCK_SHARED);
> ret = xfs_dax_fault_locked(vmf, order, false);
> - xfs_iunlock(ip, lock_mode);
> + xfs_iunlock(ip, XFS_MMAPLOCK_SHARED);
>
> return ret;
> }
> --
> 2.43.0
>
>