linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: David Hildenbrand <david@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Alexander Potapenko <glider@google.com>,
	Kees Cook <keescook@chromium.org>,
	Michal Hocko <mhocko@kernel.org>,
	Mateusz Nosek <mateusznosek0@gmail.com>
Subject: Re: [PATCH 3/3] mm, page_alloc: reduce static keys in prep_new_page()
Date: Tue, 27 Oct 2020 12:05:21 +0100	[thread overview]
Message-ID: <1fc7ec3a-367c-eb9f-1cb4-b9e015fea87c@suse.cz> (raw)
In-Reply-To: <93ab79df-cf8c-294b-3ed1-8a563e4a452b@redhat.com>

On 10/27/20 10:10 AM, David Hildenbrand wrote:
> On 26.10.20 18:33, Vlastimil Babka wrote:
>> prep_new_page() will always zero a new page (regardless of __GFP_ZERO) when
>> init_on_alloc is enabled, but will also always skip zeroing if the page was
>> already zeroed on free by init_on_free or page poisoning.
>> 
>> The latter check implemented by free_pages_prezeroed() can involve two
>> different static keys. As prep_new_page() is really a hot path, let's introduce
>> a single static key free_pages_not_prezeroed for this purpose and initialize it
>> in init_mem_debugging().
> 
> Is this actually observable in practice? This smells like
> micro-optimization to me.
> 
> Also, I thought the whole reason for static keys is to have basically no
> overhead at runtime, so I wonder if replacing two static key checks by a
> single one actually makes *some* difference.

You're right, the difference seems to be just a single NOP. The static key 
infrastructure seems to be working really well.
(At least the asm inspection made me realize that kernel_poison_pages() is 
called unconditionally and the static key is checked inside, not inline so I'll 
be amending patch 2...)

Initially I thought I would be reducing 3 keys to 1 in this patch, but I got the 
code wrong. So unless others think it's a readability improvements, we can drop 
this patch.

Or we can also reconsider this whole optimization. If the point is to be 
paranoid and enable both init_on_free and init_on_alloc, should we trust that 
nobody wrote something after the clearing on free via use-after-free? :) Kees/Alex?




  reply	other threads:[~2020-10-27 11:05 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-26 17:33 [PATCH 0/3] optimize handling of memory debugging parameters Vlastimil Babka
2020-10-26 17:33 ` [PATCH 1/3] mm, page_alloc: do not rely on the order of page_poison and init_on_alloc/free parameters Vlastimil Babka
2020-10-27  9:03   ` David Hildenbrand
2020-10-27  9:58     ` Vlastimil Babka
2020-10-27  9:58       ` David Hildenbrand
2020-10-28  8:31   ` Mike Rapoport
2020-10-26 17:33 ` [PATCH 2/3] mm, page_poison: use static key more efficiently Vlastimil Babka
2020-10-27  9:07   ` David Hildenbrand
2020-10-30 16:27   ` Luis Chamberlain
2020-10-30 22:56     ` Vlastimil Babka
2020-11-11 13:29       ` Luis Chamberlain
2020-10-26 17:33 ` [PATCH 3/3] mm, page_alloc: reduce static keys in prep_new_page() Vlastimil Babka
2020-10-27  9:10   ` David Hildenbrand
2020-10-27 11:05     ` Vlastimil Babka [this message]
2020-10-27 13:32       ` Vlastimil Babka
2020-10-27 17:41         ` Vlastimil Babka
2020-10-28  8:38           ` David Hildenbrand
2020-10-29 17:37         ` Alexander Potapenko

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=1fc7ec3a-367c-eb9f-1cb4-b9e015fea87c@suse.cz \
    --to=vbabka@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=glider@google.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mateusznosek0@gmail.com \
    --cc=mhocko@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox