Re: [PATCH 1/8] mm/memory-failure: Remove fsdax_pgoff argument from __add_to_kill

[Miaohe Lin <linmiaohe@huawei.com>]

On 2024/3/1 5:20, Matthew Wilcox (Oracle) wrote:
> Unify the KSM and DAX codepaths by calculating the addr in
> add_to_kill_fsdax() instead of telling __add_to_kill() to calculate it.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> Cc: Dan Williams <dan.j.williams@intel.com>

This patch looks good to me. Thanks.

Acked-by: Miaohe Lin <linmiaohe@huawei.com>

> ---
>  mm/memory-failure.c | 27 +++++++++------------------
>  1 file changed, 9 insertions(+), 18 deletions(-)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 9349948f1abf..9356227a50bb 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -416,21 +416,13 @@ static unsigned long dev_pagemap_mapping_shift(struct vm_area_struct *vma,
>   * not much we can do.	We just print a message and ignore otherwise.
>   */
>  
> -#define FSDAX_INVALID_PGOFF ULONG_MAX
> -
>  /*
>   * Schedule a process for later kill.
>   * Uses GFP_ATOMIC allocations to avoid potential recursions in the VM.
> - *
> - * Note: @fsdax_pgoff is used only when @p is a fsdax page and a
> - * filesystem with a memory failure handler has claimed the
> - * memory_failure event. In all other cases, page->index and
> - * page->mapping are sufficient for mapping the page back to its
> - * corresponding user virtual address.
>   */
>  static void __add_to_kill(struct task_struct *tsk, struct page *p,
>  			  struct vm_area_struct *vma, struct list_head *to_kill,
> -			  unsigned long ksm_addr, pgoff_t fsdax_pgoff)
> +			  unsigned long addr)
>  {
>  	struct to_kill *tk;
>  
> @@ -440,12 +432,10 @@ static void __add_to_kill(struct task_struct *tsk, struct page *p,
>  		return;
>  	}
>  
> -	tk->addr = ksm_addr ? ksm_addr : page_address_in_vma(p, vma);
> -	if (is_zone_device_page(p)) {
> -		if (fsdax_pgoff != FSDAX_INVALID_PGOFF)
> -			tk->addr = vma_pgoff_address(fsdax_pgoff, 1, vma);
> +	tk->addr = addr ? addr : page_address_in_vma(p, vma);
> +	if (is_zone_device_page(p))
>  		tk->size_shift = dev_pagemap_mapping_shift(vma, tk->addr);
> -	} else
> +	else
>  		tk->size_shift = page_shift(compound_head(p));
>  
>  	/*
> @@ -475,7 +465,7 @@ static void add_to_kill_anon_file(struct task_struct *tsk, struct page *p,
>  				  struct vm_area_struct *vma,
>  				  struct list_head *to_kill)
>  {
> -	__add_to_kill(tsk, p, vma, to_kill, 0, FSDAX_INVALID_PGOFF);
> +	__add_to_kill(tsk, p, vma, to_kill, 0);
>  }
>  
>  #ifdef CONFIG_KSM
> @@ -493,10 +483,10 @@ static bool task_in_to_kill_list(struct list_head *to_kill,
>  }
>  void add_to_kill_ksm(struct task_struct *tsk, struct page *p,
>  		     struct vm_area_struct *vma, struct list_head *to_kill,
> -		     unsigned long ksm_addr)
> +		     unsigned long addr)
>  {
>  	if (!task_in_to_kill_list(to_kill, tsk))
> -		__add_to_kill(tsk, p, vma, to_kill, ksm_addr, FSDAX_INVALID_PGOFF);
> +		__add_to_kill(tsk, p, vma, to_kill, addr);
>  }
>  #endif
>  /*
> @@ -670,7 +660,8 @@ static void add_to_kill_fsdax(struct task_struct *tsk, struct page *p,
>  			      struct vm_area_struct *vma,
>  			      struct list_head *to_kill, pgoff_t pgoff)
>  {
> -	__add_to_kill(tsk, p, vma, to_kill, 0, pgoff);
> +	unsigned long addr = vma_pgoff_address(pgoff, 1, vma);
> +	__add_to_kill(tsk, p, vma, to_kill, addr);
>  }
>  
>  /*
>