On Fri, Nov 04, 2016 at 05:25:06AM +0100, Jan Kara wrote:
> Move final handling of COW faults from generic code into DAX fault
> handler. That way generic code doesn't have to be aware of peculiarities
> of DAX locking so remove that knowledge and make locking functions
> private to fs/dax.c.
>
> Signed-off-by: Jan Kara <jack@xxxxxxx>
> ---
> @@ -1006,13 +1007,14 @@ int dax_iomap_fault(struct vm_area_struct *vma, struct vm_fault *vmf,
>
> if (error)
> goto finish_iomap;
> - if (!radix_tree_exceptional_entry(entry)) {
> +
> + __SetPageUptodate(vmf->cow_page);
> + if (!radix_tree_exceptional_entry(entry))
> vmf->page = entry;
I don't think we need to set vmf->page anymore. We would clear it to NULL in
a few lines anyway, and the only call in between is finish_fault(), which
only cares about vmf->cow_page(). This allows us to remove the vmf->page =
NULL line a few lines below as well.
> @@ -1051,7 +1053,7 @@ int dax_iomap_fault(struct vm_area_struct *vma, struct vm_fault *vmf,
> }
> }
> unlock_entry:
> - if (!locked_status || error)
> + if (vmf_ret != VM_FAULT_LOCKED || error)
> put_locked_mapping_entry(mapping, vmf->pgoff, entry);
I don't think this is quite right. For example, for dax_load_hole(), if we
can't get a page we put_locked_mapping_entry() and return VM_FAULT_OOM.
Previously this logic would have skipped the second call to
put_locked_mapping_entry(), but now with the strict check against
VM_FAULT_LOCKED put the entry twice.
Maybe the right thing to do is just fix dax_load_hole() so it never calls
put_locked_mapping_entry(), and leave this check as you have it?
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
