Received-SPF: pass (google.com: domain of linux-kernel+bounces-131404-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <30bef3d6-3381-491e-a00a-bd1fb7e8d8f8@linux.alibaba.com>
Date: Thu, 4 Apr 2024 20:19:29 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] mm: page_alloc: use the correct THP order for THP PCP
To: Vlastimil Babka <vbabka@suse.cz>, akpm@linux-foundation.org
Cc: mgorman@techsingularity.net, linux-mm@kvack.org,
 linux-kernel@vger.kernel.org
References: <a25c9e14cd03907d5978b60546a69e6aa3fc2a7d.1712151833.git.baolin.wang@linux.alibaba.com>
 <76457ec5-d789-449b-b8ca-dcb6ceb12445@suse.cz>
From: Baolin Wang <baolin.wang@linux.alibaba.com>
In-Reply-To: <76457ec5-d789-449b-b8ca-dcb6ceb12445@suse.cz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


On 2024/4/4 18:03, Vlastimil Babka wrote:
> On 4/3/24 3:47 PM, Baolin Wang wrote:
>> Commit 44042b449872 ("mm/page_alloc: allow high-order pages to be stored
>> on the per-cpu lists") extends the PCP allocator to store THP pages, and
>> it determines whether to cache THP pags in PCP by comparing with pageblock_order.
>> But the pageblock_order is not always equal to THP order, it might also
>> be MAX_PAGE_ORDER, which could prevent PCP from caching THP pages.
>>
>> Therefore, using HPAGE_PMD_ORDER instead to determine the need for caching
>> THP for PCP can fix this issue
>>
>> Fixes: 44042b449872 ("mm/page_alloc: allow high-order pages to be stored on the per-cpu lists")
>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> 
> IIUC this happens with CONFIG_HUGETLB_PAGE disabled because HUGETLBFS is
> disabled? But THPs are still enabled? I think there might be more of THP

Right, and seems the Powerpc arch will set pageblock_order via 
set_pageblock_order() when the huge page sizes are variable (not sure if 
this is always equal to THP order).

Moreover, it still does not make sense to use pageblock_order to 
indicate a THP page, especially when we already have HPAGE_PMD_ORDER to 
represent THP.

> working suboptimally in that case with pageblock_order being larger
> (MAX_PAGE_ORDER).
> 
> In other words, should be rather make pageblock_order itself defined as
> 
>   min_t(unsigned int, HPAGE_PMD_ORDER, MAX_PAGE_ORDER)
> 
> in case with !CONFIG_HUGETLB_PAGE but THP enabled.

Yes, this makes sense to me (I wonder why this wasn't done before?). I 
can create a seperate patch to do this, what do you think? Thanks.