On Tue, Sep 27, 2016 at 06:08:14PM +0200, 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.
> 
> Signed-off-by: Jan Kara <j...@suse.cz>
> ---
>  fs/dax.c            | 22 ++++++++++++++++------
>  include/linux/dax.h |  7 -------
>  include/linux/mm.h  |  9 +--------
>  mm/memory.c         | 14 ++++----------
>  4 files changed, 21 insertions(+), 31 deletions(-)
> 
> diff --git a/fs/dax.c b/fs/dax.c
> index 0dc251ca77b8..b1c503930d1d 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -876,10 +876,15 @@ int dax_fault(struct vm_area_struct *vma, struct 
> vm_fault *vmf,
>                       goto unlock_entry;
>               if (!radix_tree_exceptional_entry(entry)) {
>                       vmf->page = entry;
> -                     return VM_FAULT_LOCKED;
> +                     if (unlikely(PageHWPoison(entry))) {
> +                             put_locked_mapping_entry(mapping, vmf->pgoff,
> +                                                      entry);
> +                             return VM_FAULT_HWPOISON;
> +                     }
>               }
> -             vmf->entry = entry;
> -             return VM_FAULT_DAX_LOCKED;
> +             error = finish_fault(vmf);
> +             put_locked_mapping_entry(mapping, vmf->pgoff, entry);
> +             return error ? error : VM_FAULT_DONE_COW;
>       }
>  
>       if (!buffer_mapped(&bh)) {
> @@ -1430,10 +1435,15 @@ int iomap_dax_fault(struct vm_area_struct *vma, 
> struct vm_fault *vmf,
>                       goto unlock_entry;
>               if (!radix_tree_exceptional_entry(entry)) {
>                       vmf->page = entry;

In __do_fault() we explicitly clear vmf->page in the case where PageHWPoison()
is set.  I think we can get the same behavior here by moving the call that
sets vmf->page after the PageHWPoison() check.

> -                     return VM_FAULT_LOCKED;
> +                     if (unlikely(PageHWPoison(entry))) {
> +                             put_locked_mapping_entry(mapping, vmf->pgoff,
> +                                                      entry);
> +                             return VM_FAULT_HWPOISON;
> +                     }
>               }
> -             vmf->entry = entry;
> -             return VM_FAULT_DAX_LOCKED;

I think we're missing a call to 

        __SetPageUptodate(new_page);

before finish_fault()?  This call currently lives in do_cow_fault(), and
is part of the path that we don't skip as part of the VM_FAULT_DAX_LOCKED
logic.

Both of these comments apply equally to the iomap_dax_fault() code.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Reply via email to