From patchwork Tue Dec 22 22:46:05 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Morton X-Patchwork-Id: 560310 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id E7CED140311 for ; Wed, 23 Dec 2015 09:49:12 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933027AbbLVWqI (ORCPT ); Tue, 22 Dec 2015 17:46:08 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:59630 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932711AbbLVWqH (ORCPT ); Tue, 22 Dec 2015 17:46:07 -0500 Received: from akpm3.mtv.corp.google.com (unknown [216.239.45.65]) by mail.linuxfoundation.org (Postfix) with ESMTPSA id BEB30E7D; Tue, 22 Dec 2015 22:46:05 +0000 (UTC) Date: Tue, 22 Dec 2015 14:46:05 -0800 From: Andrew Morton To: Ross Zwisler Cc: linux-kernel@vger.kernel.org, "H. Peter Anvin" , "J. Bruce Fields" , "Theodore Ts'o" , Alexander Viro , Andreas Dilger , Dave Chinner , Ingo Molnar , Jan Kara , Jeff Layton , Matthew Wilcox , Thomas Gleixner , linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-nvdimm@ml01.01.org, x86@kernel.org, xfs@oss.sgi.com, Dan Williams , Matthew Wilcox , Dave Hansen Subject: Re: [PATCH v5 2/7] dax: support dirty DAX entries in radix tree Message-Id: <20151222144605.08a84ded98a42d6125a7991e@linux-foundation.org> In-Reply-To: <1450502540-8744-3-git-send-email-ross.zwisler@linux.intel.com> References: <1450502540-8744-1-git-send-email-ross.zwisler@linux.intel.com> <1450502540-8744-3-git-send-email-ross.zwisler@linux.intel.com> X-Mailer: Sylpheed 3.4.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) Mime-Version: 1.0 Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org On Fri, 18 Dec 2015 22:22:15 -0700 Ross Zwisler wrote: > Add support for tracking dirty DAX entries in the struct address_space > radix tree. This tree is already used for dirty page writeback, and it > already supports the use of exceptional (non struct page*) entries. > > In order to properly track dirty DAX pages we will insert new exceptional > entries into the radix tree that represent dirty DAX PTE or PMD pages. > These exceptional entries will also contain the writeback addresses for the > PTE or PMD faults that we can use at fsync/msync time. > > There are currently two types of exceptional entries (shmem and shadow) > that can be placed into the radix tree, and this adds a third. We rely on > the fact that only one type of exceptional entry can be found in a given > radix tree based on its usage. This happens for free with DAX vs shmem but > we explicitly prevent shadow entries from being added to radix trees for > DAX mappings. > > The only shadow entries that would be generated for DAX radix trees would > be to track zero page mappings that were created for holes. These pages > would receive minimal benefit from having shadow entries, and the choice > to have only one type of exceptional entry in a given radix tree makes the > logic simpler both in clear_exceptional_entry() and in the rest of DAX. > > > ... > > --- a/include/linux/dax.h > +++ b/include/linux/dax.h > @@ -36,4 +36,9 @@ static inline bool vma_is_dax(struct vm_area_struct *vma) > { > return vma->vm_file && IS_DAX(vma->vm_file->f_mapping->host); > } > + > +static inline bool dax_mapping(struct address_space *mapping) > +{ > + return mapping->host && IS_DAX(mapping->host); > +} Can we make this evaluate to plain old "0" when CONFIG_FS_DAX=n? That way a bunch of code in callers will fall away as well. If the compiler has any brains then a good way to do this would be to make IS_DAX be "0" but one would need to check that the zeroness properly propagated out of the inline. > #endif > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 3aa5142..b9ac534 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -433,6 +433,7 @@ struct address_space { > /* Protected by tree_lock together with the radix tree */ > unsigned long nrpages; /* number of total pages */ > unsigned long nrshadows; /* number of shadow entries */ > + unsigned long nrdax; /* number of DAX entries */ hm, that's unfortunate - machines commonly carry tremendous numbers of address_spaces in memory and adding pork to them is rather a big deal. We can't avoid this somehow? Maybe share the space with nrshadows by some means? Find some other field which is unused for dax files? > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -11,6 +11,7 @@ > */ > #include > #include > +#include > #include > #include > #include > @@ -579,6 +580,12 @@ static int page_cache_tree_insert(struct address_space *mapping, > p = radix_tree_deref_slot_protected(slot, &mapping->tree_lock); > if (!radix_tree_exceptional_entry(p)) > return -EEXIST; > + > + if (dax_mapping(mapping)) { > + WARN_ON(1); > + return -EINVAL; > + } this: --- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- a/mm/filemap.c~dax-support-dirty-dax-entries-in-radix-tree-fix +++ a/mm/filemap.c @@ -581,10 +581,8 @@ static int page_cache_tree_insert(struct if (!radix_tree_exceptional_entry(p)) return -EEXIST; - if (dax_mapping(mapping)) { - WARN_ON(1); + if (WARN_ON(dax_mapping(mapping))) return -EINVAL; - } if (shadowp) *shadowp = p;