Received-SPF: pass (google.com: domain of linux-kernel+bounces-178345-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <2ec286a2-4620-4e6d-ad3d-9b4c0d9e1394@linux.alibaba.com>
Date: Tue, 14 May 2024 14:26:44 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH RESEND v5 1/4] mm/rmap: remove duplicated exit code in
 pagewalk loop
To: Lance Yang <ioworker0@gmail.com>, akpm@linux-foundation.org
Cc: willy@infradead.org, sj@kernel.org, maskray@google.com, ziy@nvidia.com,
 ryan.roberts@arm.com, david@redhat.com, 21cnbao@gmail.com, mhocko@suse.com,
 fengwei.yin@intel.com, zokeefe@google.com, shy828301@gmail.com,
 xiehuan09@gmail.com, libang.li@antgroup.com, wangkefeng.wang@huawei.com,
 songmuchun@bytedance.com, peterx@redhat.com, minchan@kernel.org,
 linux-mm@kvack.org, linux-kernel@vger.kernel.org
References: <20240513074712.7608-1-ioworker0@gmail.com>
 <20240513074712.7608-2-ioworker0@gmail.com>
From: Baolin Wang <baolin.wang@linux.alibaba.com>
In-Reply-To: <20240513074712.7608-2-ioworker0@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


On 2024/5/13 15:47, Lance Yang wrote:
> Introduce the labels walk_done and walk_done_err as exit points to
> eliminate duplicated exit code in the pagewalk loop.
> 
> Reviewed-by: Zi Yan <ziy@nvidia.com>
> Signed-off-by: Lance Yang <ioworker0@gmail.com>

LGTM.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

> ---
>   mm/rmap.c | 40 +++++++++++++++-------------------------
>   1 file changed, 15 insertions(+), 25 deletions(-)
> 
> diff --git a/mm/rmap.c b/mm/rmap.c
> index e8fc5ecb59b2..ddffa30c79fb 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1679,9 +1679,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>   			/* Restore the mlock which got missed */
>   			if (!folio_test_large(folio))
>   				mlock_vma_folio(folio, vma);
> -			page_vma_mapped_walk_done(&pvmw);
> -			ret = false;
> -			break;
> +			goto walk_done_err;
>   		}
>   
>   		pfn = pte_pfn(ptep_get(pvmw.pte));
> @@ -1719,11 +1717,8 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>   			 */
>   			if (!anon) {
>   				VM_BUG_ON(!(flags & TTU_RMAP_LOCKED));
> -				if (!hugetlb_vma_trylock_write(vma)) {
> -					page_vma_mapped_walk_done(&pvmw);
> -					ret = false;
> -					break;
> -				}
> +				if (!hugetlb_vma_trylock_write(vma))
> +					goto walk_done_err;
>   				if (huge_pmd_unshare(mm, vma, address, pvmw.pte)) {
>   					hugetlb_vma_unlock_write(vma);
>   					flush_tlb_range(vma,
> @@ -1738,8 +1733,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>   					 * actual page and drop map count
>   					 * to zero.
>   					 */
> -					page_vma_mapped_walk_done(&pvmw);
> -					break;
> +					goto walk_done;
>   				}
>   				hugetlb_vma_unlock_write(vma);
>   			}
> @@ -1811,9 +1805,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>   			if (unlikely(folio_test_swapbacked(folio) !=
>   					folio_test_swapcache(folio))) {
>   				WARN_ON_ONCE(1);
> -				ret = false;
> -				page_vma_mapped_walk_done(&pvmw);
> -				break;
> +				goto walk_done_err;
>   			}
>   
>   			/* MADV_FREE page check */
> @@ -1852,23 +1844,17 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>   				 */
>   				set_pte_at(mm, address, pvmw.pte, pteval);
>   				folio_set_swapbacked(folio);
> -				ret = false;
> -				page_vma_mapped_walk_done(&pvmw);
> -				break;
> +				goto walk_done_err;
>   			}
>   
>   			if (swap_duplicate(entry) < 0) {
>   				set_pte_at(mm, address, pvmw.pte, pteval);
> -				ret = false;
> -				page_vma_mapped_walk_done(&pvmw);
> -				break;
> +				goto walk_done_err;
>   			}
>   			if (arch_unmap_one(mm, vma, address, pteval) < 0) {
>   				swap_free(entry);
>   				set_pte_at(mm, address, pvmw.pte, pteval);
> -				ret = false;
> -				page_vma_mapped_walk_done(&pvmw);
> -				break;
> +				goto walk_done_err;
>   			}
>   
>   			/* See folio_try_share_anon_rmap(): clear PTE first. */
> @@ -1876,9 +1862,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>   			    folio_try_share_anon_rmap_pte(folio, subpage)) {
>   				swap_free(entry);
>   				set_pte_at(mm, address, pvmw.pte, pteval);
> -				ret = false;
> -				page_vma_mapped_walk_done(&pvmw);
> -				break;
> +				goto walk_done_err;
>   			}
>   			if (list_empty(&mm->mmlist)) {
>   				spin_lock(&mmlist_lock);
> @@ -1918,6 +1902,12 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>   		if (vma->vm_flags & VM_LOCKED)
>   			mlock_drain_local();
>   		folio_put(folio);
> +		continue;
> +walk_done_err:
> +		ret = false;
> +walk_done:
> +		page_vma_mapped_walk_done(&pvmw);
> +		break;
>   	}
>   
>   	mmu_notifier_invalidate_range_end(&range);