All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yajun Deng <yajun.deng@linux.dev>
To: Mike Rapoport <rppt@kernel.org>, David Hildenbrand <david@redhat.com>
Cc: Matthew Wilcox <willy@infradead.org>,
	akpm@linux-foundation.org, mike.kravetz@oracle.com,
	muchun.song@linux.dev, glider@google.com, elver@google.com,
	dvyukov@google.com, osalvador@suse.de, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com
Subject: Re: [PATCH 1/4] mm: pass set_count and set_reserved to __init_single_page
Date: Mon, 25 Sep 2023 11:23:03 +0800	[thread overview]
Message-ID: <798ddb57-ba09-e337-01b3-c80711f1e277@linux.dev> (raw)
In-Reply-To: <20230922080831.GH3303@kernel.org>


On 2023/9/22 16:08, Mike Rapoport wrote:
> On Fri, Sep 22, 2023 at 09:48:59AM +0200, David Hildenbrand wrote:
>> On 22.09.23 09:47, Matthew Wilcox wrote:
>>> On Fri, Sep 22, 2023 at 03:09:20PM +0800, Yajun Deng wrote:
>>>> -		__init_single_page(page, pfn, zone, nid);
>>>> +		__init_single_page(page, pfn, zone, nid, true, false);
>>> So Linus has just had a big rant about not doing bool flags to
>>> functions.  And in particular _multiple_ bool flags to functions.
>>>
>>> ie this should be:
>>>
>>> #define INIT_PAGE_COUNT		(1 << 0)
>>> #define INIT_PAGE_RESERVED	(1 << 1)
>>>
>>> 		__init_single_page(page, pfn, zone, nid, INIT_PAGE_COUNT);
>>>
>>> or something similar.
>>>
>>> I have no judgement on the merits of this patch so far.  Do you have
>>> performance numbers for each of these patches?  Some of them seem quite
>>> unlikely to actually help, at least on a machine which is constrained
>>> by cacheline fetches.
>> The last patch contains
>>
>> before:
>> node 0 deferred pages initialised in 78ms
>>
>> after:
>> node 0 deferred pages initialised in 72ms
>>
>> Not earth-shattering :D Maybe with much bigger machines relevant?
> Patch 3 contains
>
> The following data was tested on an x86 machine with 190GB of RAM.
>
> before:
> free_low_memory_core_early()    342ms
>
> after:
> free_low_memory_core_early()    286ms
>
> Which is more impressive, but still I'm not convinced that it's worth the
> added complexity and potential subtle bugs.
>
I will send v2.  It will be simpler and safer.
>> -- 
>> Cheers,
>>
>> David / dhildenb
>>

  reply	other threads:[~2023-09-25  3:25 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-22  7:09 [PATCH 0/4] mm: Don't set and reset page count in MEMINIT_EARLY Yajun Deng
2023-09-22  7:09 ` [PATCH 1/4] mm: pass set_count and set_reserved to __init_single_page Yajun Deng
2023-09-22  7:47   ` Matthew Wilcox
2023-09-22  7:48     ` David Hildenbrand
2023-09-22  8:08       ` Mike Rapoport
2023-09-25  3:23         ` Yajun Deng [this message]
2023-09-22  7:09 ` [PATCH 2/4] mm: Introduce MEMINIT_LATE context Yajun Deng
2023-09-22  7:09 ` [PATCH 3/4] mm: Set page count and mark page reserved in reserve_bootmem_region Yajun Deng
2023-09-22  7:09 ` [PATCH 4/4] mm: don't set page count in deferred_init_pages Yajun Deng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=798ddb57-ba09-e337-01b3-c80711f1e277@linux.dev \
    --to=yajun.deng@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=glider@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mike.kravetz@oracle.com \
    --cc=muchun.song@linux.dev \
    --cc=osalvador@suse.de \
    --cc=rppt@kernel.org \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.