Linux-api Archive on lore.kernel.org
 help / color / Atom feed
From: Michal Hocko <mhocko@suse.com>
To: Mike Rapoport <rppt@kernel.org>
Cc: David Hildenbrand <david@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Andy Lutomirski <luto@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
	Borislav Petkov <bp@alien8.de>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Christopher Lameter <cl@linux.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Elena Reshetova <elena.reshetova@intel.com>,
	"H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@redhat.com>,
	James Bottomley <jejb@linux.ibm.com>,
	"Kirill A. Shutemov" <kirill@shutemov.name>,
	Matthew Wilcox <willy@infradead.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Mike Rapoport <rppt@linux.ibm.com>,
	Michael Kerrisk <mtk.manpages@gmail.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Rick Edgecombe <rick.p.edgecombe@intel.com>,
	Roman Gushchin <guro@fb.com>, Shakeel Butt <shakeelb@google.com>,
	Shuah Khan <shuah@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Tycho Andersen <tycho@tycho.ws>, Will Deacon <will@kernel.org>,
	linux-api@vger.kernel.org, linux-arch@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
	linux-nvdimm@lists.01.org, linux-riscv@lists.infradead.org,
	x86@kernel.org, Hagen Paul Pfeifer <hagen@jauu.net>,
	Palmer Dabbelt <palmerdabbelt@google.com>
Subject: Re: [PATCH v16 07/11] secretmem: use PMD-size pages to amortize direct map fragmentation
Date: Fri, 29 Jan 2021 09:51:15 +0100
Message-ID: <YBPMg/C5Sb78gFEB@dhcp22.suse.cz> (raw)
In-Reply-To: <20210129072128.GD242749@kernel.org>

On Fri 29-01-21 09:21:28, Mike Rapoport wrote:
> On Thu, Jan 28, 2021 at 02:01:06PM +0100, Michal Hocko wrote:
> > On Thu 28-01-21 11:22:59, Mike Rapoport wrote:
> > 
> > > And hugetlb pools may be also depleted by anybody by calling
> > > mmap(MAP_HUGETLB) and there is no any limiting knob for this, while
> > > secretmem has RLIMIT_MEMLOCK.
> > 
> > Yes it can fail. But it would fail at the mmap time when the reservation
> > fails. Not during the #PF time which can be at any time.
> 
> It may fail at $PF time as well:
> 
> hugetlb_fault()
>         hugeltb_no_page()
>                 ...
>                 alloc_huge_page()
>                         alloc_gigantic_page()
>                                 cma_alloc()
>                                         -ENOMEM; 

I would have to double check. From what I remember cma allocator is an
optimization to increase chances to allocate hugetlb pages when
overcommiting because pages should be normally pre-allocated in the pool
and reserved during mmap time. But even if a hugetlb page is not pre
allocated then this will get propagated as SIGBUS unless that has
changed.
  
> > > That said, simply replacing VM_FAULT_OOM with VM_FAULT_SIGBUS makes
> > > secretmem at least as controllable and robust than hugeltbfs even without
> > > complex reservation at mmap() time.
> > 
> > Still sucks huge!
>  
> Any #PF can get -ENOMEM for whatever reason. Sucks huge indeed.

I certainly can. But it doesn't in practice because most allocations
will simply not fail and rather invoke OOM killer directly. Maybe there
are cases which still might fail (higher order, weaker reclaim
capabilities etc) but that would result in a bug in the end because the
#PF handler would trigger the oom killer.

