Re: [PATCH v2 4/5] mm/swap: remove confusing checking for non_swap_entry() in swap_ra_info()

[Miaohe Lin <linmiaohe@huawei.com>]

On 2021/4/19 9:53, Huang, Ying wrote:
> Miaohe Lin <linmiaohe@huawei.com> writes:
> 
>> While we released the pte lock, somebody else might faulted in this pte.
>> So we should check whether it's swap pte first to guard against such race
>> or swp_type would be unexpected. But the swap_entry isn't used in this
>> function and we will have enough checking when we really operate the PTE
>> entries later. So checking for non_swap_entry() is not really needed here
>> and should be removed to avoid confusion.
> 
> Please rephrase the change log to describe why we have the code and why
> it's unnecessary now.  You can dig the git history via git-blame to find
> out it.
> 

Will try to do it. Thanks.

> The patch itself looks good to me.
> 
> Best Regards,
> Huang, Ying
> 
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>> ---
>>  mm/swap_state.c | 6 ------
>>  1 file changed, 6 deletions(-)
>>
>> diff --git a/mm/swap_state.c b/mm/swap_state.c
>> index 272ea2108c9d..df5405384520 100644
>> --- a/mm/swap_state.c
>> +++ b/mm/swap_state.c
>> @@ -721,7 +721,6 @@ static void swap_ra_info(struct vm_fault *vmf,
>>  {
>>  	struct vm_area_struct *vma = vmf->vma;
>>  	unsigned long ra_val;
>> -	swp_entry_t entry;
>>  	unsigned long faddr, pfn, fpfn;
>>  	unsigned long start, end;
>>  	pte_t *pte, *orig_pte;
>> @@ -739,11 +738,6 @@ static void swap_ra_info(struct vm_fault *vmf,
>>  
>>  	faddr = vmf->address;
>>  	orig_pte = pte = pte_offset_map(vmf->pmd, faddr);
>> -	entry = pte_to_swp_entry(*pte);
>> -	if ((unlikely(non_swap_entry(entry)))) {
>> -		pte_unmap(orig_pte);
>> -		return;
>> -	}
>>  
>>  	fpfn = PFN_DOWN(faddr);
>>  	ra_val = GET_SWAP_RA_VAL(vma);
> .
>