From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi0-x233.google.com (mail-oi0-x233.google.com [IPv6:2607:f8b0:4003:c06::233]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id ED2F921EA15D3 for ; Tue, 10 Oct 2017 19:09:22 -0700 (PDT) Received: by mail-oi0-x233.google.com with SMTP id m198so646938oig.5 for ; Tue, 10 Oct 2017 19:12:51 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20171011010922.GY3666@dastard> References: <150764693502.16882.15848797003793552156.stgit@dwillia2-desk3.amr.corp.intel.com> <150764697001.16882.13486539828150761233.stgit@dwillia2-desk3.amr.corp.intel.com> <20171011010922.GY3666@dastard> From: Dan Williams Date: Tue, 10 Oct 2017 19:12:50 -0700 Message-ID: Subject: Re: [PATCH v8 06/14] xfs: wire up MAP_DIRECT List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-nvdimm-bounces@lists.01.org Sender: "Linux-nvdimm" To: Dave Chinner Cc: "J. Bruce Fields" , Jan Kara , Arnd Bergmann , "Darrick J. Wong" , linux-rdma@vger.kernel.org, Linux API , "linux-nvdimm@lists.01.org" , linux-xfs@vger.kernel.org, Linux MM , iommu@lists.linux-foundation.org, Alexander Viro , linux-fsdevel , Jeff Layton , Christoph Hellwig List-ID: On Tue, Oct 10, 2017 at 6:09 PM, Dave Chinner 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 > " 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. _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Williams Subject: Re: [PATCH v8 06/14] xfs: wire up MAP_DIRECT Date: Tue, 10 Oct 2017 19:12:50 -0700 Message-ID: References: <150764693502.16882.15848797003793552156.stgit@dwillia2-desk3.amr.corp.intel.com> <150764697001.16882.13486539828150761233.stgit@dwillia2-desk3.amr.corp.intel.com> <20171011010922.GY3666@dastard> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20171011010922.GY3666@dastard> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-nvdimm-bounces-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org Sender: "Linux-nvdimm" To: Dave Chinner Cc: "J. Bruce Fields" , Jan Kara , Arnd Bergmann , "Darrick J. Wong" , linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux API , "linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org" , linux-xfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux MM , iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, Alexander Viro , linux-fsdevel , Jeff Layton , Christoph Hellwig List-Id: linux-rdma@vger.kernel.org On Tue, Oct 10, 2017 at 6:09 PM, Dave Chinner 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 > " 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. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 In-Reply-To: <20171011010922.GY3666@dastard> References: <150764693502.16882.15848797003793552156.stgit@dwillia2-desk3.amr.corp.intel.com> <150764697001.16882.13486539828150761233.stgit@dwillia2-desk3.amr.corp.intel.com> <20171011010922.GY3666@dastard> From: Dan Williams Date: Tue, 10 Oct 2017 19:12:50 -0700 Message-ID: Subject: Re: [PATCH v8 06/14] xfs: wire up MAP_DIRECT To: Dave Chinner Cc: "linux-nvdimm@lists.01.org" , linux-xfs@vger.kernel.org, Jan Kara , Arnd Bergmann , "Darrick J. Wong" , linux-rdma@vger.kernel.org, Linux API , iommu@lists.linux-foundation.org, Christoph Hellwig , "J. Bruce Fields" , Linux MM , Jeff Moyer , Alexander Viro , linux-fsdevel , Jeff Layton , Ross Zwisler Content-Type: text/plain; charset="UTF-8" Sender: owner-linux-mm@kvack.org List-ID: On Tue, Oct 10, 2017 at 6:09 PM, Dave Chinner 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 > " 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. -- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi0-f54.google.com ([209.85.218.54]:53801 "EHLO mail-oi0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756830AbdJKCMv (ORCPT ); Tue, 10 Oct 2017 22:12:51 -0400 Received: by mail-oi0-f54.google.com with SMTP id j126so628763oia.10 for ; Tue, 10 Oct 2017 19:12:51 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20171011010922.GY3666@dastard> References: <150764693502.16882.15848797003793552156.stgit@dwillia2-desk3.amr.corp.intel.com> <150764697001.16882.13486539828150761233.stgit@dwillia2-desk3.amr.corp.intel.com> <20171011010922.GY3666@dastard> From: Dan Williams Date: Tue, 10 Oct 2017 19:12:50 -0700 Message-ID: Subject: Re: [PATCH v8 06/14] xfs: wire up MAP_DIRECT Content-Type: text/plain; charset="UTF-8" Sender: linux-xfs-owner@vger.kernel.org List-ID: List-Id: xfs To: Dave Chinner Cc: "linux-nvdimm@lists.01.org" , linux-xfs@vger.kernel.org, Jan Kara , Arnd Bergmann , "Darrick J. Wong" , linux-rdma@vger.kernel.org, Linux API , iommu@lists.linux-foundation.org, Christoph Hellwig , "J. Bruce Fields" , Linux MM , Jeff Moyer , Alexander Viro , linux-fsdevel , Jeff Layton , Ross Zwisler On Tue, Oct 10, 2017 at 6:09 PM, Dave Chinner 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 > " 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.