From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Chinner Subject: Re: [PATCH v8 06/14] xfs: wire up MAP_DIRECT Date: Wed, 11 Oct 2017 12:09:22 +1100 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-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <150764697001.16882.13486539828150761233.stgit-p8uTFz9XbKj2zm6wflaqv1nYeNYlB/vhral2JQCrhuEAvxtiuMwx3w@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-nvdimm-bounces-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org Sender: "Linux-nvdimm" To: Dan Williams Cc: "J. Bruce Fields" , Jan Kara , Arnd Bergmann , "Darrick J. Wong" , linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org, linux-xfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, Alexander Viro , linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jeff Layton , Christoph Hellwig List-Id: linux-api@vger.kernel.org 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-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org