[...]
> > I would still like to understand whether that data is actually
> > representative. With some underlying reasoning rather than I have run
> > these XYZ benchmarks and numbers do not look terrible.
> 
> I would also very much like to see, for example, reasoning to enabling 1GB
> pages in the direct map beyond "because we can" (commits 00d1c5e05736
> ("x86: add gbpages switches") and ef9257668e31 ("x86: do kernel direct
> mapping at boot using GB pages")).
> 
> The original Kconfig text for CONFIG_DIRECT_GBPAGES said
> 
>           Enable gigabyte pages support (if the CPU supports it). This can
>           improve the kernel's performance a tiny bit by reducing TLB
>           pressure.
> 
> So it is very interesting how tiny that bit was.

Yeah and that sucks! Because it is leaving us with speculations now. I
hope you do not want to repeat the same mistake now and leave somebody
in the future in the same situation.

> > > I like the idea to have a pool as an optimization rather than a hard
> > > requirement but I don't see why would it need a careful access control. As
> > > the direct map fragmentation is not necessarily degrades the performance
> > > (and even sometimes it actually improves it) and even then the degradation
> > > is small, trying a PMD_ORDER allocation for a pool and then falling back to
> > > 4K page may be just fine.
> > 
> > Well, as soon as this is a scarce resource then an access control seems
> > like a first thing to think of. Maybe it is not really necessary but
> > then this should be really justified.
> 
> And what being a scarce resource here?

A fixed size pool shared by all users of this feature.

> If we consider lack of the direct
> map fragmentation as this resource, there enough measures secretmem
> implements to limit user ability to fragment the direct map, as was already
> discussed several times. Global limit, memcg and rlimit provide enough
> access control already.

Try to do a simple excercise. You have X amout of secret memory. How do
you distribute that to all interested users (some of them adversaries)
based on the above. Global limit is a DoS vector potentially, memcg is a
mixed bag of all other memory and it would become really tricky to
enforece proportion of the X while having other memory consumed and
rlimit is per process rather than per user.

Look at how hugetlb had to develop its cgroup controler to distribute
the pool among workloads. Then it has turned out that even reservations
have to be per workload. Quite a convoluted stuff evolved around that
feature because it turned out that the initial assumption that only few
users would be using the pool simply didn't pass the reality check.

As I've mentioned in other response to James. If the direct map
manipulation is not as big of a problem as most of us dogmatically
believed then things become much simpler. There is no need for global
pool and you are back to mlock kinda model.
-- 
Michal Hocko
SUSE Labs

  reply index

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-21 12:27 [PATCH v16 00/11] mm: introduce memfd_secret system call to create "secret" memory areas Mike Rapoport
2021-01-21 12:27 ` [PATCH v16 01/11] mm: add definition of PMD_PAGE_ORDER Mike Rapoport
2021-01-21 12:27 ` [PATCH v16 02/11] mmap: make mlock_future_check() global Mike Rapoport
2021-01-21 12:27 ` [PATCH v16 03/11] riscv/Kconfig: make direct map manipulation options depend on MMU Mike Rapoport
2021-01-21 12:27 ` [PATCH v16 04/11] set_memory: allow set_direct_map_*_noflush() for multiple pages Mike Rapoport
2021-01-21 12:27 ` [PATCH v16 05/11] set_memory: allow querying whether set_direct_map_*() is actually enabled Mike Rapoport
2021-01-21 12:27 ` [PATCH v16 06/11] mm: introduce memfd_secret system call to create "secret" memory areas Mike Rapoport
2021-01-25 17:01   ` Michal Hocko
2021-01-25 21:36     ` Mike Rapoport
2021-01-26  7:16       ` Michal Hocko
2021-01-26  8:33         ` Mike Rapoport
2021-01-26  9:00           ` Michal Hocko
2021-01-26  9:20             ` Mike Rapoport
2021-01-26  9:49               ` Michal Hocko
2021-01-26  9:53                 ` David Hildenbrand
2021-01-26 10:19                   ` Michal Hocko
2021-01-26  9:20             ` Michal Hocko
2021-02-03 12:15   ` Michal Hocko
2021-02-04 11:34     ` Mike Rapoport
2021-01-21 12:27 ` [PATCH v16 07/11] secretmem: use PMD-size pages to amortize direct map fragmentation Mike Rapoport
2021-01-26 11:46   ` Michal Hocko
2021-01-26 11:56     ` David Hildenbrand
2021-01-26 12:08       ` Michal Hocko
2021-01-28  9:22         ` Mike Rapoport
2021-01-28 13:01           ` Michal Hocko
2021-01-28 13:28             ` Christoph Lameter
2021-01-28 13:49               ` Michal Hocko
2021-01-28 15:56                 ` Christoph Lameter
2021-01-28 16:23                   ` Michal Hocko
2021-01-28 15:28             ` James Bottomley
2021-01-29  7:03               ` Mike Rapoport
2021-01-28 21:05             ` James Bottomley
     [not found]               ` <YBPF8ETGBHUzxaZR@dhcp22.suse.cz>
2021-02-01 16:56                 ` James Bottomley
2021-02-02  9:35                   ` Michal Hocko
2021-02-02 12:48                     ` Mike Rapoport
2021-02-02 13:14                       ` David Hildenbrand
2021-02-02 13:32                         ` Michal Hocko
2021-02-02 14:12                           ` David Hildenbrand
2021-02-02 14:22                             ` Michal Hocko
2021-02-02 14:26                               ` David Hildenbrand
2021-02-02 14:32                                 ` Michal Hocko
2021-02-02 14:34                                   ` David Hildenbrand
2021-02-02 18:15                                     ` Mike Rapoport
2021-02-02 18:55                                       ` James Bottomley
2021-02-03 12:09                                         ` Michal Hocko
2021-02-04 11:31                                           ` Mike Rapoport
2021-02-02 13:27                       ` Michal Hocko
2021-02-02 19:10                         ` Mike Rapoport
2021-02-03  9:12                           ` Michal Hocko
2021-02-04  9:58                             ` Mike Rapoport
2021-02-04 13:02                               ` Michal Hocko
2021-01-29  7:21             ` Mike Rapoport
2021-01-29  8:51               ` Michal Hocko [this message]
2021-02-02 14:42                 ` David Hildenbrand
2021-01-21 12:27 ` [PATCH v16 08/11] secretmem: add memcg accounting Mike Rapoport
2021-01-25 16:17   ` Matthew Wilcox
2021-01-25 17:18     ` Shakeel Butt
2021-01-25 21:35       ` Mike Rapoport
2021-01-28 15:07         ` Shakeel Butt
2021-01-25 16:54   ` Michal Hocko
2021-01-25 21:38     ` Mike Rapoport
2021-01-26  7:31       ` Michal Hocko
2021-01-26  8:56         ` Mike Rapoport
2021-01-26  9:15           ` Michal Hocko
2021-01-26 14:48       ` Matthew Wilcox
2021-01-26 15:05         ` Michal Hocko
2021-01-27 18:42           ` Roman Gushchin
2021-01-28  7:58             ` Michal Hocko
2021-01-28 14:05               ` Shakeel Butt
2021-01-28 14:22                 ` Michal Hocko
2021-01-28 14:57                   ` Shakeel Butt
2021-01-21 12:27 ` [PATCH v16 09/11] PM: hibernate: disable when there are active secretmem users Mike Rapoport
2021-01-21 12:27 ` [PATCH v16 10/11] arch, mm: wire up memfd_secret system call where relevant Mike Rapoport
2021-01-25 18:18   ` Catalin Marinas
2021-01-21 12:27 ` [PATCH v16 11/11] secretmem: test: add basic selftest for memfd_secret(2) Mike Rapoport
2021-01-21 22:18 ` [PATCH v16 00/11] mm: introduce memfd_secret system call to create "secret" memory areas Andrew Morton

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=YBPMg/C5Sb78gFEB@dhcp22.suse.cz \
    --to=mhocko@suse.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=bp@alien8.de \
    --cc=catalin.marinas@arm.com \
    --cc=cl@linux.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=david@redhat.com \
    --cc=elena.reshetova@intel.com \
    --cc=guro@fb.com \
    --cc=hagen@jauu.net \
    --cc=hpa@zytor.com \
    --cc=jejb@linux.ibm.com \
    --cc=kirill@shutemov.name \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=luto@kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=mtk.manpages@gmail.com \
    --cc=palmer@dabbelt.com \
    --cc=palmerdabbelt@google.com \
    --cc=paul.walmsley@sifive.com \
    --cc=peterz@infradead.org \
    --cc=rick.p.edgecombe@intel.com \
    --cc=rppt@kernel.org \
    --cc=rppt@linux.ibm.com \
    --cc=shakeelb@google.com \
    --cc=shuah@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tycho@tycho.ws \
    --cc=viro@zeniv.linux.org.uk \
    --cc=will@kernel.org \
    --cc=willy@infradead.org \
    --cc=x86@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

Linux-api Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-api/0 linux-api/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-api linux-api/ https://lore.kernel.org/linux-api \
		linux-api@vger.kernel.org
	public-inbox-index linux-api

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-api


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git