on 8/16/2023 1:46 AM, Chris Li wrote:
> Hi Kemeng,
>
> Since I am discussing the other patch in this series, I might just commend on this one
> as well.
>
> On Wed, Aug 09, 2023 at 06:07:54PM +0800, Kemeng Shi wrote:
>> We get batch from pcp and just pass it to nr_pcp_free immediately. Get
>> batch from pcp inside nr_pcp_free to remove unnecessary parameter batch.
>>
>> Signed-off-by: Kemeng Shi <shikemeng@xxxxxxxxxxxxxxx>
>> ---
>> mm/page_alloc.c | 8 +++-----
>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 1ddcb2707d05..bb1d14e806ad 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -2376,10 +2376,10 @@ static bool free_unref_page_prepare(struct page *page, unsigned long pfn,
>> return true;
>> }
>>
>> -static int nr_pcp_free(struct per_cpu_pages *pcp, int high, int batch,
>> - bool free_high)
>> +static int nr_pcp_free(struct per_cpu_pages *pcp, int high, bool free_high)
>> {
>> int min_nr_free, max_nr_free;
>> + int batch = READ_ONCE(pcp->batch);
>
> Because nr_pcp_free is static and has only one caller. This function gets inlined
> at the caller's side. I verified that on X86_64 compiled code.
>
> So this fix in my opinion is not worthwhile to fix. It will produce the same
> machine code. One minor side effect is that it will hide the commit under it
> in "git blame".
>
Hi Chris, thank for the reply. Except to reduce argument to pass, this patch also
tries make code look little cleaner. I think it's always better to reduce variable
scope and keep relevant code tight. In this case, we know batch is from
per_cpu_pages during reading nr_pcp_free alone rather than search caller to find it
out. And more callers of nr_pcp_free in fulture is free from pass pcp->batch. And so
on. Anyway, this patch definely gains a little without lost in my opinion.:) With it
makes sense to you.
> Chris
>
