Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966477AbdIYXis (ORCPT ); Mon, 25 Sep 2017 19:38:48 -0400 Received: from mail-oi0-f47.google.com ([209.85.218.47]:48115 "EHLO mail-oi0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966409AbdIYXiq (ORCPT ); Mon, 25 Sep 2017 19:38:46 -0400 X-Google-Smtp-Source: AOwi7QAaDA8cD+u7iWyx2aUqq7uswit58InAVm7fzsQ8OmMQkfn5hFdDy4WT9A9bxby4ZllyoENIs6v0aUwjCVeW5Rg= MIME-Version: 1.0 In-Reply-To: <20170925231404.32723-7-ross.zwisler@linux.intel.com> References: <20170925231404.32723-1-ross.zwisler@linux.intel.com> <20170925231404.32723-7-ross.zwisler@linux.intel.com> From: Dan Williams Date: Mon, 25 Sep 2017 16:38:45 -0700 Message-ID: Subject: Re: [PATCH 6/7] mm, fs: introduce file_operations->post_mmap() To: Ross Zwisler Cc: Andrew Morton , "linux-kernel@vger.kernel.org" , "Darrick J. Wong" , "J. Bruce Fields" , Christoph Hellwig , Dave Chinner , Jan Kara , Jeff Layton , linux-fsdevel , Linux MM , "linux-nvdimm@lists.01.org" , linux-xfs@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2397 Lines: 58 On Mon, Sep 25, 2017 at 4:14 PM, Ross Zwisler wrote: > When mappings are created the vma->vm_flags that they use vary based on > whether the inode being mapped is using DAX or not. This setup happens in > XFS via mmap_region()=>call_mmap()=>xfs_file_mmap(). > > For us to be able to safely use the DAX per-inode flag we need to prevent > S_DAX transitions when any mappings are present, and we will do that by > looking at the address_space->i_mmap tree and returning -EBUSY if any > mappings are present. > > Unfortunately at the time that the filesystem's file_operations->mmap() > entry point is called the mapping has not yet been added to the > address_space->i_mmap tree. This means that at that point in time we > cannot determine whether or not the mapping will be set up to support DAX. > > Fix this by adding a new file_operations entry called post_mmap() which is > called after the mapping has been added to the address_space->i_mmap tree. > This post_mmap() op now happens at a time when we can be sure whether the > mapping will use DAX or not, and we can set up the vma->vm_flags > appropriately. > > Signed-off-by: Ross Zwisler > --- > fs/xfs/xfs_file.c | 15 ++++++++++++++- > include/linux/fs.h | 1 + > mm/mmap.c | 2 ++ > 3 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c > index 2816858..9d66aaa 100644 > --- a/fs/xfs/xfs_file.c > +++ b/fs/xfs/xfs_file.c > @@ -1087,9 +1087,21 @@ xfs_file_mmap( > { > file_accessed(filp); > vma->vm_ops = &xfs_file_vm_ops; > + return 0; > +} > + > +/* This call happens during mmap(), after the vma has been inserted into the > + * inode->i_mapping->i_mmap tree. At this point the decision on whether or > + * not to use DAX for this mapping has been set and will not change for the > + * duration of the mapping. > + */ > +STATIC void > +xfs_file_post_mmap( > + struct file *filp, > + struct vm_area_struct *vma) > +{ > if (IS_DAX(file_inode(filp))) > vma->vm_flags |= VM_MIXEDMAP | VM_HUGEPAGE; It's not clear to me what this is actually protecting? vma_is_dax() returns true regardless of the vm_flags state , so what is the benefit to delaying the vm_flags setting to ->post_mmap()? Also, why is this a file_operation and not a vm_operation?