On 2023/3/8 2:00, SeongJae Park wrote:
On Tue, 7 Mar 2023 09:22:33 +0800 Kefeng Wang <wangkefeng.wang@xxxxxxxxxx> wrote:
On 2023/3/7 5:27, SeongJae Park wrote:
Hi Kefeng,
On Mon, 6 Mar 2023 09:56:49 +0800 Kefeng Wang <wangkefeng.wang@xxxxxxxxxx> wrote:
On 2023/3/6 9:10, Kefeng Wang wrote:
On 2023/3/4 2:39, SeongJae Park wrote:
Hi Kefeng,
On Fri, 3 Mar 2023 16:43:42 +0800 Kefeng Wang
<wangkefeng.wang@xxxxxxxxxx> wrote:
Omit three lines by unified folio_put(), and make code more clear.
Signed-off-by: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx>
---
mm/damon/paddr.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index 3fda00a0f786..2ef9db0189ca 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -130,24 +130,21 @@ static bool damon_pa_young(unsigned long paddr,
unsigned long *folio_sz)
accessed = false;
else
accessed = true;
- folio_put(folio);
goto out;
Because you moved 'out' label to not include *folio_sz setting,
folio_sz will
not set in this case. It should be set.
oh, it should be fixed.
}
need_lock = !folio_test_anon(folio) || folio_test_ksm(folio);
- if (need_lock && !folio_trylock(folio)) {
- folio_put(folio);
- return false;
- }
Hi SJ, apart from above issue, it looks that this branch need the
folio_size() setting, right?
folio_sz is effectively used by caller of damon_pa_young() only if this
function returns true, so this branch doesn't need to set folio_sz.
__damon_pa_check_access() store last_addr, last_accessed and
last_folio_sz, even damon_pa_young() return false, the following check
still use last_folio_sz,
ALIGN_DOWN(last_addr, last_folio_sz) == ALIGN_DOWN(r->sampling_addr,
last_folio_sz)
but last_folio_sz is not up to date, so I think it need to update, and
update last_folio_sz is harmless, which could let's unify the return
path, correct me if I am wrong.
Ah, you're right. Thank you for kind explanation. I was out of my mind for
some reason. Maybe we could just do 'goto out' in the branch.
Yes, will update this patchset with this change.
Thanks,
SJ
