| Message ID | 20220908035533.2186159-1-liushixin2@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] mm/huge_memory: prevent THP_ZERO_PAGE_ALLOC increased twice | expand |
On Thu, Sep 08, 2022 at 11:55:33AM +0800, Liu Shixin wrote: > If two or more threads call get_huge_zero_page concurrently, > THP_ZERO_PAGE_ALLOC may increased two or more times. But actually, > this should only count as once since the extra zero pages has been > freed. Redefine its meaning to indicate the times a huge zero page > used for thp is successfully allocated. I don't particularly care, but it is not obvoius why the new behaviour is better. All huge zero pages are freed apart from possibly the last one allocated.
On 2022/9/8 20:31, Kirill A. Shutemov wrote: > On Thu, Sep 08, 2022 at 11:55:33AM +0800, Liu Shixin wrote: >> If two or more threads call get_huge_zero_page concurrently, >> THP_ZERO_PAGE_ALLOC may increased two or more times. But actually, >> this should only count as once since the extra zero pages has been >> freed. Redefine its meaning to indicate the times a huge zero page >> used for thp is successfully allocated. > I don't particularly care, but it is not obvoius why the new behaviour is > better. The user who read the value may be more concerned about the huge zero pages that are really allocated using for thp and can indicated the times of calling huge_zero_page_shrinker. I misunderstood when I first saw it. > > All huge zero pages are freed apart from possibly the last one allocated. >
On Thu, Sep 08, 2022 at 09:07:04PM +0800, Liu Shixin wrote: > > > On 2022/9/8 20:31, Kirill A. Shutemov wrote: > > On Thu, Sep 08, 2022 at 11:55:33AM +0800, Liu Shixin wrote: > >> If two or more threads call get_huge_zero_page concurrently, > >> THP_ZERO_PAGE_ALLOC may increased two or more times. But actually, > >> this should only count as once since the extra zero pages has been > >> freed. Redefine its meaning to indicate the times a huge zero page > >> used for thp is successfully allocated. > > I don't particularly care, but it is not obvoius why the new behaviour is > > better. > The user who read the value may be more concerned about the huge zero > pages that are really allocated using for thp and can indicated the > times of calling huge_zero_page_shrinker. > I misunderstood when I first saw it. Please, explain the motivation in the commit message.
On 2022/9/8 21:25, Kirill A. Shutemov wrote: > On Thu, Sep 08, 2022 at 09:07:04PM +0800, Liu Shixin wrote: >> >> On 2022/9/8 20:31, Kirill A. Shutemov wrote: >>> On Thu, Sep 08, 2022 at 11:55:33AM +0800, Liu Shixin wrote: >>>> If two or more threads call get_huge_zero_page concurrently, >>>> THP_ZERO_PAGE_ALLOC may increased two or more times. But actually, >>>> this should only count as once since the extra zero pages has been >>>> freed. Redefine its meaning to indicate the times a huge zero page >>>> used for thp is successfully allocated. >>> I don't particularly care, but it is not obvoius why the new behaviour is >>> better. >> The user who read the value may be more concerned about the huge zero >> pages that are really allocated using for thp and can indicated the >> times of calling huge_zero_page_shrinker. >> I misunderstood when I first saw it. > Please, explain the motivation in the commit message. Thanks, I add the motivation. https://lore.kernel.org/all/20220909021653.3371879-1-liushixin2@huawei.com/
diff --git a/Documentation/admin-guide/mm/transhuge.rst b/Documentation/admin-guide/mm/transhuge.rst index c9c37f16eef8..8e3418ec4503 100644 --- a/Documentation/admin-guide/mm/transhuge.rst +++ b/Documentation/admin-guide/mm/transhuge.rst @@ -366,10 +366,9 @@ thp_split_pmd page table entry. thp_zero_page_alloc - is incremented every time a huge zero page is - successfully allocated. It includes allocations which where - dropped due race with other allocation. Note, it doesn't count - every map of the huge zero page, only its allocation. + is incremented every time a huge zero page used for thp is + successfully allocated. Note, it doesn't count every map of + the huge zero page, only its allocation. thp_zero_page_alloc_failed is incremented if kernel fails to allocate diff --git a/mm/huge_memory.c b/mm/huge_memory.c index 88d98241a635..5c83a424803a 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -163,7 +163,6 @@ static bool get_huge_zero_page(void) count_vm_event(THP_ZERO_PAGE_ALLOC_FAILED); return false; } - count_vm_event(THP_ZERO_PAGE_ALLOC); preempt_disable(); if (cmpxchg(&huge_zero_page, NULL, zero_page)) { preempt_enable(); @@ -175,6 +174,7 @@ static bool get_huge_zero_page(void) /* We take additional reference here. It will be put back by shrinker */ atomic_set(&huge_zero_refcount, 2); preempt_enable(); + count_vm_event(THP_ZERO_PAGE_ALLOC); return true; }
If two or more threads call get_huge_zero_page concurrently, THP_ZERO_PAGE_ALLOC may increased two or more times. But actually, this should only count as once since the extra zero pages has been freed. Redefine its meaning to indicate the times a huge zero page used for thp is successfully allocated. Update Documentation/admin-guide/mm/transhuge.rst together. Signed-off-by: Liu Shixin <liushixin2@huawei.com> --- v1->v2: Update documnet. Documentation/admin-guide/mm/transhuge.rst | 7 +++---- mm/huge_memory.c | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-)