Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1032347AbdIZVl6 (ORCPT ); Tue, 26 Sep 2017 17:41:58 -0400 Received: from mail-oi0-f48.google.com ([209.85.218.48]:49557 "EHLO mail-oi0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1031049AbdIZVly (ORCPT ); Tue, 26 Sep 2017 17:41:54 -0400 X-Google-Smtp-Source: AOwi7QCmIRpAvnbxKU5ZCMx7onsoZDM1wTySoksJuzqJ2lQcCLY07EaoJ2tWXSWp/0G6/JQnjpI10svYCa0GAqWccuk= MIME-Version: 1.0 In-Reply-To: <20170926210645.GA7798@linux.intel.com> References: <20170925231404.32723-1-ross.zwisler@linux.intel.com> <20170925231404.32723-7-ross.zwisler@linux.intel.com> <20170926185751.GB31146@linux.intel.com> <20170926210645.GA7798@linux.intel.com> From: Dan Williams Date: Tue, 26 Sep 2017 14:41:53 -0700 Message-ID: Subject: Re: [PATCH 6/7] mm, fs: introduce file_operations->post_mmap() To: Ross Zwisler , Dan Williams , 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: 1908 Lines: 37 On Tue, Sep 26, 2017 at 2:06 PM, Ross Zwisler wrote: > On Tue, Sep 26, 2017 at 12:19:21PM -0700, Dan Williams wrote: >> On Tue, Sep 26, 2017 at 11:57 AM, Ross Zwisler > <> >> > This decision can only be made (in this >> > proposed scheme) *after* the inode->i_mapping->i_mmap tree has been >> > populated, which means we need another call into the filesystem after this >> > insertion has happened. >> >> I get that, but it seems over-engineered and something that can also >> be safely cleaned up after the fact by the code path that is disabling >> DAX. > > I don't think you can safely clean it up after the fact because some thread > might have already called ->mmap() to set up the vma->vm_flags for their new > mapping, but they haven't added it to inode->i_mapping->i_mmap. If madvise(MADV_NOHUGEPAGE) can dynamically change vm_flags, then the DAX disable path can as well. VM_MIXEDMAP looks to be a nop for normal memory mappings. > The inode->i_mapping->i_mmap tree is the only way (that I know of at least) > that the filesystem has any idea about about the mapping. This is the method > by which we would try and clean up mapping flags, if we were to do so, and > it's the only way that the filesystem can know whether or not mappings exist. > > The only way that I could think of to make this safely work is to have the > insertion into the inode->i_mapping->i_mmap tree be our sync point. After > that the filesystem and the mapping code can communicate on the state of DAX, > but before that I think it's basically indeterminate. If we lose the race and leak VM_HUGEPAGE to a non-DAX mapping what breaks? I'd rather be in favor of not setting VM_HUGEPAGE at all in the ->mmap() handler and let the default THP policy take over. In fact, see transparent_hugepage_enabled() we already auto-enable huge page support for dax mappings regardless of VM_HUGEPAGE.