Messages in this thread | | | Subject | Re: [PATCH] mm: page_alloc: update the COMPACT[STALL|FAIL] events properly | From | Charan Teja Kalla <> | Date | Tue, 2 Feb 2021 18:19:46 +0530 |
| |
Thanks David for the review!!
On 2/2/2021 2:54 AM, David Rientjes wrote: > On Mon, 1 Feb 2021, Charan Teja Reddy wrote: > >> By defination, COMPACT[STALL|FAIL] events needs to be counted when there > > s/defination/definition/\
Done.
> >> is 'At least in one zone compaction wasn't deferred or skipped from the >> direct compaction'. And when compaction is skipped or deferred, >> COMPACT_SKIPPED will be returned but it will still go and update these >> compaction events which is wrong in the sense that COMPACT[STALL|FAIL] >> is counted without even trying the compaction. >> >> Correct this by skipping the counting of these events when >> COMPACT_SKIPPED is returned for compaction. This indirectly also avoid >> the unnecessary try into the get_page_from_freelist() when compaction is >> not even tried. >> >> Signed-off-by: Charan Teja Reddy <charante@codeaurora.org> >> --- >> mm/page_alloc.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >> index 519a60d..531f244 100644 >> --- a/mm/page_alloc.c >> +++ b/mm/page_alloc.c >> @@ -4152,6 +4152,8 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order, >> memalloc_noreclaim_restore(noreclaim_flag); >> psi_memstall_leave(&pflags); >> >> + if (*compact_result == COMPACT_SKIPPED) >> + return NULL; >> /* >> * At least in one zone compaction wasn't deferred or skipped, so let's >> * count a compaction stall > > This makes sense, I wonder if it would also be useful to check that > page == NULL, either in try_to_compact_pages() or here for > COMPACT_SKIPPED?
In the code, when COMPACT_SKIPPED is being returned, the page will always be NULL. So, I'm not sure how much useful it is for the page == NULL check here. Or I failed to understand your point here?
As, till now, COMPACTFAIL also presents page allocation failures because of the direct compaction is skipped, creating a separate COMPACTSKIPFAIL event which indicates that 'failed to get the free page as direct compaction is skipped' is useful? >
-- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
| |