lkml.org 
[lkml]   [2022]   [Aug]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] mm/damon: Validate if the pmd entry is present before accessing
From


On 8/18/2022 10:29 AM, SeongJae Park wrote:
> Hi Baolin,
>
> On Thu, 18 Aug 2022 09:05:58 +0800 Baolin Wang <baolin.wang@linux.alibaba.com> wrote:
>
>>
>>
>> On 8/18/2022 12:09 AM, SeongJae Park wrote:
>>> Hi Baolin,
>>>
>>>
>>> Thank you always for your great patch!
>>>
>>> On Wed, 17 Aug 2022 14:21:12 +0800 Baolin Wang <baolin.wang@linux.alibaba.com> wrote:
>>>
>>>> The pmd_huge() is used to validate if the pmd entry is mapped by a huge
>>>> page, also including the case of non-present (migration or hwpoisoned)
>>>> pmd entry on arm64 or x86 architectures. Thus we should validate if it
>>>> is present before making the pmd entry old or getting young state,
>>>> otherwise we can not get the correct corresponding page.
>>>
>>> Maybe I'm missing something, but... I'm unsure if the page is present or not
>>> really matters from the perspective of access checking. In the case, DAMON
>>> could simply report the page has accessed once for the first check after the
>>> page being non-present if it really accessed before, and then report the page
>>> as not accessed, which is true.
>>
>> Yes, that's the patch's goal to make the accesses correct. However if
>> the PMD entry is not present, we can not get the correct page object by
>> pmd_pfn(*pmd), since the non-present pmd entry will contain swap type
>> and swap offset with below format on ARM64, that means the pfn number is
>> saved in bits 8-57 in a migration or poisoned entry, but pmd_pfn() still
>> treat bits 12-47 as the pfn number on ARM64, which may get an incorrect
>> page struct (also maybe is NULL by pfn_to_online_page()) to make the
>> access statistics incorrect.
>>
>> /*
>> * Encode and decode a swap entry:
>> * bits 0-1: present (must be zero)
>> * bits 2: remember PG_anon_exclusive
>> * bits 3-7: swap type
>> * bits 8-57: swap offset
>> * bit 58: PTE_PROT_NONE (must be zero)
>> */
>>
>>
>> Moreoever I don't think we should still waste time to get the page of
>> the non-present entry, just treat it as not-accessed and skip it, that
>> keeps consistent with non-present pte level entry.
>>
>> Does that make sense for you? Thanks.
>
> Yes, that totally makes sense. Thank you very much for the kind answer. I
> think it would be great if we could put the detailed explanation in the commit
> message. Could you please update the commit message and post v2 of the patch?

Sure, will update the commit message to make it more clear and I think
that can also answer Andrew's concern.

>
> Reviewed-by: SeongJae Park <sj@kernel.org>

Thanks.

\
 
 \ /
  Last update: 2022-08-18 04:41    [W:0.033 / U:0.212 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site