From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 72B4F7E for ; Mon, 6 Mar 2023 01:12:48 +0000 (UTC) Received: from dggpemm500001.china.huawei.com (unknown [172.30.72.56]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4PVL8L1x7gzSkgG; Mon, 6 Mar 2023 09:09:38 +0800 (CST) Received: from [10.174.177.243] (10.174.177.243) by dggpemm500001.china.huawei.com (7.185.36.107) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.21; Mon, 6 Mar 2023 09:12:39 +0800 Message-ID: Date: Mon, 6 Mar 2023 09:12:38 +0800 Precedence: bulk X-Mailing-List: damon@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.5.1 Subject: Re: [PATCH v2 3/3] mm/damon/paddr: minor refactor of damon_pa_mark_accessed_or_deactivate() Content-Language: en-US To: SeongJae Park CC: Andrew Morton , , , References: <20230303183722.113464-1-sj@kernel.org> From: Kefeng Wang In-Reply-To: <20230303183722.113464-1-sj@kernel.org> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.174.177.243] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To dggpemm500001.china.huawei.com (7.185.36.107) X-CFilter-Loop: Reflected On 2023/3/4 2:37, SeongJae Park wrote: > On Fri, 3 Mar 2023 18:26:33 +0000 SeongJae Park wrote: > >> On Fri, 3 Mar 2023 16:43:43 +0800 Kefeng Wang wrote: >> >>> Omit one line by unified folio_put(), and make code more clear. >>> >>> Signed-off-by: Kefeng Wang >>> --- >>> mm/damon/paddr.c | 9 ++++----- >>> 1 file changed, 4 insertions(+), 5 deletions(-) >>> >>> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c >>> index 2ef9db0189ca..6930ebf3667c 100644 >>> --- a/mm/damon/paddr.c >>> +++ b/mm/damon/paddr.c >>> @@ -266,17 +266,16 @@ static inline unsigned long damon_pa_mark_accessed_or_deactivate( >>> if (!folio) >>> continue; >>> >>> - if (damos_pa_filter_out(s, folio)) { >>> - folio_put(folio); >>> - continue; >>> - } >>> + if (damos_pa_filter_out(s, folio)) >>> + goto put_folio; >>> >>> if (mark_accessed) >>> folio_mark_accessed(folio); >>> else >>> folio_deactivate(folio); >>> - folio_put(folio); >>> applied += folio_nr_pages(folio); >>> +put_folio: >>> + folio_put(folio); >> >> I think this change is ok, but shouldn't the 'folio_put()' have called before > > s/before/after/ > >> 'folio_nr_pages()' anyway? If so, could we make the change as a separate fix >> first, and then make this change, so that it can be easily applied to relevant >> stable kernels? Yes, seem to previous one. >> >> >> Thanks, >> SJ >> >>> } >>> return applied * PAGE_SIZE; >>> } >>> -- >>> 2.35.3 >>> >>>