From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Subject: Re: [PATCH v5 13/17] dax: dax_iomap_fault() needs to call iomap_end() Date: Tue, 11 Oct 2016 09:21:47 +0200 Message-ID: <20161011072147.GE6952@quack2.suse.cz> References: <1475874544-24842-1-git-send-email-ross.zwisler@linux.intel.com> <1475874544-24842-14-git-send-email-ross.zwisler@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: Theodore Ts'o , Matthew Wilcox , Dave Chinner , linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-xfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, Andreas Dilger , Alexander Viro , Jan Kara , linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Andrew Morton , linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Christoph Hellwig To: Ross Zwisler Return-path: Content-Disposition: inline In-Reply-To: <1475874544-24842-14-git-send-email-ross.zwisler-VuQAYsv1563Yd54FQh9/CA@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 Fri 07-10-16 15:09:00, Ross Zwisler wrote: > Currently iomap_end() doesn't do anything for DAX page faults for both ext2 > and XFS. ext2_iomap_end() just checks for a write underrun, and > xfs_file_iomap_end() checks to see if it needs to finish a delayed > allocation. However, in the future iomap_end() calls might be needed to > make sure we have balanced allocations, locks, etc. So, add calls to > iomap_end() with appropriate error handling to dax_iomap_fault(). > > Signed-off-by: Ross Zwisler > Suggested-by: Jan Kara ... > @@ -1239,6 +1253,17 @@ int dax_iomap_fault(struct vm_area_struct *vma, struct vm_fault *vmf, > break; > } > > + finish_iomap: > + if (ops->iomap_end) { > + if (error) { > + /* keep previous error */ > + ops->iomap_end(inode, pos, PAGE_SIZE, PAGE_SIZE, flags, > + &iomap); I think for the error case we should set number of 'written' bytes to 0 to tell fs to cancel what it has prepared. This is mostly cosmetic since the only case where I can imagine this would matter is shared write fault and in that case we have currently no error path but still it could bite us in the future. Other than that the patch looks good so you can add: Reviewed-by: Jan Kara Honza -- Jan Kara SUSE Labs, CR From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 5F26B1A1EC7 for ; Tue, 11 Oct 2016 11:42:33 -0700 (PDT) Date: Tue, 11 Oct 2016 09:21:47 +0200 From: Jan Kara Subject: Re: [PATCH v5 13/17] dax: dax_iomap_fault() needs to call iomap_end() Message-ID: <20161011072147.GE6952@quack2.suse.cz> References: <1475874544-24842-1-git-send-email-ross.zwisler@linux.intel.com> <1475874544-24842-14-git-send-email-ross.zwisler@linux.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1475874544-24842-14-git-send-email-ross.zwisler@linux.intel.com> 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: Ross Zwisler Cc: Theodore Ts'o , Matthew Wilcox , Dave Chinner , linux-nvdimm@lists.01.org, linux-kernel@vger.kernel.org, linux-xfs@vger.kernel.org, linux-mm@kvack.org, Andreas Dilger , Alexander Viro , Jan Kara , linux-fsdevel@vger.kernel.org, Andrew Morton , linux-ext4@vger.kernel.org, Christoph Hellwig List-ID: On Fri 07-10-16 15:09:00, Ross Zwisler wrote: > Currently iomap_end() doesn't do anything for DAX page faults for both ext2 > and XFS. ext2_iomap_end() just checks for a write underrun, and > xfs_file_iomap_end() checks to see if it needs to finish a delayed > allocation. However, in the future iomap_end() calls might be needed to > make sure we have balanced allocations, locks, etc. So, add calls to > iomap_end() with appropriate error handling to dax_iomap_fault(). > > Signed-off-by: Ross Zwisler > Suggested-by: Jan Kara ... > @@ -1239,6 +1253,17 @@ int dax_iomap_fault(struct vm_area_struct *vma, struct vm_fault *vmf, > break; > } > > + finish_iomap: > + if (ops->iomap_end) { > + if (error) { > + /* keep previous error */ > + ops->iomap_end(inode, pos, PAGE_SIZE, PAGE_SIZE, flags, > + &iomap); I think for the error case we should set number of 'written' bytes to 0 to tell fs to cancel what it has prepared. This is mostly cosmetic since the only case where I can imagine this would matter is shared write fault and in that case we have currently no error path but still it could bite us in the future. Other than that the patch looks good so you can add: Reviewed-by: Jan Kara Honza -- Jan Kara SUSE Labs, CR _______________________________________________ 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 mx2.suse.de ([195.135.220.15]:42817 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753535AbcJKSnj (ORCPT ); Tue, 11 Oct 2016 14:43:39 -0400 Date: Tue, 11 Oct 2016 09:21:47 +0200 From: Jan Kara Subject: Re: [PATCH v5 13/17] dax: dax_iomap_fault() needs to call iomap_end() Message-ID: <20161011072147.GE6952@quack2.suse.cz> References: <1475874544-24842-1-git-send-email-ross.zwisler@linux.intel.com> <1475874544-24842-14-git-send-email-ross.zwisler@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1475874544-24842-14-git-send-email-ross.zwisler@linux.intel.com> Sender: linux-xfs-owner@vger.kernel.org List-ID: List-Id: xfs To: Ross Zwisler Cc: linux-kernel@vger.kernel.org, Theodore Ts'o , Alexander Viro , Andreas Dilger , Andrew Morton , Christoph Hellwig , Dan Williams , Dave Chinner , Jan Kara , Matthew Wilcox , 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 Fri 07-10-16 15:09:00, Ross Zwisler wrote: > Currently iomap_end() doesn't do anything for DAX page faults for both ext2 > and XFS. ext2_iomap_end() just checks for a write underrun, and > xfs_file_iomap_end() checks to see if it needs to finish a delayed > allocation. However, in the future iomap_end() calls might be needed to > make sure we have balanced allocations, locks, etc. So, add calls to > iomap_end() with appropriate error handling to dax_iomap_fault(). > > Signed-off-by: Ross Zwisler > Suggested-by: Jan Kara ... > @@ -1239,6 +1253,17 @@ int dax_iomap_fault(struct vm_area_struct *vma, struct vm_fault *vmf, > break; > } > > + finish_iomap: > + if (ops->iomap_end) { > + if (error) { > + /* keep previous error */ > + ops->iomap_end(inode, pos, PAGE_SIZE, PAGE_SIZE, flags, > + &iomap); I think for the error case we should set number of 'written' bytes to 0 to tell fs to cancel what it has prepared. This is mostly cosmetic since the only case where I can imagine this would matter is shared write fault and in that case we have currently no error path but still it could bite us in the future. Other than that the patch looks good so you can add: Reviewed-by: Jan Kara Honza -- Jan Kara SUSE Labs, CR From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 11 Oct 2016 09:21:47 +0200 From: Jan Kara To: Ross Zwisler Cc: linux-kernel@vger.kernel.org, Theodore Ts'o , Alexander Viro , Andreas Dilger , Andrew Morton , Christoph Hellwig , Dan Williams , Dave Chinner , Jan Kara , Matthew Wilcox , 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 v5 13/17] dax: dax_iomap_fault() needs to call iomap_end() Message-ID: <20161011072147.GE6952@quack2.suse.cz> References: <1475874544-24842-1-git-send-email-ross.zwisler@linux.intel.com> <1475874544-24842-14-git-send-email-ross.zwisler@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1475874544-24842-14-git-send-email-ross.zwisler@linux.intel.com> Sender: owner-linux-mm@kvack.org List-ID: On Fri 07-10-16 15:09:00, Ross Zwisler wrote: > Currently iomap_end() doesn't do anything for DAX page faults for both ext2 > and XFS. ext2_iomap_end() just checks for a write underrun, and > xfs_file_iomap_end() checks to see if it needs to finish a delayed > allocation. However, in the future iomap_end() calls might be needed to > make sure we have balanced allocations, locks, etc. So, add calls to > iomap_end() with appropriate error handling to dax_iomap_fault(). > > Signed-off-by: Ross Zwisler > Suggested-by: Jan Kara ... > @@ -1239,6 +1253,17 @@ int dax_iomap_fault(struct vm_area_struct *vma, struct vm_fault *vmf, > break; > } > > + finish_iomap: > + if (ops->iomap_end) { > + if (error) { > + /* keep previous error */ > + ops->iomap_end(inode, pos, PAGE_SIZE, PAGE_SIZE, flags, > + &iomap); I think for the error case we should set number of 'written' bytes to 0 to tell fs to cancel what it has prepared. This is mostly cosmetic since the only case where I can imagine this would matter is shared write fault and in that case we have currently no error path but still it could bite us in the future. Other than that the patch looks good so you can add: Reviewed-by: Jan Kara Honza -- Jan Kara SUSE Labs, CR -- 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754362AbcJKSoh (ORCPT ); Tue, 11 Oct 2016 14:44:37 -0400 Received: from mx2.suse.de ([195.135.220.15]:42817 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753535AbcJKSnj (ORCPT ); Tue, 11 Oct 2016 14:43:39 -0400 Date: Tue, 11 Oct 2016 09:21:47 +0200 From: Jan Kara To: Ross Zwisler Cc: linux-kernel@vger.kernel.org, "Theodore Ts'o" , Alexander Viro , Andreas Dilger , Andrew Morton , Christoph Hellwig , Dan Williams , Dave Chinner , Jan Kara , Matthew Wilcox , linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-nvdimm@ml01.01.org, linux-xfs@vger.kernel.org Subject: Re: [PATCH v5 13/17] dax: dax_iomap_fault() needs to call iomap_end() Message-ID: <20161011072147.GE6952@quack2.suse.cz> References: <1475874544-24842-1-git-send-email-ross.zwisler@linux.intel.com> <1475874544-24842-14-git-send-email-ross.zwisler@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1475874544-24842-14-git-send-email-ross.zwisler@linux.intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri 07-10-16 15:09:00, Ross Zwisler wrote: > Currently iomap_end() doesn't do anything for DAX page faults for both ext2 > and XFS. ext2_iomap_end() just checks for a write underrun, and > xfs_file_iomap_end() checks to see if it needs to finish a delayed > allocation. However, in the future iomap_end() calls might be needed to > make sure we have balanced allocations, locks, etc. So, add calls to > iomap_end() with appropriate error handling to dax_iomap_fault(). > > Signed-off-by: Ross Zwisler > Suggested-by: Jan Kara ... > @@ -1239,6 +1253,17 @@ int dax_iomap_fault(struct vm_area_struct *vma, struct vm_fault *vmf, > break; > } > > + finish_iomap: > + if (ops->iomap_end) { > + if (error) { > + /* keep previous error */ > + ops->iomap_end(inode, pos, PAGE_SIZE, PAGE_SIZE, flags, > + &iomap); I think for the error case we should set number of 'written' bytes to 0 to tell fs to cancel what it has prepared. This is mostly cosmetic since the only case where I can imagine this would matter is shared write fault and in that case we have currently no error path but still it could bite us in the future. Other than that the patch looks good so you can add: Reviewed-by: Jan Kara Honza -- Jan Kara SUSE Labs, CR