Re: [PATCH v5 13/17] dax: dax_iomap_fault() needs to call iomap_end()

[Jan Kara]

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 <ross.zwisler@xxxxxxxxxxxxxxx>
> Suggested-by: Jan Kara <jack@xxxxxxx>
...
> @@ -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 <jack@xxxxxxx>

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR