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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 10, 2017 at 6:09 PM, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
> 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;
>         }
> ....


Looks good to me.



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux