From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Wed, 11 Oct 2017 12:09:22 +1100 From: Dave Chinner To: Dan Williams 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@vger.kernel.org, iommu@lists.linux-foundation.org, Christoph Hellwig , "J. Bruce Fields" , linux-mm@kvack.org, Jeff Moyer , Alexander Viro , linux-fsdevel@vger.kernel.org, Jeff Layton , Ross Zwisler Subject: Re: [PATCH v8 06/14] xfs: wire up MAP_DIRECT Message-ID: <20171011010922.GY3666@dastard> References: <150764693502.16882.15848797003793552156.stgit@dwillia2-desk3.amr.corp.intel.com> <150764697001.16882.13486539828150761233.stgit@dwillia2-desk3.amr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <150764697001.16882.13486539828150761233.stgit@dwillia2-desk3.amr.corp.intel.com> Sender: owner-linux-mm@kvack.org List-ID: 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; } .... 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: email@kvack.org