From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ross Zwisler Subject: Re: [PATCH v2 3/3] dax: use common 4k zero page for dax mmap reads Date: Fri, 16 Jun 2017 13:45:40 -0600 Message-ID: <20170616194540.GB20742@linux.intel.com> References: <20170614172211.19820-1-ross.zwisler@linux.intel.com> <20170614172211.19820-4-ross.zwisler@linux.intel.com> <20170615145856.GO1764@quack2.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Theodore Ts'o , Matthew Wilcox , linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, "Darrick J. Wong" , Jonathan Corbet , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Steven Rostedt , Christoph Hellwig , linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, Dave Hansen , Ingo Molnar , Andreas Dilger , Alexander Viro , linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Andrew Morton , linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org To: Jan Kara Return-path: Content-Disposition: inline In-Reply-To: <20170615145856.GO1764-4I4JzKEfoa/jFM9bn6wA6Q@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" List-Id: linux-ext4.vger.kernel.org On Thu, Jun 15, 2017 at 04:58:56PM +0200, Jan Kara wrote: > On Wed 14-06-17 11:22:11, Ross Zwisler wrote: > > @@ -216,17 +217,6 @@ static void dax_unlock_mapping_entry(struct address_space *mapping, > > dax_wake_mapping_entry_waiter(mapping, index, entry, false); > > } > > > > -static void put_locked_mapping_entry(struct address_space *mapping, > > - pgoff_t index, void *entry) > > -{ > > - if (!radix_tree_exceptional_entry(entry)) { > > - unlock_page(entry); > > - put_page(entry); > > - } else { > > - dax_unlock_mapping_entry(mapping, index); > > - } > > -} > > - > > The naming becomes asymetric with this. So I'd prefer keeping > put_locked_mapping_entry() as a trivial wrapper around > dax_unlock_mapping_entry() unless we can craft more sensible naming / API > for entry grabbing (and that would be a separate patch anyway). Sure, that works for me. I'll fix for v3. > > -static int dax_load_hole(struct address_space *mapping, void **entry, > > +static int dax_load_hole(struct address_space *mapping, void *entry, > > struct vm_fault *vmf) > > { > > struct inode *inode = mapping->host; > > - struct page *page; > > - int ret; > > - > > - /* Hole page already exists? Return it... */ > > - if (!radix_tree_exceptional_entry(*entry)) { > > - page = *entry; > > - goto finish_fault; > > - } > > + unsigned long vaddr = vmf->address; > > + int ret = VM_FAULT_NOPAGE; > > + struct page *zero_page; > > + void *entry2; > > > > - /* This will replace locked radix tree entry with a hole page */ > > - page = find_or_create_page(mapping, vmf->pgoff, > > - vmf->gfp_mask | __GFP_ZERO); > > With this gone, you can also remove the special DAX handling from > mm/filemap.c: page_cache_tree_insert() and remove from dax.h > dax_wake_mapping_entry_waiter(), dax_radix_locked_entry() and RADIX_DAX > definitions. Yay! As a separate patch please. Oh, yay! :) Sure, I'll have this patch for v3. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 4FB1021A07A81 for ; Fri, 16 Jun 2017 12:44:23 -0700 (PDT) Date: Fri, 16 Jun 2017 13:45:40 -0600 From: Ross Zwisler Subject: Re: [PATCH v2 3/3] dax: use common 4k zero page for dax mmap reads Message-ID: <20170616194540.GB20742@linux.intel.com> References: <20170614172211.19820-1-ross.zwisler@linux.intel.com> <20170614172211.19820-4-ross.zwisler@linux.intel.com> <20170615145856.GO1764@quack2.suse.cz> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170615145856.GO1764@quack2.suse.cz> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-nvdimm-bounces@lists.01.org Sender: "Linux-nvdimm" To: Jan Kara Cc: linux-xfs@vger.kernel.org, Theodore Ts'o , Matthew Wilcox , linux-doc@vger.kernel.org, "Darrick J. Wong" , Jonathan Corbet , linux-kernel@vger.kernel.org, Steven Rostedt , Christoph Hellwig , linux-mm@kvack.org, Dave Hansen , Ingo Molnar , Andreas Dilger , Alexander Viro , linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org, Andrew Morton , linux-nvdimm@lists.01.org List-ID: On Thu, Jun 15, 2017 at 04:58:56PM +0200, Jan Kara wrote: > On Wed 14-06-17 11:22:11, Ross Zwisler wrote: > > @@ -216,17 +217,6 @@ static void dax_unlock_mapping_entry(struct address_space *mapping, > > dax_wake_mapping_entry_waiter(mapping, index, entry, false); > > } > > > > -static void put_locked_mapping_entry(struct address_space *mapping, > > - pgoff_t index, void *entry) > > -{ > > - if (!radix_tree_exceptional_entry(entry)) { > > - unlock_page(entry); > > - put_page(entry); > > - } else { > > - dax_unlock_mapping_entry(mapping, index); > > - } > > -} > > - > > The naming becomes asymetric with this. So I'd prefer keeping > put_locked_mapping_entry() as a trivial wrapper around > dax_unlock_mapping_entry() unless we can craft more sensible naming / API > for entry grabbing (and that would be a separate patch anyway). Sure, that works for me. I'll fix for v3. > > -static int dax_load_hole(struct address_space *mapping, void **entry, > > +static int dax_load_hole(struct address_space *mapping, void *entry, > > struct vm_fault *vmf) > > { > > struct inode *inode = mapping->host; > > - struct page *page; > > - int ret; > > - > > - /* Hole page already exists? Return it... */ > > - if (!radix_tree_exceptional_entry(*entry)) { > > - page = *entry; > > - goto finish_fault; > > - } > > + unsigned long vaddr = vmf->address; > > + int ret = VM_FAULT_NOPAGE; > > + struct page *zero_page; > > + void *entry2; > > > > - /* This will replace locked radix tree entry with a hole page */ > > - page = find_or_create_page(mapping, vmf->pgoff, > > - vmf->gfp_mask | __GFP_ZERO); > > With this gone, you can also remove the special DAX handling from > mm/filemap.c: page_cache_tree_insert() and remove from dax.h > dax_wake_mapping_entry_waiter(), dax_radix_locked_entry() and RADIX_DAX > definitions. Yay! As a separate patch please. Oh, yay! :) Sure, I'll have this patch for v3. _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com ([134.134.136.24]:42197 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750903AbdFPTpn (ORCPT ); Fri, 16 Jun 2017 15:45:43 -0400 Date: Fri, 16 Jun 2017 13:45:40 -0600 From: Ross Zwisler Subject: Re: [PATCH v2 3/3] dax: use common 4k zero page for dax mmap reads Message-ID: <20170616194540.GB20742@linux.intel.com> References: <20170614172211.19820-1-ross.zwisler@linux.intel.com> <20170614172211.19820-4-ross.zwisler@linux.intel.com> <20170615145856.GO1764@quack2.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170615145856.GO1764@quack2.suse.cz> Sender: linux-xfs-owner@vger.kernel.org List-ID: List-Id: xfs To: Jan Kara Cc: Ross Zwisler , Andrew Morton , linux-kernel@vger.kernel.org, "Darrick J. Wong" , Theodore Ts'o , Alexander Viro , Andreas Dilger , Christoph Hellwig , Dan Williams , Dave Hansen , Ingo Molnar , Jonathan Corbet , Matthew Wilcox , Steven Rostedt , linux-doc@vger.kernel.org, linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-nvdimm@lists.01.org, linux-xfs@vger.kernel.org On Thu, Jun 15, 2017 at 04:58:56PM +0200, Jan Kara wrote: > On Wed 14-06-17 11:22:11, Ross Zwisler wrote: > > @@ -216,17 +217,6 @@ static void dax_unlock_mapping_entry(struct address_space *mapping, > > dax_wake_mapping_entry_waiter(mapping, index, entry, false); > > } > > > > -static void put_locked_mapping_entry(struct address_space *mapping, > > - pgoff_t index, void *entry) > > -{ > > - if (!radix_tree_exceptional_entry(entry)) { > > - unlock_page(entry); > > - put_page(entry); > > - } else { > > - dax_unlock_mapping_entry(mapping, index); > > - } > > -} > > - > > The naming becomes asymetric with this. So I'd prefer keeping > put_locked_mapping_entry() as a trivial wrapper around > dax_unlock_mapping_entry() unless we can craft more sensible naming / API > for entry grabbing (and that would be a separate patch anyway). Sure, that works for me. I'll fix for v3. > > -static int dax_load_hole(struct address_space *mapping, void **entry, > > +static int dax_load_hole(struct address_space *mapping, void *entry, > > struct vm_fault *vmf) > > { > > struct inode *inode = mapping->host; > > - struct page *page; > > - int ret; > > - > > - /* Hole page already exists? Return it... */ > > - if (!radix_tree_exceptional_entry(*entry)) { > > - page = *entry; > > - goto finish_fault; > > - } > > + unsigned long vaddr = vmf->address; > > + int ret = VM_FAULT_NOPAGE; > > + struct page *zero_page; > > + void *entry2; > > > > - /* This will replace locked radix tree entry with a hole page */ > > - page = find_or_create_page(mapping, vmf->pgoff, > > - vmf->gfp_mask | __GFP_ZERO); > > With this gone, you can also remove the special DAX handling from > mm/filemap.c: page_cache_tree_insert() and remove from dax.h > dax_wake_mapping_entry_waiter(), dax_radix_locked_entry() and RADIX_DAX > definitions. Yay! As a separate patch please. Oh, yay! :) Sure, I'll have this patch for v3. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Fri, 16 Jun 2017 13:45:40 -0600 From: Ross Zwisler To: Jan Kara Cc: Ross Zwisler , Andrew Morton , linux-kernel@vger.kernel.org, "Darrick J. Wong" , Theodore Ts'o , Alexander Viro , Andreas Dilger , Christoph Hellwig , Dan Williams , Dave Hansen , Ingo Molnar , Jonathan Corbet , Matthew Wilcox , Steven Rostedt , linux-doc@vger.kernel.org, linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-nvdimm@lists.01.org, linux-xfs@vger.kernel.org Subject: Re: [PATCH v2 3/3] dax: use common 4k zero page for dax mmap reads Message-ID: <20170616194540.GB20742@linux.intel.com> References: <20170614172211.19820-1-ross.zwisler@linux.intel.com> <20170614172211.19820-4-ross.zwisler@linux.intel.com> <20170615145856.GO1764@quack2.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170615145856.GO1764@quack2.suse.cz> Sender: owner-linux-mm@kvack.org List-ID: On Thu, Jun 15, 2017 at 04:58:56PM +0200, Jan Kara wrote: > On Wed 14-06-17 11:22:11, Ross Zwisler wrote: > > @@ -216,17 +217,6 @@ static void dax_unlock_mapping_entry(struct address_space *mapping, > > dax_wake_mapping_entry_waiter(mapping, index, entry, false); > > } > > > > -static void put_locked_mapping_entry(struct address_space *mapping, > > - pgoff_t index, void *entry) > > -{ > > - if (!radix_tree_exceptional_entry(entry)) { > > - unlock_page(entry); > > - put_page(entry); > > - } else { > > - dax_unlock_mapping_entry(mapping, index); > > - } > > -} > > - > > The naming becomes asymetric with this. So I'd prefer keeping > put_locked_mapping_entry() as a trivial wrapper around > dax_unlock_mapping_entry() unless we can craft more sensible naming / API > for entry grabbing (and that would be a separate patch anyway). Sure, that works for me. I'll fix for v3. > > -static int dax_load_hole(struct address_space *mapping, void **entry, > > +static int dax_load_hole(struct address_space *mapping, void *entry, > > struct vm_fault *vmf) > > { > > struct inode *inode = mapping->host; > > - struct page *page; > > - int ret; > > - > > - /* Hole page already exists? Return it... */ > > - if (!radix_tree_exceptional_entry(*entry)) { > > - page = *entry; > > - goto finish_fault; > > - } > > + unsigned long vaddr = vmf->address; > > + int ret = VM_FAULT_NOPAGE; > > + struct page *zero_page; > > + void *entry2; > > > > - /* This will replace locked radix tree entry with a hole page */ > > - page = find_or_create_page(mapping, vmf->pgoff, > > - vmf->gfp_mask | __GFP_ZERO); > > With this gone, you can also remove the special DAX handling from > mm/filemap.c: page_cache_tree_insert() and remove from dax.h > dax_wake_mapping_entry_waiter(), dax_radix_locked_entry() and RADIX_DAX > definitions. Yay! As a separate patch please. Oh, yay! :) Sure, I'll have this patch for v3. -- 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