* Re: [RFC PATCH v2 11/17] cgroup: Implement new thread mode semantics
From: Waiman Long @ 2017-05-18 17:21 UTC (permalink / raw)
To: Tejun Heo
Cc: Li Zefan, Johannes Weiner, Peter Zijlstra, Ingo Molnar, cgroups,
linux-kernel, linux-doc, linux-mm, kernel-team, pjt, luto, efault
In-Reply-To: <20170517214718.GH942@htj.duckdns.org>
On 05/17/2017 05:47 PM, Tejun Heo wrote:
> Hello, Waiman.
>
> On Mon, May 15, 2017 at 09:34:10AM -0400, Waiman Long wrote:
>> The current thread mode semantics aren't sufficient to fully support
>> threaded controllers like cpu. The main problem is that when thread
>> mode is enabled at root (mainly for performance reason), all the
>> non-threaded controllers cannot be supported at all.
>>
>> To alleviate this problem, the roles of thread root and threaded
>> cgroups are now further separated. Now thread mode can only be enabled
>> on a non-root leaf cgroup whose parent will then become the thread
>> root. All the descendants of a threaded cgroup will still need to be
>> threaded. All the non-threaded resource will be accounted for in the
>> thread root. Unlike the previous thread mode, however, a thread root
>> can have non-threaded children where system resources like memory
>> can be further split down the hierarchy.
>>
>> Now we could have something like
>>
>> R -- A -- B
>> \
>> T1 -- T2
>>
>> where R is the thread root, A and B are non-threaded cgroups, T1 and
>> T2 are threaded cgroups. The cgroups R, T1, T2 form a threaded subtree
>> where all the non-threaded resources are accounted for in R. The no
>> internal process constraint does not apply in the threaded subtree.
>> Non-threaded controllers need to properly handle the competition
>> between internal processes and child cgroups at the thread root.
>>
>> This model will be flexible enough to support the need of the threaded
>> controllers.
> I do like the approach and it does address the issue with requiring at
> least one level of nesting for the thread mode to be used with other
> controllers. I need to think a bit more about it and mull over what
> Peterz was suggesting in the old thread. I'll get back to you soon
> but I'd really prefer this and the earlier related patches to be in
> its own patchset so that we aren't dealing with different things at
> the same time.
>
> Thanks.
>
I have studied the email exchanges with your original thread mode
patchset. This patchset is aimed to hopefully address all the concerns
that Peterz has. This enhanced thread mode should address a big part of
the concern. However, I am not sure if this patch, by itself, is enough
to address all his concerns. That is why I also include 2 other major
changes in the next 2 patches. My goal is to move forward to allow all
controllers to be enabled for v2 eventually. We are not there yet, but I
hope this patchset can move thing forward meaningfully.
Regards,
Longman
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCHv5, REBASED 9/9] x86/mm: Allow to have userspace mappings above 47-bits
From: Michal Hocko @ 2017-05-18 17:13 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Kirill A. Shutemov, x86, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Andi Kleen, Dave Hansen, Andy Lutomirski,
Dan Williams, linux-mm, linux-kernel, linux-api
In-Reply-To: <20170518162255.l55tm5qbmnvvsgba@node.shutemov.name>
On Thu 18-05-17 19:22:55, Kirill A. Shutemov wrote:
> On Thu, May 18, 2017 at 05:59:14PM +0200, Michal Hocko wrote:
[...]
> > I basically mean something like the following
> > ---
> > diff --git a/arch/x86/kernel/sys_x86_64.c b/arch/x86/kernel/sys_x86_64.c
> > index 74d1587b181d..d6f66ff02d0a 100644
> > --- a/arch/x86/kernel/sys_x86_64.c
> > +++ b/arch/x86/kernel/sys_x86_64.c
> > @@ -195,7 +195,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
> > goto bottomup;
> >
> > /* requesting a specific address */
> > - if (addr) {
> > + if (addr && addr <= DEFAULT_MAP_WINDOW) {
> > addr = PAGE_ALIGN(addr);
> > vma = find_vma(mm, addr);
> > if (TASK_SIZE - len >= addr &&
> > @@ -215,7 +215,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
> > * !in_compat_syscall() check to avoid high addresses for x32.
> > */
> > if (addr > DEFAULT_MAP_WINDOW && !in_compat_syscall())
> > - info.high_limit += TASK_SIZE_MAX - DEFAULT_MAP_WINDOW;
> > + info.high_limit += min(TASK_SIZE_MAX, address) - DEFAULT_MAP_WINDOW;
> >
> > info.align_mask = 0;
> > info.align_offset = pgoff << PAGE_SHIFT;
>
> You try to stretch the interface too far. With the patch you propose we
> have totally different behaviour wrt hint address if it below and above
> 47-bits:
>
> * <= 47-bits: allocate VM [addr; addr + len - 1], if free;
unless I am missing something fundamental here this is not how it works.
We just map a different range if the requested one is not free (in
absence of MAP_FIXED). And we do that in top->down direction so this is
already how it works. And you _do_ rely on the same thing when allowing
larger than 47b except you start from the top of the supported address
space. So how exactly is your new behavior any different and more clear?
Say you would do
mmap(1<<48, ...) # you will get 1<<48
mmap(1<<48, ...) # you will get something below TASK_SIZE_MAX
> * > 47-bits: allocate VM anywhere under addr;
>
> Sorry, no. That's ugly.
>
> If you feel that we need to guarantee that bits above certain limit are
> unused, introduce new interface. We have enough logic encoded in hint
> address already.
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC 1/6] mm, page_alloc: fix more premature OOM due to race with cpuset update
From: Christoph Lameter @ 2017-05-18 17:07 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Michal Hocko, linux-mm, linux-kernel, cgroups, Li Zefan,
Mel Gorman, David Rientjes, Hugh Dickins, Andrea Arcangeli,
Anshuman Khandual, Kirill A. Shutemov, linux-api
In-Reply-To: <8889d67a-adab-91e1-c320-d8bd88d7e1e0@suse.cz>
On Thu, 18 May 2017, Vlastimil Babka wrote:
> > The race is where? If you expand the node set during the move of the
> > application then you are safe in terms of the legacy apps that did not
> > include static bindings.
>
> No, that expand/shrink by itself doesn't work against parallel
Parallel? I think we are clear that ithis is inherently racy against the
app changing policies etc etc? There is a huge issue there already. The
app needs to be well behaved in some heretofore undefined way in order to
make moves clean.
> get_page_from_freelist going through a zonelist. Moving from node 0 to
> 1, with zonelist containing nodes 1 and 0 in that order:
>
> - mempolicy mask is 0
> - zonelist iteration checks node 1, it's not allowed, skip
There is an allocation from node 1? This is not allowed before the move.
So it should fail. Not skipping to another node.
> - mempolicy mask is 0,1 (expand)
> - mempolicy mask is 1 (shrink)
> - zonelist iteration checks node 0, it's not allowed, skip
> - OOM
Are you talking about a race here between zonelist scanning and the
moving? That has been there forever.
And frankly there are gazillions of these races. The best thing to do is
to get the cpuset moving logic out of the kernel and into user space.
Understand that this is a heuristic and maybe come up with a list of
restrictions that make an app safe. An safe app that can be moved must f.e
1. Not allocate new memory while its being moved
2. Not change memory policies after its initialization and while its being
moved.
3. Not save memory policy state in some variable (because the logic to
translate the memory policies for the new context cannot find it).
...
Again cpuset process migration is a huge mess that you do not want to
have in the kernel and AFAICT this is a corner case with difficult
semantics. Better have that in user space...
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v5 31/32] x86: Add sysfs support for Secure Memory Encryption
From: Borislav Petkov @ 2017-05-18 17:01 UTC (permalink / raw)
To: Tom Lendacky
Cc: linux-arch, linux-efi, kvm, linux-doc, x86, kexec, linux-kernel,
kasan-dev, linux-mm, iommu, Rik van Riel,
Radim Krčmář, Toshimitsu Kani, Arnd Bergmann,
Jonathan Corbet, Matt Fleming, Michael S. Tsirkin, Joerg Roedel,
Konrad Rzeszutek Wilk, Paolo Bonzini, Larry Woodman,
Brijesh Singh, Ingo Molnar, Andy Lutomirski, H. Peter Anvin,
Andrey Ryabinin, Alexander Potapenko, Dave Young, Thomas Gleixner,
Dmitry Vyukov
In-Reply-To: <20170418212212.10190.73484.stgit@tlendack-t1.amdoffice.net>
On Tue, Apr 18, 2017 at 04:22:12PM -0500, Tom Lendacky wrote:
> Add sysfs support for SME so that user-space utilities (kdump, etc.) can
> determine if SME is active.
But why do user-space tools need to know that?
I mean, when we load the kdump kernel, we do it with the first kernel,
with the kexec_load() syscall, AFAICT. And that code does a lot of
things during that init, like machine_kexec_prepare()->init_pgtable() to
prepare the ident mapping of the second kernel, for example.
What I'm aiming at is that the first kernel knows *exactly* whether SME
is enabled or not and doesn't need to tell the second one through some
sysfs entries - it can do that during loading.
So I don't think we need any userspace things at all...
Or?
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC 1/6] mm, page_alloc: fix more premature OOM due to race with cpuset update
From: Christoph Lameter @ 2017-05-18 16:57 UTC (permalink / raw)
To: Michal Hocko
Cc: Vlastimil Babka, linux-mm, linux-kernel, cgroups, Li Zefan,
Mel Gorman, David Rientjes, Hugh Dickins, Andrea Arcangeli,
Anshuman Khandual, Kirill A. Shutemov, linux-api
In-Reply-To: <20170518090846.GD25462@dhcp22.suse.cz>
On Thu, 18 May 2017, Michal Hocko wrote:
> > Nope. The OOM in a cpuset gets the process doing the alloc killed. Or what
> > that changed?
!!!!!
> >
> > At this point you have messed up royally and nothing is going to rescue
> > you anyways. OOM or not does not matter anymore. The app will fail.
>
> Not really. If you can trick the system to _think_ that the intersection
> between mempolicy and the cpuset is empty then the OOM killer might
> trigger an innocent task rather than the one which tricked it into that
> situation.
See above. OOM Kill in a cpuset does not kill an innocent task but a task
that does an allocation in that specific context meaning a task in that
cpuset that also has a memory policty.
Regardless of that the point earlier was that the moving logic can avoid
creating temporary situations of empty sets of nodes by analysing the
memory policies etc and only performing moves when doing so is safe.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 07/14] mm: consider zone which is not fully populated to have holes
From: Michal Hocko @ 2017-05-18 16:42 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Andrew Morton, linux-mm, Mel Gorman, Andrea Arcangeli,
Jerome Glisse, Reza Arbab, Yasuaki Ishimatsu, qiuxishi,
Kani Toshimitsu, slaoub, Joonsoo Kim, Andi Kleen, David Rientjes,
Daniel Kiper, Igor Mammedov, Vitaly Kuznetsov, LKML
In-Reply-To: <ae859e14-bf82-ae37-9c85-d4b31ce89b0a@suse.cz>
On Thu 18-05-17 18:14:39, Vlastimil Babka wrote:
> On 05/15/2017 10:58 AM, Michal Hocko wrote:
[...]
> > #ifdef CONFIG_MEMORY_HOTPLUG
> > +/*
> > + * Return page for the valid pfn only if the page is online. All pfn
> > + * walkers which rely on the fully initialized page->flags and others
> > + * should use this rather than pfn_valid && pfn_to_page
> > + */
> > +#define pfn_to_online_page(pfn) \
> > +({ \
> > + struct page *___page = NULL; \
> > + \
> > + if (online_section_nr(pfn_to_section_nr(pfn))) \
> > + ___page = pfn_to_page(pfn); \
> > + ___page; \
> > +})
>
> This seems to be already assuming pfn_valid() to be true. There's no
> "pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS" check and the comment
> suggests as such, but...
Yes, we should check the validity of the section number. We do not have
to check whether the section is valid because online sections are a
subset of those that are valid.
> > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> > index 05796ee974f7..c3a146028ba6 100644
> > --- a/mm/memory_hotplug.c
> > +++ b/mm/memory_hotplug.c
> > @@ -929,6 +929,9 @@ static int online_pages_range(unsigned long start_pfn, unsigned long nr_pages,
> > unsigned long i;
> > unsigned long onlined_pages = *(unsigned long *)arg;
> > struct page *page;
> > +
> > + online_mem_sections(start_pfn, start_pfn + nr_pages);
>
> Shouldn't this be moved *below* the loop that initializes struct pages?
> In the offline case you do mark sections offline before "tearing" struct
> pages, so that should be symmetric.
You are right! Andrew, could you fold the following intot the patch?
---
^ permalink raw reply
* Re: [PATCHv5, REBASED 9/9] x86/mm: Allow to have userspace mappings above 47-bits
From: Kirill A. Shutemov @ 2017-05-18 16:22 UTC (permalink / raw)
To: Michal Hocko
Cc: Kirill A. Shutemov, x86, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Andi Kleen, Dave Hansen, Andy Lutomirski,
Dan Williams, linux-mm, linux-kernel, linux-api
In-Reply-To: <20170518155914.GC18333@dhcp22.suse.cz>
On Thu, May 18, 2017 at 05:59:14PM +0200, Michal Hocko wrote:
> On Thu 18-05-17 17:50:03, Michal Hocko wrote:
> > On Thu 18-05-17 18:41:35, Kirill A. Shutemov wrote:
> > > On Thu, May 18, 2017 at 05:27:36PM +0200, Michal Hocko wrote:
> > > > On Thu 18-05-17 18:19:52, Kirill A. Shutemov wrote:
> > > > > On Thu, May 18, 2017 at 01:43:59PM +0200, Michal Hocko wrote:
> > > > > > On Mon 15-05-17 15:12:18, Kirill A. Shutemov wrote:
> > > > > > [...]
> > > > > > > @@ -195,6 +207,16 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
> > > > > > > info.length = len;
> > > > > > > info.low_limit = PAGE_SIZE;
> > > > > > > info.high_limit = get_mmap_base(0);
> > > > > > > +
> > > > > > > + /*
> > > > > > > + * If hint address is above DEFAULT_MAP_WINDOW, look for unmapped area
> > > > > > > + * in the full address space.
> > > > > > > + *
> > > > > > > + * !in_compat_syscall() check to avoid high addresses for x32.
> > > > > > > + */
> > > > > > > + if (addr > DEFAULT_MAP_WINDOW && !in_compat_syscall())
> > > > > > > + info.high_limit += TASK_SIZE_MAX - DEFAULT_MAP_WINDOW;
> > > > > > > +
> > > > > > > info.align_mask = 0;
> > > > > > > info.align_offset = pgoff << PAGE_SHIFT;
> > > > > > > if (filp) {
> > > > > >
> > > > > > I have two questions/concerns here. The above assumes that any address above
> > > > > > 1<<47 will use the _whole_ address space. Is this what we want?
> > > > >
> > > > > Yes, I believe so.
> > > > >
> > > > > > What if somebody does mmap(1<<52, ...) because he wants to (ab)use 53+
> > > > > > bits for some other purpose? Shouldn't we cap the high_limit by the
> > > > > > given address?
> > > > >
> > > > > This would screw existing semantics of hint address -- "map here if
> > > > > free, please".
> > > >
> > > > Well, the given address is just _hint_. We are still allowed to map to a
> > > > different place. And it is not specified whether the resulting mapping
> > > > is above or below that address. So I do not think it would screw the
> > > > existing semantic. Or do I miss something?
> > >
> > > You are right, that this behaviour is not fixed by any standard or written
> > > down in documentation, but it's de-facto policy of Linux mmap(2) the
> > > beginning.
> > >
> > > And we need to be very careful when messing with this.
> >
> > I am sorry but I still do not understand. You already touch this
> > semantic. mmap(-1UL,...) will already returns basically arbitrary
> > address. All I am asking for is that mmap doesn't return higher address
> > than the given one whent address > 1<<47. We do not have any such users
> > currently so it won't be a change in behavior while it would allow
> > different sized address spaces naturally.
>
> I basically mean something like the following
> ---
> diff --git a/arch/x86/kernel/sys_x86_64.c b/arch/x86/kernel/sys_x86_64.c
> index 74d1587b181d..d6f66ff02d0a 100644
> --- a/arch/x86/kernel/sys_x86_64.c
> +++ b/arch/x86/kernel/sys_x86_64.c
> @@ -195,7 +195,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
> goto bottomup;
>
> /* requesting a specific address */
> - if (addr) {
> + if (addr && addr <= DEFAULT_MAP_WINDOW) {
> addr = PAGE_ALIGN(addr);
> vma = find_vma(mm, addr);
> if (TASK_SIZE - len >= addr &&
> @@ -215,7 +215,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
> * !in_compat_syscall() check to avoid high addresses for x32.
> */
> if (addr > DEFAULT_MAP_WINDOW && !in_compat_syscall())
> - info.high_limit += TASK_SIZE_MAX - DEFAULT_MAP_WINDOW;
> + info.high_limit += min(TASK_SIZE_MAX, address) - DEFAULT_MAP_WINDOW;
>
> info.align_mask = 0;
> info.align_offset = pgoff << PAGE_SHIFT;
You try to stretch the interface too far. With the patch you propose we
have totally different behaviour wrt hint address if it below and above
47-bits:
* <= 47-bits: allocate VM [addr; addr + len - 1], if free;
* > 47-bits: allocate VM anywhere under addr;
Sorry, no. That's ugly.
If you feel that we need to guarantee that bits above certain limit are
unused, introduce new interface. We have enough logic encoded in hint
address already.
--
Kirill A. Shutemov
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [RFC PATCH] mm, oom: cgroup-aware OOM-killer
From: Roman Gushchin @ 2017-05-18 16:28 UTC (permalink / raw)
To: Johannes Weiner
Cc: Roman Gushchin, Tejun Heo, Li Zefan, Michal Hocko,
Vladimir Davydov, Tetsuo Handa, kernel-team, cgroups, linux-doc,
linux-kernel, linux-mm
Traditionally, the OOM killer is operating on a process level.
Under oom conditions, it finds a process with the highest oom score
and kills it.
This behavior doesn't suit well the system with many running
containers. There are two main issues:
1) There is no fairness between containers. A small container with
a few large processes will be chosen over a large one with huge
number of small processes.
2) Containers often do not expect that some random process inside
will be killed. So, in general, a much safer behavior is
to kill the whole cgroup. Traditionally, this was implemented
in userspace, but doing it in the kernel has some advantages,
especially in a case of a system-wide OOM.
To address these issues, cgroup-aware OOM killer is introduced.
Under OOM conditions, it looks for a memcg with highest oom score,
and kills all processes inside.
Memcg oom score is calculated as a size of active and inactive
anon LRU lists, unevictable LRU list and swap size.
For a cgroup-wide OOM, only cgroups belonging to the subtree of
the OOMing cgroup are considered.
If there is no elegible memcg found, OOM killer falls back to
a traditional per-process behavior.
This change affects only cgroup v2.
Signed-off-by: Roman Gushchin <guro@fb.com>
Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: kernel-team@fb.com
Cc: cgroups@vger.kernel.org
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-mm@kvack.org
---
Documentation/cgroup-v2.txt | 24 ++++++++++++++--
include/linux/memcontrol.h | 3 ++
include/linux/oom.h | 1 +
mm/memcontrol.c | 69 +++++++++++++++++++++++++++++++++++++++++++++
mm/oom_kill.c | 49 ++++++++++++++++++++++++++++----
5 files changed, 139 insertions(+), 7 deletions(-)
diff --git a/Documentation/cgroup-v2.txt b/Documentation/cgroup-v2.txt
index dc5e2dc..6583041 100644
--- a/Documentation/cgroup-v2.txt
+++ b/Documentation/cgroup-v2.txt
@@ -44,6 +44,7 @@ CONTENTS
5-2-1. Memory Interface Files
5-2-2. Usage Guidelines
5-2-3. Memory Ownership
+ 5-2-4. Cgroup-aware OOM Killer
5-3. IO
5-3-1. IO Interface Files
5-3-2. Writeback
@@ -831,8 +832,7 @@ PAGE_SIZE multiple when read back.
oom
The number of times the OOM killer has been invoked in
- the cgroup. This may not exactly match the number of
- processes killed but should generally be close.
+ the cgroup.
memory.stat
@@ -988,6 +988,26 @@ POSIX_FADV_DONTNEED to relinquish the ownership of memory areas
belonging to the affected files to ensure correct memory ownership.
+5-2-4. Cgroup-aware OOM Killer
+
+Cgroup v2 memory controller implements a cgroup-aware OOM killer.
+It means that it treats memory cgroups as memory consumers
+rather then individual processes. Under the OOM conditions it tries
+to find an elegible leaf memory cgroup, and kill all processes
+in this cgroup. If it's not possible (e.g. all processes belong
+to the root cgroup), it falls back to the traditional per-process
+behaviour.
+
+The memory controller tries to make the best choise of a victim cgroup.
+In general, it tries to select the largest cgroup, matching given
+node/zone requirements, but the concrete algorithm is not defined,
+and may be changed later.
+
+This affects both system- and cgroup-wide OOMs. For a cgroup-wide OOM
+the memory controller considers only cgroups belonging to a sub-tree
+of the OOM-ing cgroup, including itself.
+
+
5-3. IO
The "io" controller regulates the distribution of IO resources. This
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 899949b..fb0ff64 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -34,6 +34,7 @@ struct mem_cgroup;
struct page;
struct mm_struct;
struct kmem_cache;
+struct oom_control;
/* Cgroup-specific page state, on top of universal node page state */
enum memcg_stat_item {
@@ -465,6 +466,8 @@ static inline bool task_in_memcg_oom(struct task_struct *p)
bool mem_cgroup_oom_synchronize(bool wait);
+bool mem_cgroup_select_oom_victim(struct oom_control *oc);
+
#ifdef CONFIG_MEMCG_SWAP
extern int do_swap_account;
#endif
diff --git a/include/linux/oom.h b/include/linux/oom.h
index 8a266e2..51e71f2 100644
--- a/include/linux/oom.h
+++ b/include/linux/oom.h
@@ -39,6 +39,7 @@ struct oom_control {
unsigned long totalpages;
struct task_struct *chosen;
unsigned long chosen_points;
+ struct mem_cgroup *chosen_memcg;
};
extern struct mutex oom_lock;
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index c131f7e..8d07481 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2625,6 +2625,75 @@ static inline bool memcg_has_children(struct mem_cgroup *memcg)
return ret;
}
+bool mem_cgroup_select_oom_victim(struct oom_control *oc)
+{
+ struct mem_cgroup *iter;
+ unsigned long chosen_memcg_points;
+
+ oc->chosen_memcg = NULL;
+
+ if (mem_cgroup_disabled())
+ return false;
+
+ if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
+ return false;
+
+ pr_info("Choosing a victim memcg because of %s",
+ oc->memcg ?
+ "memory limit reached of cgroup " :
+ "out of memory\n");
+ if (oc->memcg) {
+ pr_cont_cgroup_path(oc->memcg->css.cgroup);
+ pr_cont("\n");
+ }
+
+ chosen_memcg_points = 0;
+
+ for_each_mem_cgroup_tree(iter, oc->memcg) {
+ unsigned long points;
+ int nid;
+
+ if (mem_cgroup_is_root(iter))
+ continue;
+
+ if (memcg_has_children(iter))
+ continue;
+
+ points = 0;
+ for_each_node_state(nid, N_MEMORY) {
+ if (oc->nodemask && !node_isset(nid, *oc->nodemask))
+ continue;
+ points += mem_cgroup_node_nr_lru_pages(iter, nid,
+ LRU_ALL_ANON | BIT(LRU_UNEVICTABLE));
+ }
+ points += mem_cgroup_get_nr_swap_pages(iter);
+
+ pr_info("Memcg ");
+ pr_cont_cgroup_path(iter->css.cgroup);
+ pr_cont(": %lu\n", points);
+
+ if (points > chosen_memcg_points) {
+ if (oc->chosen_memcg)
+ css_put(&oc->chosen_memcg->css);
+
+ oc->chosen_memcg = iter;
+ css_get(&iter->css);
+
+ chosen_memcg_points = points;
+ }
+ }
+
+ if (oc->chosen_memcg) {
+ pr_info("Kill memcg ");
+ pr_cont_cgroup_path(oc->chosen_memcg->css.cgroup);
+ pr_cont(" (%lu)\n", chosen_memcg_points);
+ } else {
+ pr_info("No elegible memory cgroup found\n");
+ }
+
+ return !!oc->chosen_memcg;
+}
+
/*
* Reclaims as many pages from the given memcg as possible.
*
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 04c9143..c000495 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -802,6 +802,8 @@ static bool task_will_free_mem(struct task_struct *task)
return ret;
}
+static void __oom_kill_process(struct task_struct *victim);
+
static void oom_kill_process(struct oom_control *oc, const char *message)
{
struct task_struct *p = oc->chosen;
@@ -809,11 +811,9 @@ static void oom_kill_process(struct oom_control *oc, const char *message)
struct task_struct *victim = p;
struct task_struct *child;
struct task_struct *t;
- struct mm_struct *mm;
unsigned int victim_points = 0;
static DEFINE_RATELIMIT_STATE(oom_rs, DEFAULT_RATELIMIT_INTERVAL,
DEFAULT_RATELIMIT_BURST);
- bool can_oom_reap = true;
/*
* If the task is already exiting, don't alarm the sysadmin or kill
@@ -863,6 +863,15 @@ static void oom_kill_process(struct oom_control *oc, const char *message)
}
read_unlock(&tasklist_lock);
+ __oom_kill_process(victim);
+}
+
+static void __oom_kill_process(struct task_struct *victim)
+{
+ struct task_struct *p;
+ struct mm_struct *mm;
+ bool can_oom_reap = true;
+
p = find_lock_task_mm(victim);
if (!p) {
put_task_struct(victim);
@@ -970,6 +979,20 @@ int unregister_oom_notifier(struct notifier_block *nb)
}
EXPORT_SYMBOL_GPL(unregister_oom_notifier);
+static int oom_kill_task_fn(struct task_struct *p, void *arg)
+{
+ if (is_global_init(p))
+ return 0;
+
+ if (p->flags & PF_KTHREAD)
+ return 0;
+
+ get_task_struct(p);
+ __oom_kill_process(p);
+
+ return 0;
+}
+
/**
* out_of_memory - kill the "best" process when we run out of memory
* @oc: pointer to struct oom_control
@@ -1032,13 +1055,29 @@ bool out_of_memory(struct oom_control *oc)
return true;
}
- select_bad_process(oc);
+ /*
+ * Try to find an elegible memory cgroup. If nothing found,
+ * fallback to a per-process OOM.
+ */
+ if (!mem_cgroup_select_oom_victim(oc))
+ select_bad_process(oc);
+
/* Found nothing?!?! Either we hang forever, or we panic. */
- if (!oc->chosen && !is_sysrq_oom(oc) && !is_memcg_oom(oc)) {
+ if (!oc->chosen_memcg && !oc->chosen && !is_sysrq_oom(oc) &&
+ !is_memcg_oom(oc)) {
dump_header(oc, NULL);
panic("Out of memory and no killable processes...\n");
}
- if (oc->chosen && oc->chosen != (void *)-1UL) {
+
+ if (oc->chosen_memcg) {
+ /* Try to kill the whole memory cgroup. */
+ if (!is_memcg_oom(oc))
+ mem_cgroup_event(oc->chosen_memcg, MEMCG_OOM);
+ mem_cgroup_scan_tasks(oc->chosen_memcg, oom_kill_task_fn, NULL);
+
+ css_put(&oc->chosen_memcg->css);
+ schedule_timeout_killable(1);
+ } else if (oc->chosen && oc->chosen != (void *)-1UL) {
oom_kill_process(oc, !is_memcg_oom(oc) ? "Out of memory" :
"Memory cgroup out of memory");
/*
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 07/14] mm: consider zone which is not fully populated to have holes
From: Vlastimil Babka @ 2017-05-18 16:14 UTC (permalink / raw)
To: Michal Hocko, Andrew Morton
Cc: linux-mm, Mel Gorman, Andrea Arcangeli, Jerome Glisse, Reza Arbab,
Yasuaki Ishimatsu, qiuxishi, Kani Toshimitsu, slaoub, Joonsoo Kim,
Andi Kleen, David Rientjes, Daniel Kiper, Igor Mammedov,
Vitaly Kuznetsov, LKML, Michal Hocko
In-Reply-To: <20170515085827.16474-8-mhocko@kernel.org>
On 05/15/2017 10:58 AM, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
>
> __pageblock_pfn_to_page has two users currently, set_zone_contiguous
> which checks whether the given zone contains holes and
> pageblock_pfn_to_page which then carefully returns a first valid
> page from the given pfn range for the given zone. This doesn't handle
> zones which are not fully populated though. Memory pageblocks can be
> offlined or might not have been onlined yet. In such a case the zone
> should be considered to have holes otherwise pfn walkers can touch
> and play with offline pages.
>
> Current callers of pageblock_pfn_to_page in compaction seem to work
> properly right now because they only isolate PageBuddy
> (isolate_freepages_block) or PageLRU resp. __PageMovable
> (isolate_migratepages_block) which will be always false for these pages.
> It would be safer to skip these pages altogether, though.
>
> In order to do this patch adds a new memory section state
> (SECTION_IS_ONLINE) which is set in memory_present (during boot
> time) or in online_pages_range during the memory hotplug. Similarly
> offline_mem_sections clears the bit and it is called when the memory
> range is offlined.
>
> pfn_to_online_page helper is then added which check the mem section and
> only returns a page if it is onlined already.
>
> Use the new helper in __pageblock_pfn_to_page and skip the whole page
> block in such a case.
>
> Changes since v3
> - clarify pfn_valid semantic - requested by Joonsoo
>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
> include/linux/memory_hotplug.h | 21 ++++++++++++++++++++
> include/linux/mmzone.h | 35 ++++++++++++++++++++++++++------
> mm/memory_hotplug.c | 3 +++
> mm/page_alloc.c | 5 ++++-
> mm/sparse.c | 45 +++++++++++++++++++++++++++++++++++++++++-
> 5 files changed, 101 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 3c8cf86201c3..fc1c873504eb 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -14,6 +14,19 @@ struct memory_block;
> struct resource;
>
> #ifdef CONFIG_MEMORY_HOTPLUG
> +/*
> + * Return page for the valid pfn only if the page is online. All pfn
> + * walkers which rely on the fully initialized page->flags and others
> + * should use this rather than pfn_valid && pfn_to_page
> + */
> +#define pfn_to_online_page(pfn) \
> +({ \
> + struct page *___page = NULL; \
> + \
> + if (online_section_nr(pfn_to_section_nr(pfn))) \
> + ___page = pfn_to_page(pfn); \
> + ___page; \
> +})
This seems to be already assuming pfn_valid() to be true. There's no
"pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS" check and the comment
suggests as such, but...
> /*
> * Types for free bootmem stored in page->lru.next. These have to be in
> @@ -203,6 +216,14 @@ extern void set_zone_contiguous(struct zone *zone);
> extern void clear_zone_contiguous(struct zone *zone);
>
> #else /* ! CONFIG_MEMORY_HOTPLUG */
> +#define pfn_to_online_page(pfn) \
> +({ \
> + struct page *___page = NULL; \
> + if (pfn_valid(pfn)) \
> + ___page = pfn_to_page(pfn); \
This includes the pfn_valid() check itself. Why the discrepancy?
Somebody might develop code with !HOTPLUG and forget the check, and then
it starts breaking with HOTPLUG?
> + ___page; \
> + })
> +
> /*
> * Stub functions for when hotplug is off
> */
...
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 05796ee974f7..c3a146028ba6 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -929,6 +929,9 @@ static int online_pages_range(unsigned long start_pfn, unsigned long nr_pages,
> unsigned long i;
> unsigned long onlined_pages = *(unsigned long *)arg;
> struct page *page;
> +
> + online_mem_sections(start_pfn, start_pfn + nr_pages);
Shouldn't this be moved *below* the loop that initializes struct pages?
In the offline case you do mark sections offline before "tearing" struct
pages, so that should be symmetric.
> +
> if (PageReserved(pfn_to_page(start_pfn)))
> for (i = 0; i < nr_pages; i++) {
> page = pfn_to_page(start_pfn + i);
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index c1670f090107..7e5151a7dd7b 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1353,7 +1353,9 @@ struct page *__pageblock_pfn_to_page(unsigned long start_pfn,
> if (!pfn_valid(start_pfn) || !pfn_valid(end_pfn))
> return NULL;
>
> - start_page = pfn_to_page(start_pfn);
> + start_page = pfn_to_online_page(start_pfn);
> + if (!start_page)
> + return NULL;
>
> if (page_zone(start_page) != zone)
> return NULL;
> @@ -7671,6 +7673,7 @@ __offline_isolated_pages(unsigned long start_pfn, unsigned long end_pfn)
> break;
> if (pfn == end_pfn)
> return;
> + offline_mem_sections(pfn, end_pfn);
> zone = page_zone(pfn_to_page(pfn));
> spin_lock_irqsave(&zone->lock, flags);
> pfn = start_pfn;
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCHv5, REBASED 9/9] x86/mm: Allow to have userspace mappings above 47-bits
From: Michal Hocko @ 2017-05-18 15:59 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Kirill A. Shutemov, x86, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Andi Kleen, Dave Hansen, Andy Lutomirski,
Dan Williams, linux-mm, linux-kernel, linux-api
In-Reply-To: <20170518155003.GB18333@dhcp22.suse.cz>
On Thu 18-05-17 17:50:03, Michal Hocko wrote:
> On Thu 18-05-17 18:41:35, Kirill A. Shutemov wrote:
> > On Thu, May 18, 2017 at 05:27:36PM +0200, Michal Hocko wrote:
> > > On Thu 18-05-17 18:19:52, Kirill A. Shutemov wrote:
> > > > On Thu, May 18, 2017 at 01:43:59PM +0200, Michal Hocko wrote:
> > > > > On Mon 15-05-17 15:12:18, Kirill A. Shutemov wrote:
> > > > > [...]
> > > > > > @@ -195,6 +207,16 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
> > > > > > info.length = len;
> > > > > > info.low_limit = PAGE_SIZE;
> > > > > > info.high_limit = get_mmap_base(0);
> > > > > > +
> > > > > > + /*
> > > > > > + * If hint address is above DEFAULT_MAP_WINDOW, look for unmapped area
> > > > > > + * in the full address space.
> > > > > > + *
> > > > > > + * !in_compat_syscall() check to avoid high addresses for x32.
> > > > > > + */
> > > > > > + if (addr > DEFAULT_MAP_WINDOW && !in_compat_syscall())
> > > > > > + info.high_limit += TASK_SIZE_MAX - DEFAULT_MAP_WINDOW;
> > > > > > +
> > > > > > info.align_mask = 0;
> > > > > > info.align_offset = pgoff << PAGE_SHIFT;
> > > > > > if (filp) {
> > > > >
> > > > > I have two questions/concerns here. The above assumes that any address above
> > > > > 1<<47 will use the _whole_ address space. Is this what we want?
> > > >
> > > > Yes, I believe so.
> > > >
> > > > > What if somebody does mmap(1<<52, ...) because he wants to (ab)use 53+
> > > > > bits for some other purpose? Shouldn't we cap the high_limit by the
> > > > > given address?
> > > >
> > > > This would screw existing semantics of hint address -- "map here if
> > > > free, please".
> > >
> > > Well, the given address is just _hint_. We are still allowed to map to a
> > > different place. And it is not specified whether the resulting mapping
> > > is above or below that address. So I do not think it would screw the
> > > existing semantic. Or do I miss something?
> >
> > You are right, that this behaviour is not fixed by any standard or written
> > down in documentation, but it's de-facto policy of Linux mmap(2) the
> > beginning.
> >
> > And we need to be very careful when messing with this.
>
> I am sorry but I still do not understand. You already touch this
> semantic. mmap(-1UL,...) will already returns basically arbitrary
> address. All I am asking for is that mmap doesn't return higher address
> than the given one whent address > 1<<47. We do not have any such users
> currently so it won't be a change in behavior while it would allow
> different sized address spaces naturally.
I basically mean something like the following
---
diff --git a/arch/x86/kernel/sys_x86_64.c b/arch/x86/kernel/sys_x86_64.c
index 74d1587b181d..d6f66ff02d0a 100644
--- a/arch/x86/kernel/sys_x86_64.c
+++ b/arch/x86/kernel/sys_x86_64.c
@@ -195,7 +195,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
goto bottomup;
/* requesting a specific address */
- if (addr) {
+ if (addr && addr <= DEFAULT_MAP_WINDOW) {
addr = PAGE_ALIGN(addr);
vma = find_vma(mm, addr);
if (TASK_SIZE - len >= addr &&
@@ -215,7 +215,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
* !in_compat_syscall() check to avoid high addresses for x32.
*/
if (addr > DEFAULT_MAP_WINDOW && !in_compat_syscall())
- info.high_limit += TASK_SIZE_MAX - DEFAULT_MAP_WINDOW;
+ info.high_limit += min(TASK_SIZE_MAX, address) - DEFAULT_MAP_WINDOW;
info.align_mask = 0;
info.align_offset = pgoff << PAGE_SHIFT;
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* Re: [RFC PATCH v2 10/17] cgroup: Make debug cgroup support v2 and thread mode
From: Waiman Long @ 2017-05-18 15:58 UTC (permalink / raw)
To: Tejun Heo
Cc: Li Zefan, Johannes Weiner, Peter Zijlstra, Ingo Molnar, cgroups,
linux-kernel, linux-doc, linux-mm, kernel-team, pjt, luto, efault
In-Reply-To: <20170517214338.GG942@htj.duckdns.org>
On 05/17/2017 05:43 PM, Tejun Heo wrote:
> Hello,
>
> On Mon, May 15, 2017 at 09:34:09AM -0400, Waiman Long wrote:
>> Besides supporting cgroup v2 and thread mode, the following changes
>> are also made:
>> 1) current_* cgroup files now resides only at the root as we don't
>> need duplicated files of the same function all over the cgroup
>> hierarchy.
>> 2) The cgroup_css_links_read() function is modified to report
>> the number of tasks that are skipped because of overflow.
>> 3) The relationship between proc_cset and threaded_csets are displayed.
>> 4) The number of extra unaccounted references are displayed.
>> 5) The status of being a thread root or threaded cgroup is displayed.
>> 6) The current_css_set_read() function now prints out the addresses of
>> the css'es associated with the current css_set.
>> 7) A new cgroup_subsys_states file is added to display the css objects
>> associated with a cgroup.
>> 8) A new cgroup_masks file is added to display the various controller
>> bit masks in the cgroup.
>>
>> Signed-off-by: Waiman Long <longman@redhat.com>
> As noted before, please make it clear that this is a debug feature and
> not expected to be stable. Also, I don't see why this and the
> previous two patches are in this series. Can you please split these
> out to a separate patchset?
>
> Thanks.
>
Sure. I can separate out the debug code into a separate patchset. It is
just easier to manage as a single patchset than 2 separate ones.
Regards,
Longman
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC PATCH v2 09/17] cgroup: Keep accurate count of tasks in each css_set
From: Waiman Long @ 2017-05-18 15:56 UTC (permalink / raw)
To: Tejun Heo
Cc: Li Zefan, Johannes Weiner, Peter Zijlstra, Ingo Molnar, cgroups,
linux-kernel, linux-doc, linux-mm, kernel-team, pjt, luto, efault
In-Reply-To: <20170517214034.GF942@htj.duckdns.org>
On 05/17/2017 05:40 PM, Tejun Heo wrote:
> Hello,
>
> On Mon, May 15, 2017 at 09:34:08AM -0400, Waiman Long wrote:
>> The reference count in the css_set data structure was used as a
>> proxy of the number of tasks attached to that css_set. However, that
>> count is actually not an accurate measure especially with thread mode
>> support. So a new variable task_count is added to the css_set to keep
>> track of the actual task count. This new variable is protected by
>> the css_set_lock. Functions that require the actual task count are
>> updated to use the new variable.
>>
>> Signed-off-by: Waiman Long <longman@redhat.com>
> Looks good. We probably should replace css_set_populated() to use
> this too.
>
> Thanks.
>
Yes, you are right. css_set_populated() can be replaced with a check on
the task_count.
Regards,
Longman
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC PATCH v2 08/17] cgroup: Move debug cgroup to its own file
From: Waiman Long @ 2017-05-18 15:52 UTC (permalink / raw)
To: Tejun Heo
Cc: Li Zefan, Johannes Weiner, Peter Zijlstra, Ingo Molnar, cgroups,
linux-kernel, linux-doc, linux-mm, kernel-team, pjt, luto, efault
In-Reply-To: <20170517213603.GE942@htj.duckdns.org>
On 05/17/2017 05:36 PM, Tejun Heo wrote:
> Hello, Waiman.
>
> On Mon, May 15, 2017 at 09:34:07AM -0400, Waiman Long wrote:
>> The debug cgroup currently resides within cgroup-v1.c and is enabled
>> only for v1 cgroup. To enable the debug cgroup also for v2, it
>> makes sense to put the code into its own file as it will no longer
>> be v1 specific. The only change in this patch is the expansion of
>> cgroup_task_count() within the debug_taskcount_read() function.
>>
>> Signed-off-by: Waiman Long <longman@redhat.com>
> I don't mind enabling the debug controller for v2 but let's please
> hide it behind an unwieldy boot param / controller name so that it's
> clear that its interface isn't expected to be stable.
>
> Thanks.
>
The controller name is "debug" and so it is obvious what this controller
is for. However, the config prompt "Example controller" is indeed vague
in meaning. So we can make the prompt more descriptive here. As for the
boot param, are you saying something like "cgroup_debug" has to be
specified in the command line even if CGROUP_DEBUG config is there for
the debug controller to be enabled? I am fine with that if you think it
is necessary.
Regards,
Longman
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCHv5, REBASED 9/9] x86/mm: Allow to have userspace mappings above 47-bits
From: Michal Hocko @ 2017-05-18 15:50 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Kirill A. Shutemov, x86, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Andi Kleen, Dave Hansen, Andy Lutomirski,
Dan Williams, linux-mm, linux-kernel, linux-api
In-Reply-To: <20170518154135.zekuqls6almevrjt@node.shutemov.name>
On Thu 18-05-17 18:41:35, Kirill A. Shutemov wrote:
> On Thu, May 18, 2017 at 05:27:36PM +0200, Michal Hocko wrote:
> > On Thu 18-05-17 18:19:52, Kirill A. Shutemov wrote:
> > > On Thu, May 18, 2017 at 01:43:59PM +0200, Michal Hocko wrote:
> > > > On Mon 15-05-17 15:12:18, Kirill A. Shutemov wrote:
> > > > [...]
> > > > > @@ -195,6 +207,16 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
> > > > > info.length = len;
> > > > > info.low_limit = PAGE_SIZE;
> > > > > info.high_limit = get_mmap_base(0);
> > > > > +
> > > > > + /*
> > > > > + * If hint address is above DEFAULT_MAP_WINDOW, look for unmapped area
> > > > > + * in the full address space.
> > > > > + *
> > > > > + * !in_compat_syscall() check to avoid high addresses for x32.
> > > > > + */
> > > > > + if (addr > DEFAULT_MAP_WINDOW && !in_compat_syscall())
> > > > > + info.high_limit += TASK_SIZE_MAX - DEFAULT_MAP_WINDOW;
> > > > > +
> > > > > info.align_mask = 0;
> > > > > info.align_offset = pgoff << PAGE_SHIFT;
> > > > > if (filp) {
> > > >
> > > > I have two questions/concerns here. The above assumes that any address above
> > > > 1<<47 will use the _whole_ address space. Is this what we want?
> > >
> > > Yes, I believe so.
> > >
> > > > What if somebody does mmap(1<<52, ...) because he wants to (ab)use 53+
> > > > bits for some other purpose? Shouldn't we cap the high_limit by the
> > > > given address?
> > >
> > > This would screw existing semantics of hint address -- "map here if
> > > free, please".
> >
> > Well, the given address is just _hint_. We are still allowed to map to a
> > different place. And it is not specified whether the resulting mapping
> > is above or below that address. So I do not think it would screw the
> > existing semantic. Or do I miss something?
>
> You are right, that this behaviour is not fixed by any standard or written
> down in documentation, but it's de-facto policy of Linux mmap(2) the
> beginning.
>
> And we need to be very careful when messing with this.
I am sorry but I still do not understand. You already touch this
semantic. mmap(-1UL,...) will already returns basically arbitrary
address. All I am asking for is that mmap doesn't return higher address
than the given one whent address > 1<<47. We do not have any such users
currently so it won't be a change in behavior while it would allow
different sized address spaces naturally.
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCHv5, REBASED 9/9] x86/mm: Allow to have userspace mappings above 47-bits
From: Kirill A. Shutemov @ 2017-05-18 15:41 UTC (permalink / raw)
To: Michal Hocko
Cc: Kirill A. Shutemov, x86, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Andi Kleen, Dave Hansen, Andy Lutomirski,
Dan Williams, linux-mm, linux-kernel, linux-api
In-Reply-To: <20170518152736.GA18333@dhcp22.suse.cz>
On Thu, May 18, 2017 at 05:27:36PM +0200, Michal Hocko wrote:
> On Thu 18-05-17 18:19:52, Kirill A. Shutemov wrote:
> > On Thu, May 18, 2017 at 01:43:59PM +0200, Michal Hocko wrote:
> > > On Mon 15-05-17 15:12:18, Kirill A. Shutemov wrote:
> > > [...]
> > > > @@ -195,6 +207,16 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
> > > > info.length = len;
> > > > info.low_limit = PAGE_SIZE;
> > > > info.high_limit = get_mmap_base(0);
> > > > +
> > > > + /*
> > > > + * If hint address is above DEFAULT_MAP_WINDOW, look for unmapped area
> > > > + * in the full address space.
> > > > + *
> > > > + * !in_compat_syscall() check to avoid high addresses for x32.
> > > > + */
> > > > + if (addr > DEFAULT_MAP_WINDOW && !in_compat_syscall())
> > > > + info.high_limit += TASK_SIZE_MAX - DEFAULT_MAP_WINDOW;
> > > > +
> > > > info.align_mask = 0;
> > > > info.align_offset = pgoff << PAGE_SHIFT;
> > > > if (filp) {
> > >
> > > I have two questions/concerns here. The above assumes that any address above
> > > 1<<47 will use the _whole_ address space. Is this what we want?
> >
> > Yes, I believe so.
> >
> > > What if somebody does mmap(1<<52, ...) because he wants to (ab)use 53+
> > > bits for some other purpose? Shouldn't we cap the high_limit by the
> > > given address?
> >
> > This would screw existing semantics of hint address -- "map here if
> > free, please".
>
> Well, the given address is just _hint_. We are still allowed to map to a
> different place. And it is not specified whether the resulting mapping
> is above or below that address. So I do not think it would screw the
> existing semantic. Or do I miss something?
You are right, that this behaviour is not fixed by any standard or written
down in documentation, but it's de-facto policy of Linux mmap(2) the
beginning.
And we need to be very careful when messing with this.
I believe that qemu linux-user to some extend relies on this behaviour to
do 32-bit allocations on 64-bit machine.
https://github.com/qemu/qemu/blob/master/linux-user/mmap.c#L256
--
Kirill A. Shutemov
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC PATCH v2 08/17] cgroup: Move debug cgroup to its own file
From: Waiman Long @ 2017-05-18 15:29 UTC (permalink / raw)
To: Tejun Heo
Cc: Li Zefan, Johannes Weiner, Peter Zijlstra, Ingo Molnar, cgroups,
linux-kernel, linux-doc, linux-mm, kernel-team, pjt, luto, efault
In-Reply-To: <20170517213603.GE942@htj.duckdns.org>
On 05/17/2017 05:36 PM, Tejun Heo wrote:
> Hello, Waiman.
>
> On Mon, May 15, 2017 at 09:34:07AM -0400, Waiman Long wrote:
>> The debug cgroup currently resides within cgroup-v1.c and is enabled
>> only for v1 cgroup. To enable the debug cgroup also for v2, it
>> makes sense to put the code into its own file as it will no longer
>> be v1 specific. The only change in this patch is the expansion of
>> cgroup_task_count() within the debug_taskcount_read() function.
>>
>> Signed-off-by: Waiman Long <longman@redhat.com>
> I don't mind enabling the debug controller for v2 but let's please
> hide it behind an unwieldy boot param / controller name so that it's
> clear that its interface isn't expected to be stable.
>
> Thanks.
>
The controller name is "debug". So it is pretty obvious what it is for.
However, the config prompt of "Example controller" is indeed a bit
vague. So I think we can make the prompt more descriptive here. As for
boot param, are you saying something like "cgroup_debug" has to be
specified in the command line also to have this controller activated
even if the CGROUP_DEBUG config parameter is specified? I am fine with
that if you think it is necessary.
Regards,
Longman
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: Strange condition in invalidate_mapping_pages()
From: Kirill A. Shutemov @ 2017-05-18 15:28 UTC (permalink / raw)
To: Jan Kara; +Cc: Kirill A. Shutemov, linux-mm
In-Reply-To: <20170518132818.GA16430@quack2.suse.cz>
On Thu, May 18, 2017 at 03:28:18PM +0200, Jan Kara wrote:
> Hi Kirill,
>
> in commit fc127da085c26 "truncate: handle file thp" you've added the
> following to invalidate_mapping_pages():
>
> /* Middle of THP: skip */
> if (PageTransTail(page)) {
> unlock_page(page);
> continue;
> } else if (PageTransHuge(page)) {
> index += HPAGE_PMD_NR - 1;
> i += HPAGE_PMD_NR - 1;
> /* 'end' is in the middle of THP */
> if (index == round_down(end, HPAGE_PMD_NR))
> continue;
> }
>
> Now how can ever condition "if (index == round_down(end,
> HPAGE_PMD_NR))" be true? We have just added HPAGE_PMD_NR - 1 to 'index'
> so it will not be a multiple of HPAGE_PMD_NR. Presumably you wanted to
> check whether the current THP is the one containing 'end' here which would
> be something like 'round_down(index, HPAGE_PMD_NR) == round_down(end,
> HPAGE_PMD_NR)'.
You're right, it's a bug. 'page->index' instead of 'index' should do the
trick.
Would you like to prepare the patch? (I'm deep in 5-level paging at the
moment.)
> but then I still miss why you'd like to avoid invalidating the partial
> THP at the end of file... Can you please enlighten me? Thanks!
My logic was that the data in the non-invalidated part of the page can be
still useful and it's better to leave it in page cache.
I don't have performance numbers to validate my intuition.
--
Kirill A. Shutemov
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCHv5, REBASED 9/9] x86/mm: Allow to have userspace mappings above 47-bits
From: Michal Hocko @ 2017-05-18 15:27 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Kirill A. Shutemov, x86, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Andi Kleen, Dave Hansen, Andy Lutomirski,
Dan Williams, linux-mm, linux-kernel, linux-api
In-Reply-To: <20170518151952.jzvz6aeelgx7ifmm@node.shutemov.name>
On Thu 18-05-17 18:19:52, Kirill A. Shutemov wrote:
> On Thu, May 18, 2017 at 01:43:59PM +0200, Michal Hocko wrote:
> > On Mon 15-05-17 15:12:18, Kirill A. Shutemov wrote:
> > [...]
> > > @@ -195,6 +207,16 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
> > > info.length = len;
> > > info.low_limit = PAGE_SIZE;
> > > info.high_limit = get_mmap_base(0);
> > > +
> > > + /*
> > > + * If hint address is above DEFAULT_MAP_WINDOW, look for unmapped area
> > > + * in the full address space.
> > > + *
> > > + * !in_compat_syscall() check to avoid high addresses for x32.
> > > + */
> > > + if (addr > DEFAULT_MAP_WINDOW && !in_compat_syscall())
> > > + info.high_limit += TASK_SIZE_MAX - DEFAULT_MAP_WINDOW;
> > > +
> > > info.align_mask = 0;
> > > info.align_offset = pgoff << PAGE_SHIFT;
> > > if (filp) {
> >
> > I have two questions/concerns here. The above assumes that any address above
> > 1<<47 will use the _whole_ address space. Is this what we want?
>
> Yes, I believe so.
>
> > What if somebody does mmap(1<<52, ...) because he wants to (ab)use 53+
> > bits for some other purpose? Shouldn't we cap the high_limit by the
> > given address?
>
> This would screw existing semantics of hint address -- "map here if
> free, please".
Well, the given address is just _hint_. We are still allowed to map to a
different place. And it is not specified whether the resulting mapping
is above or below that address. So I do not think it would screw the
existing semantic. Or do I miss something?
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 2/2 -v2] drm: drop drm_[cm]alloc* helpers
From: Daniel Vetter @ 2017-05-18 15:23 UTC (permalink / raw)
To: Christian König
Cc: Michal Hocko, dri-devel, linux-mm, linux-kernel, Daniel Vetter
In-Reply-To: <ccf8b461-5fb6-eb58-4b28-54a1e2a7e93c@vodafone.de>
On Thu, May 18, 2017 at 10:16:38AM +0200, Christian Konig wrote:
> Am 17.05.2017 um 14:23 schrieb Michal Hocko:
> > As it turned out my allyesconfig on x86_64 wasn't sufficient and 0day
> > build machinery found a failure on arm architecture. It was clearly a
> > typo. Now I have pushed this to my build battery with cross arch
> > compilers and it passes so there shouldn't more surprises hopefully.
> > Here is the v2.
> > ---
> > From 4a00b3ade5ca4514f7affd8de6f7119c8d5c5a86 Mon Sep 17 00:00:00 2001
> > From: Michal Hocko <mhocko@suse.com>
> > Date: Tue, 16 May 2017 11:00:47 +0200
> > Subject: [PATCH -v2] drm: drop drm_[cm]alloc* helpers
> >
> > Now that drm_[cm]alloc* helpers are simple one line wrappers around
> > kvmalloc_array and drm_free_large is just kvfree alias we can drop
> > them and replace by their native forms.
> >
> > This shouldn't introduce any functional change.
> >
> > Changes since v1
> > - fix typo in drivers/gpu//drm/etnaviv/etnaviv_gem.c - noticed by 0day
> > build robot
> >
> > Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> > Signed-off-by: Michal Hocko <mhocko@suse.com>
>
> Acked-by: Christian Konig <christian.koenig@amd.com>
Found another conflict in vgem (grew another user there in-between), fixed
that up and applied both patches to drm-misc-next.
Thanks, Daniel
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 16 +++----
> > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 19 ++++----
> > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 7 +--
> > drivers/gpu/drm/drm_gem.c | 6 +--
> > drivers/gpu/drm/etnaviv/etnaviv_gem.c | 12 ++---
> > drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 4 +-
> > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 12 ++---
> > drivers/gpu/drm/exynos/exynos_drm_gem.c | 11 +++--
> > drivers/gpu/drm/i915/i915_debugfs.c | 4 +-
> > drivers/gpu/drm/i915/i915_gem.c | 4 +-
> > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 34 +++++++-------
> > drivers/gpu/drm/i915/i915_gem_gtt.c | 6 +--
> > drivers/gpu/drm/i915/i915_gem_userptr.c | 8 ++--
> > drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c | 12 ++---
> > drivers/gpu/drm/msm/msm_gem.c | 10 ++--
> > drivers/gpu/drm/radeon/radeon_cs.c | 11 +++--
> > drivers/gpu/drm/radeon/radeon_gem.c | 2 +-
> > drivers/gpu/drm/radeon/radeon_ring.c | 4 +-
> > drivers/gpu/drm/radeon/radeon_vm.c | 4 +-
> > drivers/gpu/drm/ttm/ttm_tt.c | 13 +++---
> > drivers/gpu/drm/udl/udl_dmabuf.c | 2 +-
> > drivers/gpu/drm/udl/udl_gem.c | 2 +-
> > drivers/gpu/drm/vc4/vc4_gem.c | 15 +++---
> > drivers/gpu/drm/virtio/virtgpu_ioctl.c | 27 +++++------
> > include/drm/drmP.h | 1 -
> > include/drm/drm_mem_util.h | 53 ----------------------
> > 26 files changed, 126 insertions(+), 173 deletions(-)
> > delete mode 100644 include/drm/drm_mem_util.h
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> > index a6649874e6ce..9f0247cdda5e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> > @@ -96,7 +96,7 @@ static int amdgpu_bo_list_set(struct amdgpu_device *adev,
> > int r;
> > unsigned long total_size = 0;
> > - array = drm_malloc_ab(num_entries, sizeof(struct amdgpu_bo_list_entry));
> > + array = kvmalloc_array(num_entries, sizeof(struct amdgpu_bo_list_entry), GFP_KERNEL);
> > if (!array)
> > return -ENOMEM;
> > memset(array, 0, num_entries * sizeof(struct amdgpu_bo_list_entry));
> > @@ -148,7 +148,7 @@ static int amdgpu_bo_list_set(struct amdgpu_device *adev,
> > for (i = 0; i < list->num_entries; ++i)
> > amdgpu_bo_unref(&list->array[i].robj);
> > - drm_free_large(list->array);
> > + kvfree(list->array);
> > list->gds_obj = gds_obj;
> > list->gws_obj = gws_obj;
> > @@ -163,7 +163,7 @@ static int amdgpu_bo_list_set(struct amdgpu_device *adev,
> > error_free:
> > while (i--)
> > amdgpu_bo_unref(&array[i].robj);
> > - drm_free_large(array);
> > + kvfree(array);
> > return r;
> > }
> > @@ -224,7 +224,7 @@ void amdgpu_bo_list_free(struct amdgpu_bo_list *list)
> > amdgpu_bo_unref(&list->array[i].robj);
> > mutex_destroy(&list->lock);
> > - drm_free_large(list->array);
> > + kvfree(list->array);
> > kfree(list);
> > }
> > @@ -244,8 +244,8 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void *data,
> > int r;
> > - info = drm_malloc_ab(args->in.bo_number,
> > - sizeof(struct drm_amdgpu_bo_list_entry));
> > + info = kvmalloc_array(args->in.bo_number,
> > + sizeof(struct drm_amdgpu_bo_list_entry), GFP_KERNEL);
> > if (!info)
> > return -ENOMEM;
> > @@ -311,11 +311,11 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void *data,
> > memset(args, 0, sizeof(*args));
> > args->out.list_handle = handle;
> > - drm_free_large(info);
> > + kvfree(info);
> > return 0;
> > error_free:
> > - drm_free_large(info);
> > + kvfree(info);
> > return r;
> > }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > index 4e6b9501ab0a..5b3e0f63a115 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > @@ -194,7 +194,7 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, void *data)
> > size = p->chunks[i].length_dw;
> > cdata = (void __user *)(uintptr_t)user_chunk.chunk_data;
> > - p->chunks[i].kdata = drm_malloc_ab(size, sizeof(uint32_t));
> > + p->chunks[i].kdata = kvmalloc_array(size, sizeof(uint32_t), GFP_KERNEL);
> > if (p->chunks[i].kdata == NULL) {
> > ret = -ENOMEM;
> > i--;
> > @@ -247,7 +247,7 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, void *data)
> > i = p->nchunks - 1;
> > free_partial_kdata:
> > for (; i >= 0; i--)
> > - drm_free_large(p->chunks[i].kdata);
> > + kvfree(p->chunks[i].kdata);
> > kfree(p->chunks);
> > p->chunks = NULL;
> > p->nchunks = 0;
> > @@ -505,7 +505,7 @@ static int amdgpu_cs_list_validate(struct amdgpu_cs_parser *p,
> > return r;
> > if (binding_userptr) {
> > - drm_free_large(lobj->user_pages);
> > + kvfree(lobj->user_pages);
> > lobj->user_pages = NULL;
> > }
> > }
> > @@ -571,7 +571,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
> > release_pages(e->user_pages,
> > e->robj->tbo.ttm->num_pages,
> > false);
> > - drm_free_large(e->user_pages);
> > + kvfree(e->user_pages);
> > e->user_pages = NULL;
> > }
> > @@ -601,8 +601,9 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
> > list_for_each_entry(e, &need_pages, tv.head) {
> > struct ttm_tt *ttm = e->robj->tbo.ttm;
> > - e->user_pages = drm_calloc_large(ttm->num_pages,
> > - sizeof(struct page*));
> > + e->user_pages = kvmalloc_array(ttm->num_pages,
> > + sizeof(struct page*),
> > + GFP_KERNEL | __GFP_ZERO);
> > if (!e->user_pages) {
> > r = -ENOMEM;
> > DRM_ERROR("calloc failure in %s\n", __func__);
> > @@ -612,7 +613,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
> > r = amdgpu_ttm_tt_get_user_pages(ttm, e->user_pages);
> > if (r) {
> > DRM_ERROR("amdgpu_ttm_tt_get_user_pages failed.\n");
> > - drm_free_large(e->user_pages);
> > + kvfree(e->user_pages);
> > e->user_pages = NULL;
> > goto error_free_pages;
> > }
> > @@ -708,7 +709,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
> > release_pages(e->user_pages,
> > e->robj->tbo.ttm->num_pages,
> > false);
> > - drm_free_large(e->user_pages);
> > + kvfree(e->user_pages);
> > }
> > }
> > @@ -761,7 +762,7 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error, bo
> > amdgpu_bo_list_put(parser->bo_list);
> > for (i = 0; i < parser->nchunks; i++)
> > - drm_free_large(parser->chunks[i].kdata);
> > + kvfree(parser->chunks[i].kdata);
> > kfree(parser->chunks);
> > if (parser->job)
> > amdgpu_job_free(parser->job);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > index 07ff3b1514f1..749a6cde7985 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > @@ -279,8 +279,9 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
> > if (!parent->entries) {
> > unsigned num_entries = amdgpu_vm_num_entries(adev, level);
> > - parent->entries = drm_calloc_large(num_entries,
> > - sizeof(struct amdgpu_vm_pt));
> > + parent->entries = kvmalloc_array(num_entries,
> > + sizeof(struct amdgpu_vm_pt),
> > + GFP_KERNEL | __GFP_ZERO);
> > if (!parent->entries)
> > return -ENOMEM;
> > memset(parent->entries, 0 , sizeof(struct amdgpu_vm_pt));
> > @@ -2198,7 +2199,7 @@ static void amdgpu_vm_free_levels(struct amdgpu_vm_pt *level)
> > for (i = 0; i <= level->last_entry_used; i++)
> > amdgpu_vm_free_levels(&level->entries[i]);
> > - drm_free_large(level->entries);
> > + kvfree(level->entries);
> > }
> > /**
> > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > index b1e28c944637..8dc11064253d 100644
> > --- a/drivers/gpu/drm/drm_gem.c
> > +++ b/drivers/gpu/drm/drm_gem.c
> > @@ -521,7 +521,7 @@ struct page **drm_gem_get_pages(struct drm_gem_object *obj)
> > npages = obj->size >> PAGE_SHIFT;
> > - pages = drm_malloc_ab(npages, sizeof(struct page *));
> > + pages = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
> > if (pages == NULL)
> > return ERR_PTR(-ENOMEM);
> > @@ -546,7 +546,7 @@ struct page **drm_gem_get_pages(struct drm_gem_object *obj)
> > while (i--)
> > put_page(pages[i]);
> > - drm_free_large(pages);
> > + kvfree(pages);
> > return ERR_CAST(p);
> > }
> > EXPORT_SYMBOL(drm_gem_get_pages);
> > @@ -582,7 +582,7 @@ void drm_gem_put_pages(struct drm_gem_object *obj, struct page **pages,
> > put_page(pages[i]);
> > }
> > - drm_free_large(pages);
> > + kvfree(pages);
> > }
> > EXPORT_SYMBOL(drm_gem_put_pages);
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> > index fd56f92f3469..d6fb724fc3cc 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> > @@ -748,7 +748,7 @@ static struct page **etnaviv_gem_userptr_do_get_pages(
> > uintptr_t ptr;
> > unsigned int flags = 0;
> > - pvec = drm_malloc_ab(npages, sizeof(struct page *));
> > + pvec = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
> > if (!pvec)
> > return ERR_PTR(-ENOMEM);
> > @@ -772,7 +772,7 @@ static struct page **etnaviv_gem_userptr_do_get_pages(
> > if (ret < 0) {
> > release_pages(pvec, pinned, 0);
> > - drm_free_large(pvec);
> > + kvfree(pvec);
> > return ERR_PTR(ret);
> > }
> > @@ -823,7 +823,7 @@ static int etnaviv_gem_userptr_get_pages(struct etnaviv_gem_object *etnaviv_obj)
> > mm = get_task_mm(etnaviv_obj->userptr.task);
> > pinned = 0;
> > if (mm == current->mm) {
> > - pvec = drm_malloc_ab(npages, sizeof(struct page *));
> > + pvec = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
> > if (!pvec) {
> > mmput(mm);
> > return -ENOMEM;
> > @@ -832,7 +832,7 @@ static int etnaviv_gem_userptr_get_pages(struct etnaviv_gem_object *etnaviv_obj)
> > pinned = __get_user_pages_fast(etnaviv_obj->userptr.ptr, npages,
> > !etnaviv_obj->userptr.ro, pvec);
> > if (pinned < 0) {
> > - drm_free_large(pvec);
> > + kvfree(pvec);
> > mmput(mm);
> > return pinned;
> > }
> > @@ -845,7 +845,7 @@ static int etnaviv_gem_userptr_get_pages(struct etnaviv_gem_object *etnaviv_obj)
> > }
> > release_pages(pvec, pinned, 0);
> > - drm_free_large(pvec);
> > + kvfree(pvec);
> > work = kmalloc(sizeof(*work), GFP_KERNEL);
> > if (!work) {
> > @@ -879,7 +879,7 @@ static void etnaviv_gem_userptr_release(struct etnaviv_gem_object *etnaviv_obj)
> > int npages = etnaviv_obj->base.size >> PAGE_SHIFT;
> > release_pages(etnaviv_obj->pages, npages, 0);
> > - drm_free_large(etnaviv_obj->pages);
> > + kvfree(etnaviv_obj->pages);
> > }
> > put_task_struct(etnaviv_obj->userptr.task);
> > }
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> > index abed6f781281..e5da4f2300ba 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> > @@ -87,7 +87,7 @@ static void etnaviv_gem_prime_release(struct etnaviv_gem_object *etnaviv_obj)
> > * ours, just free the array we allocated:
> > */
> > if (etnaviv_obj->pages)
> > - drm_free_large(etnaviv_obj->pages);
> > + kvfree(etnaviv_obj->pages);
> > drm_prime_gem_destroy(&etnaviv_obj->base, etnaviv_obj->sgt);
> > }
> > @@ -128,7 +128,7 @@ struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
> > npages = size / PAGE_SIZE;
> > etnaviv_obj->sgt = sgt;
> > - etnaviv_obj->pages = drm_malloc_ab(npages, sizeof(struct page *));
> > + etnaviv_obj->pages = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
> > if (!etnaviv_obj->pages) {
> > ret = -ENOMEM;
> > goto fail;
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > index de80ee1b71df..ee7069e93eda 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > @@ -345,9 +345,9 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
> > * Copy the command submission and bo array to kernel space in
> > * one go, and do this outside of any locks.
> > */
> > - bos = drm_malloc_ab(args->nr_bos, sizeof(*bos));
> > - relocs = drm_malloc_ab(args->nr_relocs, sizeof(*relocs));
> > - stream = drm_malloc_ab(1, args->stream_size);
> > + bos = kvmalloc_array(args->nr_bos, sizeof(*bos), GFP_KERNEL);
> > + relocs = kvmalloc_array(args->nr_relocs, sizeof(*relocs), GFP_KERNEL);
> > + stream = kvmalloc_array(1, args->stream_size, GFP_KERNEL);
> > cmdbuf = etnaviv_cmdbuf_new(gpu->cmdbuf_suballoc,
> > ALIGN(args->stream_size, 8) + 8,
> > args->nr_bos);
> > @@ -489,11 +489,11 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
> > if (cmdbuf)
> > etnaviv_cmdbuf_free(cmdbuf);
> > if (stream)
> > - drm_free_large(stream);
> > + kvfree(stream);
> > if (bos)
> > - drm_free_large(bos);
> > + kvfree(bos);
> > if (relocs)
> > - drm_free_large(relocs);
> > + kvfree(relocs);
> > return ret;
> > }
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> > index 55a1579d11b3..c23479be4850 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> > @@ -59,7 +59,8 @@ static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem)
> > nr_pages = exynos_gem->size >> PAGE_SHIFT;
> > - exynos_gem->pages = drm_calloc_large(nr_pages, sizeof(struct page *));
> > + exynos_gem->pages = kvmalloc_array(nr_pages, sizeof(struct page *),
> > + GFP_KERNEL | __GFP_ZERO);
> > if (!exynos_gem->pages) {
> > DRM_ERROR("failed to allocate pages.\n");
> > return -ENOMEM;
> > @@ -101,7 +102,7 @@ static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem)
> > dma_free_attrs(to_dma_dev(dev), exynos_gem->size, exynos_gem->cookie,
> > exynos_gem->dma_addr, exynos_gem->dma_attrs);
> > err_free:
> > - drm_free_large(exynos_gem->pages);
> > + kvfree(exynos_gem->pages);
> > return ret;
> > }
> > @@ -122,7 +123,7 @@ static void exynos_drm_free_buf(struct exynos_drm_gem *exynos_gem)
> > (dma_addr_t)exynos_gem->dma_addr,
> > exynos_gem->dma_attrs);
> > - drm_free_large(exynos_gem->pages);
> > + kvfree(exynos_gem->pages);
> > }
> > static int exynos_drm_gem_handle_create(struct drm_gem_object *obj,
> > @@ -559,7 +560,7 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
> > exynos_gem->dma_addr = sg_dma_address(sgt->sgl);
> > npages = exynos_gem->size >> PAGE_SHIFT;
> > - exynos_gem->pages = drm_malloc_ab(npages, sizeof(struct page *));
> > + exynos_gem->pages = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
> > if (!exynos_gem->pages) {
> > ret = -ENOMEM;
> > goto err;
> > @@ -588,7 +589,7 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
> > return &exynos_gem->base;
> > err_free_large:
> > - drm_free_large(exynos_gem->pages);
> > + kvfree(exynos_gem->pages);
> > err:
> > drm_gem_object_release(&exynos_gem->base);
> > kfree(exynos_gem);
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index bd9abef40c66..c8f3c0cc79fb 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -229,7 +229,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
> > int ret;
> > total = READ_ONCE(dev_priv->mm.object_count);
> > - objects = drm_malloc_ab(total, sizeof(*objects));
> > + objects = kvmalloc_array(total, sizeof(*objects), GFP_KERNEL);
> > if (!objects)
> > return -ENOMEM;
> > @@ -274,7 +274,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
> > mutex_unlock(&dev->struct_mutex);
> > out:
> > - drm_free_large(objects);
> > + kvfree(objects);
> > return ret;
> > }
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 0c1cbe98c994..aa790a6d38e2 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -2556,7 +2556,7 @@ static void *i915_gem_object_map(const struct drm_i915_gem_object *obj,
> > if (n_pages > ARRAY_SIZE(stack_pages)) {
> > /* Too big for stack -- allocate temporary array instead */
> > - pages = drm_malloc_gfp(n_pages, sizeof(*pages), GFP_TEMPORARY);
> > + pages = kvmalloc_array(n_pages, sizeof(*pages), GFP_TEMPORARY);
> > if (!pages)
> > return NULL;
> > }
> > @@ -2578,7 +2578,7 @@ static void *i915_gem_object_map(const struct drm_i915_gem_object *obj,
> > addr = vmap(pages, n_pages, 0, pgprot);
> > if (pages != stack_pages)
> > - drm_free_large(pages);
> > + kvfree(pages);
> > return addr;
> > }
> > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > index af1965774e7b..04211c970b9f 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > @@ -1019,11 +1019,11 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
> > for (i = 0; i < count; i++)
> > total += exec[i].relocation_count;
> > - reloc_offset = drm_malloc_ab(count, sizeof(*reloc_offset));
> > - reloc = drm_malloc_ab(total, sizeof(*reloc));
> > + reloc_offset = kvmalloc_array(count, sizeof(*reloc_offset), GFP_KERNEL);
> > + reloc = kvmalloc_array(total, sizeof(*reloc), GFP_KERNEL);
> > if (reloc == NULL || reloc_offset == NULL) {
> > - drm_free_large(reloc);
> > - drm_free_large(reloc_offset);
> > + kvfree(reloc);
> > + kvfree(reloc_offset);
> > mutex_lock(&dev->struct_mutex);
> > return -ENOMEM;
> > }
> > @@ -1099,8 +1099,8 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
> > */
> > err:
> > - drm_free_large(reloc);
> > - drm_free_large(reloc_offset);
> > + kvfree(reloc);
> > + kvfree(reloc_offset);
> > return ret;
> > }
> > @@ -1871,13 +1871,13 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
> > }
> > /* Copy in the exec list from userland */
> > - exec_list = drm_malloc_ab(sizeof(*exec_list), args->buffer_count);
> > - exec2_list = drm_malloc_ab(sizeof(*exec2_list), args->buffer_count);
> > + exec_list = kvmalloc_array(sizeof(*exec_list), args->buffer_count, GFP_KERNEL);
> > + exec2_list = kvmalloc_array(sizeof(*exec2_list), args->buffer_count, GFP_KERNEL);
> > if (exec_list == NULL || exec2_list == NULL) {
> > DRM_DEBUG("Failed to allocate exec list for %d buffers\n",
> > args->buffer_count);
> > - drm_free_large(exec_list);
> > - drm_free_large(exec2_list);
> > + kvfree(exec_list);
> > + kvfree(exec2_list);
> > return -ENOMEM;
> > }
> > ret = copy_from_user(exec_list,
> > @@ -1886,8 +1886,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
> > if (ret != 0) {
> > DRM_DEBUG("copy %d exec entries failed %d\n",
> > args->buffer_count, ret);
> > - drm_free_large(exec_list);
> > - drm_free_large(exec2_list);
> > + kvfree(exec_list);
> > + kvfree(exec2_list);
> > return -EFAULT;
> > }
> > @@ -1936,8 +1936,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
> > }
> > }
> > - drm_free_large(exec_list);
> > - drm_free_large(exec2_list);
> > + kvfree(exec_list);
> > + kvfree(exec2_list);
> > return ret;
> > }
> > @@ -1955,7 +1955,7 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
> > return -EINVAL;
> > }
> > - exec2_list = drm_malloc_gfp(args->buffer_count,
> > + exec2_list = kvmalloc_array(args->buffer_count,
> > sizeof(*exec2_list),
> > GFP_TEMPORARY);
> > if (exec2_list == NULL) {
> > @@ -1969,7 +1969,7 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
> > if (ret != 0) {
> > DRM_DEBUG("copy %d exec entries failed %d\n",
> > args->buffer_count, ret);
> > - drm_free_large(exec2_list);
> > + kvfree(exec2_list);
> > return -EFAULT;
> > }
> > @@ -1996,6 +1996,6 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
> > }
> > }
> > - drm_free_large(exec2_list);
> > + kvfree(exec2_list);
> > return ret;
> > }
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index bc3c63e92c16..899427863547 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -3114,7 +3114,7 @@ intel_rotate_pages(struct intel_rotation_info *rot_info,
> > int ret = -ENOMEM;
> > /* Allocate a temporary list of source pages for random access. */
> > - page_addr_list = drm_malloc_gfp(n_pages,
> > + page_addr_list = kvmalloc_array(n_pages,
> > sizeof(dma_addr_t),
> > GFP_TEMPORARY);
> > if (!page_addr_list)
> > @@ -3147,14 +3147,14 @@ intel_rotate_pages(struct intel_rotation_info *rot_info,
> > DRM_DEBUG_KMS("Created rotated page mapping for object size %zu (%ux%u tiles, %u pages)\n",
> > obj->base.size, rot_info->plane[0].width, rot_info->plane[0].height, size);
> > - drm_free_large(page_addr_list);
> > + kvfree(page_addr_list);
> > return st;
> > err_sg_alloc:
> > kfree(st);
> > err_st_alloc:
> > - drm_free_large(page_addr_list);
> > + kvfree(page_addr_list);
> > DRM_DEBUG_KMS("Failed to create rotated mapping for object size %zu! (%ux%u tiles, %u pages)\n",
> > obj->base.size, rot_info->plane[0].width, rot_info->plane[0].height, size);
> > diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
> > index 58ccf8b8ca1c..1a0ce1dc68f5 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_userptr.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
> > @@ -507,7 +507,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work)
> > ret = -ENOMEM;
> > pinned = 0;
> > - pvec = drm_malloc_gfp(npages, sizeof(struct page *), GFP_TEMPORARY);
> > + pvec = kvmalloc_array(npages, sizeof(struct page *), GFP_TEMPORARY);
> > if (pvec != NULL) {
> > struct mm_struct *mm = obj->userptr.mm->mm;
> > unsigned int flags = 0;
> > @@ -555,7 +555,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work)
> > mutex_unlock(&obj->mm.lock);
> > release_pages(pvec, pinned, 0);
> > - drm_free_large(pvec);
> > + kvfree(pvec);
> > i915_gem_object_put(obj);
> > put_task_struct(work->task);
> > @@ -642,7 +642,7 @@ i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj)
> > pinned = 0;
> > if (mm == current->mm) {
> > - pvec = drm_malloc_gfp(num_pages, sizeof(struct page *),
> > + pvec = kvmalloc_array(num_pages, sizeof(struct page *),
> > GFP_TEMPORARY |
> > __GFP_NORETRY |
> > __GFP_NOWARN);
> > @@ -669,7 +669,7 @@ i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj)
> > if (IS_ERR(pages))
> > release_pages(pvec, pinned, 0);
> > - drm_free_large(pvec);
> > + kvfree(pvec);
> > return pages;
> > }
> > diff --git a/drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c b/drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c
> > index 19860a372d90..7276194c04f7 100644
> > --- a/drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c
> > +++ b/drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c
> > @@ -117,7 +117,7 @@ static int igt_random_insert_remove(void *arg)
> > mock_engine_reset(engine);
> > - waiters = drm_malloc_gfp(count, sizeof(*waiters), GFP_TEMPORARY);
> > + waiters = kvmalloc_array(count, sizeof(*waiters), GFP_TEMPORARY);
> > if (!waiters)
> > goto out_engines;
> > @@ -169,7 +169,7 @@ static int igt_random_insert_remove(void *arg)
> > out_bitmap:
> > kfree(bitmap);
> > out_waiters:
> > - drm_free_large(waiters);
> > + kvfree(waiters);
> > out_engines:
> > mock_engine_flush(engine);
> > return err;
> > @@ -187,7 +187,7 @@ static int igt_insert_complete(void *arg)
> > mock_engine_reset(engine);
> > - waiters = drm_malloc_gfp(count, sizeof(*waiters), GFP_TEMPORARY);
> > + waiters = kvmalloc_array(count, sizeof(*waiters), GFP_TEMPORARY);
> > if (!waiters)
> > goto out_engines;
> > @@ -254,7 +254,7 @@ static int igt_insert_complete(void *arg)
> > out_bitmap:
> > kfree(bitmap);
> > out_waiters:
> > - drm_free_large(waiters);
> > + kvfree(waiters);
> > out_engines:
> > mock_engine_flush(engine);
> > return err;
> > @@ -368,7 +368,7 @@ static int igt_wakeup(void *arg)
> > mock_engine_reset(engine);
> > - waiters = drm_malloc_gfp(count, sizeof(*waiters), GFP_TEMPORARY);
> > + waiters = kvmalloc_array(count, sizeof(*waiters), GFP_TEMPORARY);
> > if (!waiters)
> > goto out_engines;
> > @@ -454,7 +454,7 @@ static int igt_wakeup(void *arg)
> > put_task_struct(waiters[n].tsk);
> > }
> > - drm_free_large(waiters);
> > + kvfree(waiters);
> > out_engines:
> > mock_engine_flush(engine);
> > return err;
> > diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> > index 68e509b3b9e4..465dab942afa 100644
> > --- a/drivers/gpu/drm/msm/msm_gem.c
> > +++ b/drivers/gpu/drm/msm/msm_gem.c
> > @@ -50,13 +50,13 @@ static struct page **get_pages_vram(struct drm_gem_object *obj,
> > struct page **p;
> > int ret, i;
> > - p = drm_malloc_ab(npages, sizeof(struct page *));
> > + p = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
> > if (!p)
> > return ERR_PTR(-ENOMEM);
> > ret = drm_mm_insert_node(&priv->vram.mm, msm_obj->vram_node, npages);
> > if (ret) {
> > - drm_free_large(p);
> > + kvfree(p);
> > return ERR_PTR(ret);
> > }
> > @@ -127,7 +127,7 @@ static void put_pages(struct drm_gem_object *obj)
> > drm_gem_put_pages(obj, msm_obj->pages, true, false);
> > else {
> > drm_mm_remove_node(msm_obj->vram_node);
> > - drm_free_large(msm_obj->pages);
> > + kvfree(msm_obj->pages);
> > }
> > msm_obj->pages = NULL;
> > @@ -707,7 +707,7 @@ void msm_gem_free_object(struct drm_gem_object *obj)
> > * ours, just free the array we allocated:
> > */
> > if (msm_obj->pages)
> > - drm_free_large(msm_obj->pages);
> > + kvfree(msm_obj->pages);
> > drm_prime_gem_destroy(obj, msm_obj->sgt);
> > } else {
> > @@ -863,7 +863,7 @@ struct drm_gem_object *msm_gem_import(struct drm_device *dev,
> > msm_obj = to_msm_bo(obj);
> > msm_obj->sgt = sgt;
> > - msm_obj->pages = drm_malloc_ab(npages, sizeof(struct page *));
> > + msm_obj->pages = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
> > if (!msm_obj->pages) {
> > ret = -ENOMEM;
> > goto fail;
> > diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c
> > index 3ac671f6c8e1..00b22af70f5c 100644
> > --- a/drivers/gpu/drm/radeon/radeon_cs.c
> > +++ b/drivers/gpu/drm/radeon/radeon_cs.c
> > @@ -87,7 +87,8 @@ static int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
> > p->dma_reloc_idx = 0;
> > /* FIXME: we assume that each relocs use 4 dwords */
> > p->nrelocs = chunk->length_dw / 4;
> > - p->relocs = drm_calloc_large(p->nrelocs, sizeof(struct radeon_bo_list));
> > + p->relocs = kvmalloc_array(p->nrelocs, sizeof(struct radeon_bo_list),
> > + GFP_KERNEL | __GFP_ZERO);
> > if (p->relocs == NULL) {
> > return -ENOMEM;
> > }
> > @@ -341,7 +342,7 @@ int radeon_cs_parser_init(struct radeon_cs_parser *p, void *data)
> > continue;
> > }
> > - p->chunks[i].kdata = drm_malloc_ab(size, sizeof(uint32_t));
> > + p->chunks[i].kdata = kvmalloc_array(size, sizeof(uint32_t), GFP_KERNEL);
> > size *= sizeof(uint32_t);
> > if (p->chunks[i].kdata == NULL) {
> > return -ENOMEM;
> > @@ -440,10 +441,10 @@ static void radeon_cs_parser_fini(struct radeon_cs_parser *parser, int error, bo
> > }
> > }
> > kfree(parser->track);
> > - drm_free_large(parser->relocs);
> > - drm_free_large(parser->vm_bos);
> > + kvfree(parser->relocs);
> > + kvfree(parser->vm_bos);
> > for (i = 0; i < parser->nchunks; i++)
> > - drm_free_large(parser->chunks[i].kdata);
> > + kvfree(parser->chunks[i].kdata);
> > kfree(parser->chunks);
> > kfree(parser->chunks_array);
> > radeon_ib_free(parser->rdev, &parser->ib);
> > diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
> > index dddb372de2b9..574bf7e6b118 100644
> > --- a/drivers/gpu/drm/radeon/radeon_gem.c
> > +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> > @@ -587,7 +587,7 @@ static void radeon_gem_va_update_vm(struct radeon_device *rdev,
> > ttm_eu_backoff_reservation(&ticket, &list);
> > error_free:
> > - drm_free_large(vm_bos);
> > + kvfree(vm_bos);
> > if (r && r != -ERESTARTSYS)
> > DRM_ERROR("Couldn't update BO_VA (%d)\n", r);
> > diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
> > index 8c7872339c2a..84802b201bef 100644
> > --- a/drivers/gpu/drm/radeon/radeon_ring.c
> > +++ b/drivers/gpu/drm/radeon/radeon_ring.c
> > @@ -314,7 +314,7 @@ unsigned radeon_ring_backup(struct radeon_device *rdev, struct radeon_ring *ring
> > }
> > /* and then save the content of the ring */
> > - *data = drm_malloc_ab(size, sizeof(uint32_t));
> > + *data = kvmalloc_array(size, sizeof(uint32_t), GFP_KERNEL);
> > if (!*data) {
> > mutex_unlock(&rdev->ring_lock);
> > return 0;
> > @@ -356,7 +356,7 @@ int radeon_ring_restore(struct radeon_device *rdev, struct radeon_ring *ring,
> > }
> > radeon_ring_unlock_commit(rdev, ring, false);
> > - drm_free_large(data);
> > + kvfree(data);
> > return 0;
> > }
> > diff --git a/drivers/gpu/drm/radeon/radeon_vm.c b/drivers/gpu/drm/radeon/radeon_vm.c
> > index a1358748cea5..5f68245579a3 100644
> > --- a/drivers/gpu/drm/radeon/radeon_vm.c
> > +++ b/drivers/gpu/drm/radeon/radeon_vm.c
> > @@ -132,8 +132,8 @@ struct radeon_bo_list *radeon_vm_get_bos(struct radeon_device *rdev,
> > struct radeon_bo_list *list;
> > unsigned i, idx;
> > - list = drm_malloc_ab(vm->max_pde_used + 2,
> > - sizeof(struct radeon_bo_list));
> > + list = kvmalloc_array(vm->max_pde_used + 2,
> > + sizeof(struct radeon_bo_list), GFP_KERNEL);
> > if (!list)
> > return NULL;
> > diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> > index 5260179d788a..8ebc8d3560c3 100644
> > --- a/drivers/gpu/drm/ttm/ttm_tt.c
> > +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> > @@ -39,7 +39,6 @@
> > #include <linux/slab.h>
> > #include <linux/export.h>
> > #include <drm/drm_cache.h>
> > -#include <drm/drm_mem_util.h>
> > #include <drm/ttm/ttm_module.h>
> > #include <drm/ttm/ttm_bo_driver.h>
> > #include <drm/ttm/ttm_placement.h>
> > @@ -53,14 +52,16 @@
> > */
> > static void ttm_tt_alloc_page_directory(struct ttm_tt *ttm)
> > {
> > - ttm->pages = drm_calloc_large(ttm->num_pages, sizeof(void*));
> > + ttm->pages = kvmalloc_array(ttm->num_pages, sizeof(void*),
> > + GFP_KERNEL | __GFP_ZERO);
> > }
> > static void ttm_dma_tt_alloc_page_directory(struct ttm_dma_tt *ttm)
> > {
> > - ttm->ttm.pages = drm_calloc_large(ttm->ttm.num_pages,
> > + ttm->ttm.pages = kvmalloc_array(ttm->ttm.num_pages,
> > sizeof(*ttm->ttm.pages) +
> > - sizeof(*ttm->dma_address));
> > + sizeof(*ttm->dma_address),
> > + GFP_KERNEL | __GFP_ZERO);
> > ttm->dma_address = (void *) (ttm->ttm.pages + ttm->ttm.num_pages);
> > }
> > @@ -208,7 +209,7 @@ EXPORT_SYMBOL(ttm_tt_init);
> > void ttm_tt_fini(struct ttm_tt *ttm)
> > {
> > - drm_free_large(ttm->pages);
> > + kvfree(ttm->pages);
> > ttm->pages = NULL;
> > }
> > EXPORT_SYMBOL(ttm_tt_fini);
> > @@ -243,7 +244,7 @@ void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma)
> > {
> > struct ttm_tt *ttm = &ttm_dma->ttm;
> > - drm_free_large(ttm->pages);
> > + kvfree(ttm->pages);
> > ttm->pages = NULL;
> > ttm_dma->dma_address = NULL;
> > }
> > diff --git a/drivers/gpu/drm/udl/udl_dmabuf.c b/drivers/gpu/drm/udl/udl_dmabuf.c
> > index ed0e636243b2..2e031a894813 100644
> > --- a/drivers/gpu/drm/udl/udl_dmabuf.c
> > +++ b/drivers/gpu/drm/udl/udl_dmabuf.c
> > @@ -228,7 +228,7 @@ static int udl_prime_create(struct drm_device *dev,
> > return -ENOMEM;
> > obj->sg = sg;
> > - obj->pages = drm_malloc_ab(npages, sizeof(struct page *));
> > + obj->pages = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
> > if (obj->pages == NULL) {
> > DRM_ERROR("obj pages is NULL %d\n", npages);
> > return -ENOMEM;
> > diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
> > index 775c50e4f02c..db9ceceba30e 100644
> > --- a/drivers/gpu/drm/udl/udl_gem.c
> > +++ b/drivers/gpu/drm/udl/udl_gem.c
> > @@ -146,7 +146,7 @@ int udl_gem_get_pages(struct udl_gem_object *obj)
> > void udl_gem_put_pages(struct udl_gem_object *obj)
> > {
> > if (obj->base.import_attach) {
> > - drm_free_large(obj->pages);
> > + kvfree(obj->pages);
> > obj->pages = NULL;
> > return;
> > }
> > diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
> > index e9c381c42139..bf466674ca9b 100644
> > --- a/drivers/gpu/drm/vc4/vc4_gem.c
> > +++ b/drivers/gpu/drm/vc4/vc4_gem.c
> > @@ -545,14 +545,15 @@ vc4_cl_lookup_bos(struct drm_device *dev,
> > return -EINVAL;
> > }
> > - exec->bo = drm_calloc_large(exec->bo_count,
> > - sizeof(struct drm_gem_cma_object *));
> > + exec->bo = kvmalloc_array(exec->bo_count,
> > + sizeof(struct drm_gem_cma_object *),
> > + GFP_KERNEL | __GFP_ZERO);
> > if (!exec->bo) {
> > DRM_ERROR("Failed to allocate validated BO pointers\n");
> > return -ENOMEM;
> > }
> > - handles = drm_malloc_ab(exec->bo_count, sizeof(uint32_t));
> > + handles = kvmalloc_array(exec->bo_count, sizeof(uint32_t), GFP_KERNEL);
> > if (!handles) {
> > ret = -ENOMEM;
> > DRM_ERROR("Failed to allocate incoming GEM handles\n");
> > @@ -584,7 +585,7 @@ vc4_cl_lookup_bos(struct drm_device *dev,
> > spin_unlock(&file_priv->table_lock);
> > fail:
> > - drm_free_large(handles);
> > + kvfree(handles);
> > return ret;
> > }
> > @@ -622,7 +623,7 @@ vc4_get_bcl(struct drm_device *dev, struct vc4_exec_info *exec)
> > * read the contents back for validation, and I think the
> > * bo->vaddr is uncached access.
> > */
> > - temp = drm_malloc_ab(temp_size, 1);
> > + temp = kvmalloc_array(temp_size, 1, GFP_KERNEL);
> > if (!temp) {
> > DRM_ERROR("Failed to allocate storage for copying "
> > "in bin/render CLs.\n");
> > @@ -697,7 +698,7 @@ vc4_get_bcl(struct drm_device *dev, struct vc4_exec_info *exec)
> > ret = vc4_wait_for_seqno(dev, exec->bin_dep_seqno, ~0ull, true);
> > fail:
> > - drm_free_large(temp);
> > + kvfree(temp);
> > return ret;
> > }
> > @@ -710,7 +711,7 @@ vc4_complete_exec(struct drm_device *dev, struct vc4_exec_info *exec)
> > if (exec->bo) {
> > for (i = 0; i < exec->bo_count; i++)
> > drm_gem_object_unreference_unlocked(&exec->bo[i]->base);
> > - drm_free_large(exec->bo);
> > + kvfree(exec->bo);
> > }
> > while (!list_empty(&exec->unref_list)) {
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> > index 61f3a963af95..6ed4bfc9b82b 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> > @@ -119,13 +119,14 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
> > INIT_LIST_HEAD(&validate_list);
> > if (exbuf->num_bo_handles) {
> > - bo_handles = drm_malloc_ab(exbuf->num_bo_handles,
> > - sizeof(uint32_t));
> > - buflist = drm_calloc_large(exbuf->num_bo_handles,
> > - sizeof(struct ttm_validate_buffer));
> > + bo_handles = kvmalloc_array(exbuf->num_bo_handles,
> > + sizeof(uint32_t), GFP_KERNEL);
> > + buflist = kvmalloc_array(exbuf->num_bo_handles,
> > + sizeof(struct ttm_validate_buffer),
> > + GFP_KERNEL | __GFP_ZERO);
> > if (!bo_handles || !buflist) {
> > - drm_free_large(bo_handles);
> > - drm_free_large(buflist);
> > + kvfree(bo_handles);
> > + kvfree(buflist);
> > return -ENOMEM;
> > }
> > @@ -133,16 +134,16 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
> > if (copy_from_user(bo_handles, user_bo_handles,
> > exbuf->num_bo_handles * sizeof(uint32_t))) {
> > ret = -EFAULT;
> > - drm_free_large(bo_handles);
> > - drm_free_large(buflist);
> > + kvfree(bo_handles);
> > + kvfree(buflist);
> > return ret;
> > }
> > for (i = 0; i < exbuf->num_bo_handles; i++) {
> > gobj = drm_gem_object_lookup(drm_file, bo_handles[i]);
> > if (!gobj) {
> > - drm_free_large(bo_handles);
> > - drm_free_large(buflist);
> > + kvfree(bo_handles);
> > + kvfree(buflist);
> > return -ENOENT;
> > }
> > @@ -151,7 +152,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
> > list_add(&buflist[i].head, &validate_list);
> > }
> > - drm_free_large(bo_handles);
> > + kvfree(bo_handles);
> > }
> > ret = virtio_gpu_object_list_validate(&ticket, &validate_list);
> > @@ -171,7 +172,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
> > /* fence the command bo */
> > virtio_gpu_unref_list(&validate_list);
> > - drm_free_large(buflist);
> > + kvfree(buflist);
> > dma_fence_put(&fence->f);
> > return 0;
> > @@ -179,7 +180,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
> > ttm_eu_backoff_reservation(&ticket, &validate_list);
> > out_free:
> > virtio_gpu_unref_list(&validate_list);
> > - drm_free_large(buflist);
> > + kvfree(buflist);
> > return ret;
> > }
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index e1daa4f343cd..59df08d14b89 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -70,7 +70,6 @@
> > #include <drm/drm_fourcc.h>
> > #include <drm/drm_global.h>
> > #include <drm/drm_hashtab.h>
> > -#include <drm/drm_mem_util.h>
> > #include <drm/drm_mm.h>
> > #include <drm/drm_os_linux.h>
> > #include <drm/drm_sarea.h>
> > diff --git a/include/drm/drm_mem_util.h b/include/drm/drm_mem_util.h
> > deleted file mode 100644
> > index a1ddf55fda67..000000000000
> > --- a/include/drm/drm_mem_util.h
> > +++ /dev/null
> > @@ -1,53 +0,0 @@
> > -/*
> > - * Copyright (C) 2008 Intel Corporation
> > - *
> > - * Permission is hereby granted, free of charge, to any person obtaining a
> > - * copy of this software and associated documentation files (the "Software"),
> > - * to deal in the Software without restriction, including without limitation
> > - * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > - * and/or sell copies of the Software, and to permit persons to whom the
> > - * Software is furnished to do so, subject to the following conditions:
> > - *
> > - * The above copyright notice and this permission notice (including the next
> > - * paragraph) shall be included in all copies or substantial portions of the
> > - * Software.
> > - *
> > - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> > - * IN THE SOFTWARE.
> > - *
> > - * Authors:
> > - * Jesse Barnes <jbarnes@virtuousgeek.org>
> > - *
> > - */
> > -#ifndef _DRM_MEM_UTIL_H_
> > -#define _DRM_MEM_UTIL_H_
> > -
> > -#include <linux/vmalloc.h>
> > -
> > -static __inline__ void *drm_calloc_large(size_t nmemb, size_t size)
> > -{
> > - return kvmalloc_array(nmemb, size, GFP_KERNEL | __GFP_ZERO);
> > -}
> > -
> > -/* Modeled after cairo's malloc_ab, it's like calloc but without the zeroing. */
> > -static __inline__ void *drm_malloc_ab(size_t nmemb, size_t size)
> > -{
> > - return kvmalloc_array(nmemb, size, GFP_KERNEL);
> > -}
> > -
> > -static __inline__ void *drm_malloc_gfp(size_t nmemb, size_t size, gfp_t gfp)
> > -{
> > - return kvmalloc_array(nmemb, size, gfp);
> > -}
> > -
> > -static __inline void drm_free_large(void *ptr)
> > -{
> > - kvfree(ptr);
> > -}
> > -
> > -#endif
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCHv5, REBASED 9/9] x86/mm: Allow to have userspace mappings above 47-bits
From: Kirill A. Shutemov @ 2017-05-18 15:19 UTC (permalink / raw)
To: Michal Hocko
Cc: Kirill A. Shutemov, x86, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Andi Kleen, Dave Hansen, Andy Lutomirski,
Dan Williams, linux-mm, linux-kernel, linux-api
In-Reply-To: <20170518114359.GB25471@dhcp22.suse.cz>
On Thu, May 18, 2017 at 01:43:59PM +0200, Michal Hocko wrote:
> On Mon 15-05-17 15:12:18, Kirill A. Shutemov wrote:
> [...]
> > @@ -195,6 +207,16 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
> > info.length = len;
> > info.low_limit = PAGE_SIZE;
> > info.high_limit = get_mmap_base(0);
> > +
> > + /*
> > + * If hint address is above DEFAULT_MAP_WINDOW, look for unmapped area
> > + * in the full address space.
> > + *
> > + * !in_compat_syscall() check to avoid high addresses for x32.
> > + */
> > + if (addr > DEFAULT_MAP_WINDOW && !in_compat_syscall())
> > + info.high_limit += TASK_SIZE_MAX - DEFAULT_MAP_WINDOW;
> > +
> > info.align_mask = 0;
> > info.align_offset = pgoff << PAGE_SHIFT;
> > if (filp) {
>
> I have two questions/concerns here. The above assumes that any address above
> 1<<47 will use the _whole_ address space. Is this what we want?
Yes, I believe so.
> What if somebody does mmap(1<<52, ...) because he wants to (ab)use 53+
> bits for some other purpose? Shouldn't we cap the high_limit by the
> given address?
This would screw existing semantics of hint address -- "map here if
free, please".
> Another thing would be that
> /* requesting a specific address */
> if (addr) {
> addr = PAGE_ALIGN(addr);
> vma = find_vma(mm, addr);
> if (TASK_SIZE - len >= addr &&
> (!vma || addr + len <= vma->vm_start))
> return addr;
> }
>
> would fail for mmap(-1UL, ...) which is good because we do want to
> fallback to vm_unmapped_area and have randomized address which is
> ensured by your info.high_limit += ... but that wouldn't work for
> mmap(1<<N, ...) where N>47. So the first such mapping won't be
> randomized while others will be. This is quite unexpected I would say.
> So it should be documented at least or maybe we want to skip the above
> shortcut for addr > DEFAULT_MAP_WINDOW altogether.
Again, you're missing existing semantics of hint address. You may have a
reason to set hint address above 47-bit, besides getting access to full
address space.
--
Kirill A. Shutemov
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] mm,oom: fix oom invocation issues
From: Michal Hocko @ 2017-05-18 15:07 UTC (permalink / raw)
To: Tetsuo Handa
Cc: guro, hannes, vdavydov.dev, kernel-team, linux-mm, linux-kernel
In-Reply-To: <201705182357.GJH90607.FVHMQOJtOLFFOS@I-love.SAKURA.ne.jp>
On Thu 18-05-17 23:57:23, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > On Thu 18-05-17 22:57:10, Tetsuo Handa wrote:
> > > Michal Hocko wrote:
> > > > It is racy and it basically doesn't have any allocation context so we
> > > > might kill a task from a different domain. So can we do this instead?
> > > > There is a slight risk that somebody might have returned VM_FAULT_OOM
> > > > without doing an allocation but from my quick look nobody does that
> > > > currently.
> > >
> > > I can't tell whether it is safe to remove out_of_memory() from
> > > pagefault_out_of_memory(). There are VM_FAULT_OOM users in fs/
> > > directory. What happens if pagefault_out_of_memory() was called as a
> > > result of e.g. GFP_NOFS allocation failure?
> >
> > Then we would bypass GFP_NOFS oom protection and could trigger a
> > premature OOM killer invocation.
>
> Excuse me, but I couldn't understand your answer.
>
> We have __GFP_FS check in out_of_memory(). If we remove out_of_memory() from
> pagefault_out_of_memory(), pagefault_out_of_memory() called as a result of
> a !__GFP_FS allocation failure won't be able to call oom_kill_process().
> Unless somebody else calls oom_kill_process() via a __GFP_FS allocation
> request, a thread which triggered a page fault event will spin forever.
which is basically the same as looping inside the allocator. Except that
we unwind the full falt path and drop all the locks which is an
advantage.
> > > Is it guaranteed that all memory allocations that might occur from
> > > page fault event (or any action that might return VM_FAULT_OOM)
> > > are allowed to call oom_kill_process() from out_of_memory() before
> > > reaching pagefault_out_of_memory() ?
> >
> > The same applies here.
>
> So, my question is, can pagefault_out_of_memory() be called as a result of
> an allocation request (or action) which cannot call oom_kill_process() ?
> Please answer with "yes" or "no".
I haven't checked all of them but considering that we do not fail small
allocations I would consider that unlikely.
> > > Anyway, I want
> > >
> > > /* Avoid allocations with no watermarks from looping endlessly */
> > > - if (test_thread_flag(TIF_MEMDIE))
> > > + if (alloc_flags == ALLOC_NO_WATERMARKS && test_thread_flag(TIF_MEMDIE))
> > > goto nopage;
> > >
> > > so that we won't see similar backtraces and memory information from both
> > > out_of_memory() and warn_alloc().
> >
> > I do not think this is an improvement and it is unrelated to the
> > discussion here.
>
> If we allow current thread to allocate memory when current thread was
> chosen as an OOM victim by giving current thread a chance to do
> ALLOC_NO_WATERMARKS allocation request, all memory allocation requests
> that might occur from page fault event will likely succeed and thus
> current thread will not call pagefault_out_of_memory(). This will
> prevent current thread from selecting next OOM victim by calling
> out_of_memory() from pagefault_out_of_memory().
yes, I have realized that later and responded already.
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] mm,oom: fix oom invocation issues
From: Michal Hocko @ 2017-05-18 15:01 UTC (permalink / raw)
To: Tetsuo Handa
Cc: guro, hannes, vdavydov.dev, kernel-team, linux-mm, linux-kernel
In-Reply-To: <20170518142901.GA13940@dhcp22.suse.cz>
On Thu 18-05-17 16:29:01, Michal Hocko wrote:
> On Thu 18-05-17 22:57:10, Tetsuo Handa wrote:
[...]
> > Anyway, I want
> >
> > /* Avoid allocations with no watermarks from looping endlessly */
> > - if (test_thread_flag(TIF_MEMDIE))
> > + if (alloc_flags == ALLOC_NO_WATERMARKS && test_thread_flag(TIF_MEMDIE))
> > goto nopage;
> >
> > so that we won't see similar backtraces and memory information from both
> > out_of_memory() and warn_alloc().
>
> I do not think this is an improvement and it is unrelated to the
> discussion here.
I am sorry, I've misread the diff. It was the comment below the diff
which confused me. Now that I looked at it again it actually makes sense.
I would still like to get rid of out_of_memory from pagefault_out_of_memory
but doing the above sounds safer for the stable backport. Care to create
a proper patch with the full changelog, please?
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] mm,oom: fix oom invocation issues
From: Tetsuo Handa @ 2017-05-18 14:57 UTC (permalink / raw)
To: mhocko; +Cc: guro, hannes, vdavydov.dev, kernel-team, linux-mm, linux-kernel
In-Reply-To: <20170518142901.GA13940@dhcp22.suse.cz>
Michal Hocko wrote:
> On Thu 18-05-17 22:57:10, Tetsuo Handa wrote:
> > Michal Hocko wrote:
> > > It is racy and it basically doesn't have any allocation context so we
> > > might kill a task from a different domain. So can we do this instead?
> > > There is a slight risk that somebody might have returned VM_FAULT_OOM
> > > without doing an allocation but from my quick look nobody does that
> > > currently.
> >
> > I can't tell whether it is safe to remove out_of_memory() from
> > pagefault_out_of_memory(). There are VM_FAULT_OOM users in fs/
> > directory. What happens if pagefault_out_of_memory() was called as a
> > result of e.g. GFP_NOFS allocation failure?
>
> Then we would bypass GFP_NOFS oom protection and could trigger a
> premature OOM killer invocation.
Excuse me, but I couldn't understand your answer.
We have __GFP_FS check in out_of_memory(). If we remove out_of_memory() from
pagefault_out_of_memory(), pagefault_out_of_memory() called as a result of
a !__GFP_FS allocation failure won't be able to call oom_kill_process().
Unless somebody else calls oom_kill_process() via a __GFP_FS allocation
request, a thread which triggered a page fault event will spin forever.
>
> > Is it guaranteed that all memory allocations that might occur from
> > page fault event (or any action that might return VM_FAULT_OOM)
> > are allowed to call oom_kill_process() from out_of_memory() before
> > reaching pagefault_out_of_memory() ?
>
> The same applies here.
So, my question is, can pagefault_out_of_memory() be called as a result of
an allocation request (or action) which cannot call oom_kill_process() ?
Please answer with "yes" or "no".
>
> > Anyway, I want
> >
> > /* Avoid allocations with no watermarks from looping endlessly */
> > - if (test_thread_flag(TIF_MEMDIE))
> > + if (alloc_flags == ALLOC_NO_WATERMARKS && test_thread_flag(TIF_MEMDIE))
> > goto nopage;
> >
> > so that we won't see similar backtraces and memory information from both
> > out_of_memory() and warn_alloc().
>
> I do not think this is an improvement and it is unrelated to the
> discussion here.
If we allow current thread to allocate memory when current thread was
chosen as an OOM victim by giving current thread a chance to do
ALLOC_NO_WATERMARKS allocation request, all memory allocation requests
that might occur from page fault event will likely succeed and thus
current thread will not call pagefault_out_of_memory(). This will
prevent current thread from selecting next OOM victim by calling
out_of_memory() from pagefault_out_of_memory().
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] mm,oom: fix oom invocation issues
From: Michal Hocko @ 2017-05-18 14:29 UTC (permalink / raw)
To: Tetsuo Handa
Cc: guro, hannes, vdavydov.dev, kernel-team, linux-mm, linux-kernel
In-Reply-To: <201705182257.HJJ52185.OQStFLFMHVOJOF@I-love.SAKURA.ne.jp>
On Thu 18-05-17 22:57:10, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > It is racy and it basically doesn't have any allocation context so we
> > might kill a task from a different domain. So can we do this instead?
> > There is a slight risk that somebody might have returned VM_FAULT_OOM
> > without doing an allocation but from my quick look nobody does that
> > currently.
>
> I can't tell whether it is safe to remove out_of_memory() from
> pagefault_out_of_memory(). There are VM_FAULT_OOM users in fs/
> directory. What happens if pagefault_out_of_memory() was called as a
> result of e.g. GFP_NOFS allocation failure?
Then we would bypass GFP_NOFS oom protection and could trigger a
premature OOM killer invocation.
> Is it guaranteed that all memory allocations that might occur from
> page fault event (or any action that might return VM_FAULT_OOM)
> are allowed to call oom_kill_process() from out_of_memory() before
> reaching pagefault_out_of_memory() ?
The same applies here.
> Anyway, I want
>
> /* Avoid allocations with no watermarks from looping endlessly */
> - if (test_thread_flag(TIF_MEMDIE))
> + if (alloc_flags == ALLOC_NO_WATERMARKS && test_thread_flag(TIF_MEMDIE))
> goto nopage;
>
> so that we won't see similar backtraces and memory information from both
> out_of_memory() and warn_alloc().
I do not think this is an improvement and it is unrelated to the
discussion here.
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] mm: clarify why we want kmalloc before falling backto vmallock
From: Vlastimil Babka @ 2017-05-18 14:08 UTC (permalink / raw)
To: Michal Hocko, Andrew Morton; +Cc: Chris Wilson, linux-mm, LKML, Michal Hocko
In-Reply-To: <20170517080932.21423-1-mhocko@kernel.org>
On 05/17/2017 10:09 AM, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
>
> While converting drm_[cm]alloc* helpers to kvmalloc* variants Chris
> Wilson has wondered why we want to try kmalloc before vmalloc fallback
> even for larger allocations requests. Let's clarify that one larger
> physically contiguous block is less likely to fragment memory than many
> scattered pages which can prevent more large blocks from being created.
>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
> ---
> mm/util.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/mm/util.c b/mm/util.c
> index 464df3489903..87499f8119f2 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -357,7 +357,10 @@ void *kvmalloc_node(size_t size, gfp_t flags, int node)
> WARN_ON_ONCE((flags & GFP_KERNEL) != GFP_KERNEL);
>
> /*
> - * Make sure that larger requests are not too disruptive - no OOM
> + * We want to attempt a large physically contiguous block first because
> + * it is less likely to fragment multiple larger blocks and therefore
> + * contribute to a long term fragmentation less than vmalloc fallback.
> + * However make sure that larger requests are not too disruptive - no OOM
> * killer and no allocation failure warnings as we have a fallback
> */
> if (size > PAGE_SIZE) {
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox