Re: [PATCH v8 06/14] xfs: wire up MAP_DIRECT

[Dave Chinner <david@fromorbit.com>]

On Tue, Oct 10, 2017 at 07:49:30AM -0700, Dan Williams wrote:
> @@ -1009,6 +1019,22 @@ xfs_file_llseek(
>  }
>  
>  /*
> + * MAP_DIRECT faults can only be serviced while the FL_LAYOUT lease is
> + * valid. See map_direct_invalidate.
> + */
> +static int
> +xfs_can_fault_direct(
> +	struct vm_area_struct	*vma)
> +{
> +	if (!xfs_vma_is_direct(vma))
> +		return 0;
> +
> +	if (!test_map_direct_valid(vma->vm_private_data))
> +		return VM_FAULT_SIGBUS;
> +	return 0;
> +}

Better, but I'm going to be an annoying pedant here: a "can
<something>" check should return a boolean true/false.

Also, it's a bit jarring to see that a non-direct VMA that /can't/
do direct faults returns the same thing as a direct-vma that /can/
do direct faults, so a couple of extra comments for people who will
quickly forget how this code works (i.e. me) will be helpful. Say
something like this:

/*
 * MAP_DIRECT faults can only be serviced while the FL_LAYOUT lease is
 * valid. See map_direct_invalidate.
 */
static bool
xfs_vma_has_direct_lease(
	struct vm_area_struct	*vma)
{
	/* Non MAP_DIRECT vmas do not require layout leases */
	if (!xfs_vma_is_direct(vma))
		return true;

	if (!test_map_direct_valid(vma->vm_private_data))
		return false;

	/* We have a valid lease */
	return true;
}

.....
	if (!xfs_vma_has_direct_lease(vma)) {
		ret = VM_FAULT_SIGBUS;
		goto out_unlock;
	}
....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>