* Re: [RFC PATCH v2 12/17] cgroup: Remove cgroup v2 no internal process constraint
From: Mike Galbraith @ 2017-05-20 2:10 UTC (permalink / raw)
To: Tejun Heo, Waiman Long
Cc: Li Zefan, Johannes Weiner, Peter Zijlstra, Ingo Molnar, cgroups,
linux-kernel, linux-doc, linux-mm, kernel-team, pjt, luto
In-Reply-To: <20170519203824.GC15279@wtj.duckdns.org>
On Fri, 2017-05-19 at 16:38 -0400, Tejun Heo wrote:
> Hello, Waiman.
>
> On Mon, May 15, 2017 at 09:34:11AM -0400, Waiman Long wrote:
> > The rationale behind the cgroup v2 no internal process constraint is
> > to avoid resouorce competition between internal processes and child
> > cgroups. However, not all controllers have problem with internal
> > process competiton. Enforcing this rule may lead to unnatural process
> > hierarchy and unneeded levels for those controllers.
>
> This isn't necessarily something we can determine by looking at the
> current state of controllers. It's true that some controllers - pid
> and perf - inherently only care about membership of each task but at
> the same time neither really suffers from the constraint either. CPU
> which is the problematic one here...
(+ cpuacct + cpuset)
--
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: mm, something wring in page_lock_anon_vma_read()?
From: Hugh Dickins @ 2017-05-20 2:02 UTC (permalink / raw)
To: Xishi Qiu
Cc: Hugh Dickins, Andrew Morton, Tejun Heo, Michal Hocko,
Johannes Weiner, Mel Gorman, Michal Hocko, Vlastimil Babka,
Minchan Kim, David Rientjes, Joonsoo Kim, aarcange,
sumeet.keswani, Rik van Riel, Linux MM, LKML, zhong jiang
In-Reply-To: <591F9A09.6010707@huawei.com>
On Sat, 20 May 2017, Xishi Qiu wrote:
> On 2017/5/20 6:00, Hugh Dickins wrote:
> >
> > You're ignoring the rcu_read_lock() on entry to page_lock_anon_vma_read(),
> > and the SLAB_DESTROY_BY_RCU (recently renamed SLAB_TYPESAFE_BY_RCU) nature
> > of the anon_vma_cachep kmem cache. It is not safe to muck with anon_vma->
> > root in anon_vma_free(), others could still be looking at it.
> >
> > Hugh
> >
>
> Hi Hugh,
>
> Thanks for your reply.
>
> SLAB_DESTROY_BY_RCU will let it call call_rcu() in free_slab(), but if the
> anon_vma *reuse* by someone again, access root_anon_vma is not safe, right?
That is safe, on reuse it is still a struct anon_vma; then the test for
!page_mapped(page) will show that it's no longer a reliable anon_vma for
this page, so page_lock_anon_vma_read() returns NULL.
But of course, if page->_mapcount has been corrupted or misaccounted,
it may think page_mapped(page) when actually page is not mapped,
and the anon_vma is not good for it.
>
> e.g. if I clean the root pointer before free it, then access root_anon_vma
> in page_lock_anon_vma_read() is NULL pointer access, right?
Yes, cleaning root pointer before free may result in NULL pointer access.
Hugh
>
> anon_vma_free()
> ...
> anon_vma->root = NULL;
> kmem_cache_free(anon_vma_cachep, anon_vma);
> ...
>
> Thanks,
> Xishi Qiu
--
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: mm, something wring in page_lock_anon_vma_read()?
From: Xishi Qiu @ 2017-05-20 1:21 UTC (permalink / raw)
To: Hugh Dickins
Cc: Andrew Morton, Tejun Heo, Michal Hocko, Johannes Weiner,
Mel Gorman, Michal Hocko, Vlastimil Babka, Minchan Kim,
David Rientjes, Joonsoo Kim, aarcange, sumeet.keswani,
Rik van Riel, Linux MM, LKML, zhong jiang
In-Reply-To: <alpine.LSU.2.11.1705191453040.3819@eggly.anvils>
On 2017/5/20 6:00, Hugh Dickins wrote:
> On Fri, 19 May 2017, Xishi Qiu wrote:
>> On 2017/5/19 16:52, Xishi Qiu wrote:
>>> On 2017/5/18 17:46, Xishi Qiu wrote:
>>>
>>>> Hi, my system triggers this bug, and the vmcore shows the anon_vma seems be freed.
>>>> The kernel is RHEL 7.2, and the bug is hard to reproduce, so I don't know if it
>>>> exists in mainline, any reply is welcome!
>>>>
>>>
>>> When we alloc anon_vma, we will init the value of anon_vma->root,
>>> so can we set anon_vma->root to NULL when calling
>>> anon_vma_free -> kmem_cache_free(anon_vma_cachep, anon_vma);
>>>
>>> anon_vma_free()
>>> ...
>>> anon_vma->root = NULL;
>>> kmem_cache_free(anon_vma_cachep, anon_vma);
>>>
>>> I find if we do this above, system boot failed, why?
>>>
>>
>> If anon_vma was freed, we should not to access the root_anon_vma, because it maybe also
>> freed(e.g. anon_vma == root_anon_vma), right?
>>
>> page_lock_anon_vma_read()
>> ...
>> anon_vma = (struct anon_vma *) (anon_mapping - PAGE_MAPPING_ANON);
>> root_anon_vma = ACCESS_ONCE(anon_vma->root);
>> if (down_read_trylock(&root_anon_vma->rwsem)) { // it's not safe
>> ...
>> if (!atomic_inc_not_zero(&anon_vma->refcount)) { // check anon_vma was not freed
>> ...
>> anon_vma_lock_read(anon_vma); // it's safe
>> ...
>
> You're ignoring the rcu_read_lock() on entry to page_lock_anon_vma_read(),
> and the SLAB_DESTROY_BY_RCU (recently renamed SLAB_TYPESAFE_BY_RCU) nature
> of the anon_vma_cachep kmem cache. It is not safe to muck with anon_vma->
> root in anon_vma_free(), others could still be looking at it.
>
> Hugh
>
Hi Hugh,
Thanks for your reply.
SLAB_DESTROY_BY_RCU will let it call call_rcu() in free_slab(), but if the
anon_vma *reuse* by someone again, access root_anon_vma is not safe, right?
e.g. if I clean the root pointer before free it, then access root_anon_vma
in page_lock_anon_vma_read() is NULL pointer access, right?
anon_vma_free()
...
anon_vma->root = NULL;
kmem_cache_free(anon_vma_cachep, anon_vma);
...
Thanks,
Xishi Qiu
> .
>
--
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: John Hubbard @ 2017-05-20 0:46 UTC (permalink / raw)
To: Michal Hocko, Andrew Morton
Cc: Chris Wilson, Vlastimil Babka, linux-mm, LKML, Michal Hocko
In-Reply-To: <20170517080932.21423-1-mhocko@kernel.org>
On 05/17/2017 01: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>
> ---
> 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
> */
Thanks for adding this, it's great to have. Here's a slightly polished version of your words, if you
like:
/*
* We want to attempt a large physically contiguous block first because
* it is less likely to fragment multiple larger blocks. This approach
* therefore contributes less to long term fragmentation than a vmalloc
* fallback would. However, make sure that larger requests are not too
* disruptive: no OOM killer and no allocation failure warnings, as we
* have a fallback.
*/
thanks,
john h
> if (size > PAGE_SIZE) {
> --
> 2.11.0
>
> --
> 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>
>
--
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] x86/mm: synchronize pgd in vmemmap_free()
From: John Hubbard @ 2017-05-20 0:34 UTC (permalink / raw)
To: Jérôme Glisse, linux-kernel, linux-mm
Cc: Kirill A. Shutemov, Andrew Morton, Ingo Molnar, Michal Hocko,
Mel Gorman
In-Reply-To: <1495216887-3175-2-git-send-email-jglisse@redhat.com>
Hi Jerome,
On 05/19/2017 11:01 AM, Jérôme Glisse wrote:
> When we free kernel virtual map we should synchronize p4d/pud for
> all the pgds to avoid any stall entry in non canonical pgd.
"any stale entry in the non-canonical pgd", is what I think you meant to type there.
Also, it would be nice to clarify that commit description a bit: I'm not sure what is meant here by
a "non-canonical pgd".
Also, it seems like the reshuffling of the internals of sync_global_pgds() deserves at least some
mention here. More below.
>
> Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Mel Gorman <mgorman@suse.de>
> ---
> arch/x86/mm/init_64.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index ff95fe8..df753f8 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -108,8 +108,6 @@ void sync_global_pgds(unsigned long start, unsigned long end)
> BUILD_BUG_ON(pgd_none(*pgd_ref));
> p4d_ref = p4d_offset(pgd_ref, address);
>
> - if (p4d_none(*p4d_ref))
> - continue;
>
> spin_lock(&pgd_lock);
> list_for_each_entry(page, &pgd_list, lru) {
> @@ -123,12 +121,16 @@ void sync_global_pgds(unsigned long start, unsigned long end)
> pgt_lock = &pgd_page_get_mm(page)->page_table_lock;
> spin_lock(pgt_lock);
>
> - if (!p4d_none(*p4d_ref) && !p4d_none(*p4d))
> - BUG_ON(p4d_page_vaddr(*p4d)
> - != p4d_page_vaddr(*p4d_ref));
> -
> - if (p4d_none(*p4d))
> + if (p4d_none(*p4d_ref)) {
> set_p4d(p4d, *p4d_ref);
Is the intention really to set p4d to a zeroed *p4d_ref, or is that a mistake?
> + } else {
> + if (!p4d_none(*p4d_ref) && !p4d_none(*p4d))
I think the code needs to be somewhat restructured, but as it stands, the above !p4d_none(*p4d_ref)
will always be true, because first part of the if/else checked for the opposite case:
p4d_none(*p4d_ref). This is a side effect of moving that block of code.
> + BUG_ON(p4d_page_vaddr(*p4d)
> + != p4d_page_vaddr(*p4d_ref));
> +
> + if (p4d_none(*p4d))
> + set_p4d(p4d, *p4d_ref);
> + }
>
> spin_unlock(pgt_lock);
> }
> @@ -1024,6 +1026,7 @@ remove_pagetable(unsigned long start, unsigned long end, bool direct)
> void __ref vmemmap_free(unsigned long start, unsigned long end)
> {
> remove_pagetable(start, end, false);
> + sync_global_pgds(start, end - 1);
This does fix the HMM crash that I was seeing in hmm-next.
thanks,
John Hubbard
NVIDIA
> }
>
> #ifdef CONFIG_MEMORY_HOTREMOVE
> --
> 2.4.11
>
> --
> 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>
>
--
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 2/2] mm, oom: do not trigger out_of_memory from the #PF
From: Tetsuo Handa @ 2017-05-19 23:43 UTC (permalink / raw)
To: mhocko; +Cc: akpm, hannes, guro, vdavydov.dev, linux-mm, linux-kernel
In-Reply-To: <20170519155057.GM29839@dhcp22.suse.cz>
Michal Hocko wrote:
> On Sat 20-05-17 00:22:30, Tetsuo Handa wrote:
> > Michal Hocko wrote:
> > > On Fri 19-05-17 22:02:44, Tetsuo Handa wrote:
> > > > Michal Hocko wrote:
> > > > > Any allocation failure during the #PF path will return with VM_FAULT_OOM
> > > > > which in turn results in pagefault_out_of_memory. This can happen for
> > > > > 2 different reasons. a) Memcg is out of memory and we rely on
> > > > > mem_cgroup_oom_synchronize to perform the memcg OOM handling or b)
> > > > > normal allocation fails.
> > > > >
> > > > > The later is quite problematic because allocation paths already trigger
> > > > > out_of_memory and the page allocator tries really hard to not fail
> > > >
> > > > We made many memory allocation requests from page fault path (e.g. XFS)
> > > > __GFP_FS some time ago, didn't we? But if I recall correctly (I couldn't
> > > > find the message), there are some allocation requests from page fault path
> > > > which cannot use __GFP_FS. Then, not all allocation requests can call
> > > > oom_kill_process() and reaching pagefault_out_of_memory() will be
> > > > inevitable.
> > >
> > > Even if such an allocation fail without the OOM killer then we simply
> > > retry the PF and will do that the same way how we keep retrying the
> > > allocation inside the page allocator. So how is this any different?
> >
> > You are trying to remove out_of_memory() from pagefault_out_of_memory()
> > by this patch. But you also want to make !__GFP_FS allocations not to
> > keep retrying inside the page allocator in future kernels, don't you?
>
> I would _love_ to but I am much less optimistic this is achiveable
>
> > Then, a thread which need to allocate memory from page fault path but
> > cannot call oom_kill_process() will spin forever (unless somebody else
> > calls oom_kill_process() via a __GFP_FS allocation request). I consider
> > that introducing such possibility is a problem.
>
> What I am trying to say is that this is already happening. The
> difference with the VM_FAULT_OOM would only be that the whole PF path
> would be unwinded back to the PF, all locks dropped and then the PF
> retries so in principle this would be safer.
I can't understand why the PF retries helps. If the PF has to be retried due to
failing to allocate memory, situation will not improve until memory is allocated.
And I don't like an assumption that somebody else calls oom_kill_process() via
a __GFP_FS allocation request so that the PF will succeed without calling
oom_kill_process().
>
> > > > > allocations. Anyway, if the OOM killer has been already invoked there
> > > > > is no reason to invoke it again from the #PF path. Especially when the
> > > > > OOM condition might be gone by that time and we have no way to find out
> > > > > other than allocate.
> > > > >
> > > > > Moreover if the allocation failed and the OOM killer hasn't been
> > > > > invoked then we are unlikely to do the right thing from the #PF context
> > > > > because we have already lost the allocation context and restictions and
> > > > > therefore might oom kill a task from a different NUMA domain.
> > > >
> > > > If we carry a flag via task_struct that indicates whether it is an memory
> > > > allocation request from page fault and allocation failure is not acceptable,
> > > > we can call out_of_memory() from page allocator path.
> > >
> > > I do not understand
> >
> > We need to allocate memory from page fault path in order to avoid spinning forever
> > (unless somebody else calls oom_kill_process() via a __GFP_FS allocation request),
> > doesn't it? Then, memory allocation requests from page fault path can pass flags
> > like __GFP_NOFAIL | __GFP_KILLABLE because retrying the page fault without
> > allocating memory is pointless. I called such flags as carry a flag via task_struct.
> >
> > > > By the way, can page fault occur after reaching do_exit()? When a thread
> > > > reached do_exit(), fatal_signal_pending(current) becomes false, doesn't it?
> > >
> > > yes fatal_signal_pending will be false at the time and I believe we can
> > > perform a page fault past that moment and go via allocation path which would
> > > trigger the OOM or give this task access to reserves but it is more
> > > likely that the oom reaper will push to kill another task by that time
> > > if the situation didn't get resolved. Or did I miss your concern?
> >
> > How checking fatal_signal_pending() here helps?
>
> It just skips the warning because we know that we would handle the
> signal before retrying the page fault and go to exit path. Those that do
> not have such a signal should warn just that we know that such a
> situation happens. With the current allocator semantic it shouldn't
>
> > It only suppresses printk().
> > If current thread needs to allocate memory because not all allocation requests
> > can call oom_kill_process(), doing printk() is not the right thing to do.
> > Allocate memory by some means (e.g. __GFP_NOFAIL | __GFP_KILLABLE) will be
> > the right thing to do.
>
> Why would looping inside an allocator with a restricted context be any
> better than retrying the whole thing?
I'm not suggesting you to loop inside an allocator nor retry the whole thing.
I'm suggesting you to avoid returning VM_FAULT_OOM by making allocations succeed
(by e.g. calling oom_kill_process()) regardless of restricted context if you
want to remove out_of_memory() from pagefault_out_of_memory(), for situation
will not improve until memory is allocated (e.g. somebody else calls
oom_kill_process() via a __GFP_FS allocation request).
--
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] dm ioctl: Restore __GFP_HIGH in copy_params()
From: Mikulas Patocka @ 2017-05-19 23:43 UTC (permalink / raw)
To: Michal Hocko
Cc: Junaid Shahid, David Rientjes, Alasdair Kergon, Mike Snitzer,
Andrew Morton, linux-mm, andreslc, gthelen, vbabka, linux-kernel
In-Reply-To: <20170519074647.GC13041@dhcp22.suse.cz>
On Fri, 19 May 2017, Michal Hocko wrote:
> On Thu 18-05-17 19:50:46, Junaid Shahid wrote:
> > (Adding back the correct linux-mm email address and also adding linux-kernel.)
> >
> > On Thursday, May 18, 2017 01:41:33 PM David Rientjes wrote:
> [...]
> > > Let's ask Mikulas, who changed this from PF_MEMALLOC to __GFP_HIGH,
> > > assuming there was a reason to do it in the first place in two different
> > > ways.
>
> Hmm, the old PF_MEMALLOC used to have the following comment
> /*
> * Trying to avoid low memory issues when a device is
> * suspended.
> */
>
> I am not really sure what that means but __GFP_HIGH certainly have a
> different semantic than PF_MEMALLOC. The later grants the full access to
> the memory reserves while the prior on partial access. If this is _really_
> needed then it deserves a comment explaining why.
> --
> Michal Hocko
> SUSE Labs
Sometimes, I/O to a device mapper device is blocked until the userspace
daemon dmeventd does some action (for example, when dm-mirror leg fails,
dmeventd needs to mark the leg as failed in the lvm metadata and then
reload the device).
The dmeventd daemon mlocks itself in memory so that it doesn't generate
any I/O. But it must be able to call ioctls. __GFP_HIGH is there so that
the ioctls issued by dmeventd have higher chance of succeeding if some I/O
is blocked, waiting for dmeventd action. It reduces the possibility of
low-memory-deadlock, though it doesn't eliminate it entirely.
Mikulas
--
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: mm, something wring in page_lock_anon_vma_read()?
From: Hugh Dickins @ 2017-05-19 22:00 UTC (permalink / raw)
To: Xishi Qiu
Cc: Andrew Morton, Tejun Heo, Michal Hocko, Johannes Weiner,
Mel Gorman, Michal Hocko, Vlastimil Babka, Minchan Kim,
David Rientjes, Joonsoo Kim, aarcange, sumeet.keswani,
Rik van Riel, Hugh Dickins, Linux MM, LKML, zhong jiang
In-Reply-To: <591EBE71.7080402@huawei.com>
On Fri, 19 May 2017, Xishi Qiu wrote:
> On 2017/5/19 16:52, Xishi Qiu wrote:
> > On 2017/5/18 17:46, Xishi Qiu wrote:
> >
> >> Hi, my system triggers this bug, and the vmcore shows the anon_vma seems be freed.
> >> The kernel is RHEL 7.2, and the bug is hard to reproduce, so I don't know if it
> >> exists in mainline, any reply is welcome!
> >>
> >
> > When we alloc anon_vma, we will init the value of anon_vma->root,
> > so can we set anon_vma->root to NULL when calling
> > anon_vma_free -> kmem_cache_free(anon_vma_cachep, anon_vma);
> >
> > anon_vma_free()
> > ...
> > anon_vma->root = NULL;
> > kmem_cache_free(anon_vma_cachep, anon_vma);
> >
> > I find if we do this above, system boot failed, why?
> >
>
> If anon_vma was freed, we should not to access the root_anon_vma, because it maybe also
> freed(e.g. anon_vma == root_anon_vma), right?
>
> page_lock_anon_vma_read()
> ...
> anon_vma = (struct anon_vma *) (anon_mapping - PAGE_MAPPING_ANON);
> root_anon_vma = ACCESS_ONCE(anon_vma->root);
> if (down_read_trylock(&root_anon_vma->rwsem)) { // it's not safe
> ...
> if (!atomic_inc_not_zero(&anon_vma->refcount)) { // check anon_vma was not freed
> ...
> anon_vma_lock_read(anon_vma); // it's safe
> ...
You're ignoring the rcu_read_lock() on entry to page_lock_anon_vma_read(),
and the SLAB_DESTROY_BY_RCU (recently renamed SLAB_TYPESAFE_BY_RCU) nature
of the anon_vma_cachep kmem cache. It is not safe to muck with anon_vma->
root in anon_vma_free(), others could still be looking at it.
Hugh
--
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 02/11] mm: mempolicy: add queue_pages_node_check()
From: Mel Gorman @ 2017-05-19 21:39 UTC (permalink / raw)
To: Zi Yan
Cc: Anshuman Khandual, n-horiguchi, kirill.shutemov, linux-kernel,
linux-mm, akpm, minchan, vbabka, mhocko, dnellans
In-Reply-To: <61FCE04A-0227-4D5E-92E5-81EA06979FD3@cs.rutgers.edu>
On Fri, May 19, 2017 at 04:48:54PM -0400, Zi Yan wrote:
> On 19 May 2017, at 16:28, Mel Gorman wrote:
>
> > On Fri, May 19, 2017 at 12:37:38PM -0400, Zi Yan wrote:
> >>> As you say, there is no functional change but the helper name is vague
> >>> and gives no hint to what's it's checking for. It's somewhat tolerable as
> >>> it is as it's obvious what is being checked but the same is not true with
> >>> the helper name.
> >>>
> >>
> >> Does queue_pages_invert_nodemask_check() work? I can change the helper name
> >> in the next version.
> >>
> >
> > Not particularly, maybe queue_pages_required and invert the check with a
> > comment above it explaining what it's checking for would be ok.
> >
>
> queue_pages_required() is too broad,
I'm somewhat amused that you'd complain that "required" is too broad while
thinking "check" is somehow self-explanatory.
> I would take queue_pages_page_nid_check()
> and invert the check with a comment above saying
>
> /*
> * Check if the page's nid is in qp->nmask.
> *
> * If MPOL_MF_INVERT is set in qp->flags, check if the nid is
> * in the invert of qp->nmask.
> */
>
> Does it work?
>
I still don't like the name but I also am not interested in debating it
further for something so small. Add the comment, it's better than nothing.
--
Mel Gorman
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 v5 28/32] x86/mm, kexec: Allow kexec to be used with SME
From: Tom Lendacky @ 2017-05-19 21:38 UTC (permalink / raw)
To: Borislav Petkov
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: <20170519212854.3dvtt3lawzij5cae@pd.tnic>
On 5/19/2017 4:28 PM, Borislav Petkov wrote:
> On Fri, May 19, 2017 at 04:07:24PM -0500, Tom Lendacky wrote:
>> As long as those never change from static inline everything will be
>> fine. I can change it, but I really like how it explicitly indicates
>
> I know what you want to do. But you're practically defining a helper
> which contains two arbitrary instructions which probably no one else
> will need.
>
> So how about we simplify this function even more. We don't need to pay
> attention to kexec being in progress because we're halting anyway so who
> cares how fast we halt.
>
> Might have to state that in the comment below though, instead of what's
> there now.
>
> And for the exact same moot reason, we don't need to look at SME CPUID
> feature - we can just as well WBINVD unconditionally.
>
> void stop_this_cpu(void *dummy)
> {
> local_irq_disable();
> /*
> * Remove this CPU:
> */
> set_cpu_online(smp_processor_id(), false);
> disable_local_APIC();
> mcheck_cpu_clear(this_cpu_ptr(&cpu_info));
>
> for (;;) {
> /*
> * If we are performing a kexec and the processor supports
> * SME then we need to clear out cache information before
> * halting. With kexec, going from SME inactive to SME active
> * requires clearing cache entries so that addresses without
> * the encryption bit set don't corrupt the same physical
> * address that has the encryption bit set when caches are
> * flushed. Perform a wbinvd followed by a halt to achieve
> * this.
> */
> asm volatile("wbinvd; hlt" ::: "memory");
> }
> }
>
> How's that?
I can live with that!
Thanks,
Tom
>
--
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 28/32] x86/mm, kexec: Allow kexec to be used with SME
From: Borislav Petkov @ 2017-05-19 21:28 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: <5ef96f3a-6ebd-1d4d-7ac9-05dbed45d998@amd.com>
On Fri, May 19, 2017 at 04:07:24PM -0500, Tom Lendacky wrote:
> As long as those never change from static inline everything will be
> fine. I can change it, but I really like how it explicitly indicates
I know what you want to do. But you're practically defining a helper
which contains two arbitrary instructions which probably no one else
will need.
So how about we simplify this function even more. We don't need to pay
attention to kexec being in progress because we're halting anyway so who
cares how fast we halt.
Might have to state that in the comment below though, instead of what's
there now.
And for the exact same moot reason, we don't need to look at SME CPUID
feature - we can just as well WBINVD unconditionally.
void stop_this_cpu(void *dummy)
{
local_irq_disable();
/*
* Remove this CPU:
*/
set_cpu_online(smp_processor_id(), false);
disable_local_APIC();
mcheck_cpu_clear(this_cpu_ptr(&cpu_info));
for (;;) {
/*
* If we are performing a kexec and the processor supports
* SME then we need to clear out cache information before
* halting. With kexec, going from SME inactive to SME active
* requires clearing cache entries so that addresses without
* the encryption bit set don't corrupt the same physical
* address that has the encryption bit set when caches are
* flushed. Perform a wbinvd followed by a halt to achieve
* this.
*/
asm volatile("wbinvd; hlt" ::: "memory");
}
}
How's that?
--
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 PATCH v2 13/17] cgroup: Allow fine-grained controllers control in cgroup v2
From: Waiman Long @ 2017-05-19 21:20 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: <20170519205550.GD15279@wtj.duckdns.org>
On 05/19/2017 04:55 PM, Tejun Heo wrote:
> Hello, Waiman.
>
> On Mon, May 15, 2017 at 09:34:12AM -0400, Waiman Long wrote:
>> For cgroup v1, different controllers can be binded to different cgroup
>> hierarchies optimized for their own use cases. That is not currently
>> the case for cgroup v2 where combining all these controllers into
>> the same hierarchy will probably require more levels than is needed
>> by each individual controller.
>>
>> By not enabling a controller in a cgroup and its descendants, we can
>> effectively trim the hierarchy as seen by a controller from the leafs
>> up. However, there is currently no way to compress the hierarchy in
>> the intermediate levels.
>>
>> This patch implements a fine-grained mechanism to allow a controller to
>> skip some intermediate levels in a hierarchy and effectively flatten
>> the hierarchy as seen by that controller.
>>
>> Controllers can now be directly enabled or disabled in a cgroup
>> by writing to the "cgroup.controllers" file. The special prefix
>> '#' with the controller name is used to set that controller in
>> pass-through mode. In that mode, the controller is disabled for that
>> cgroup but it allows its children to have that controller enabled or
>> in pass-through mode again.
>>
>> With this change, each controller can now have a unique view of their
>> virtual process hierarchy that can be quite different from other
>> controllers. We now have the freedom and flexibility to create the
>> right hierarchy for each controller to suit their own needs without
>> performance loss when compared with cgroup v1.
> I can see the appeal but this needs at least more refinements.
>
> This breaks the invariant that in a cgroup its resource control knobs
> control distribution of resources from its parent. IOW, the resource
> control knobs of a cgroup always belong to the parent. This is also
> reflected in how delegation is done. The delegatee assumes ownership
> of the cgroup itself and the ability to manage sub-cgroups but doesn't
> get the ownership of the resource control knobs as otherwise the
> parent would lose control over how it distributes its resources.
One twist that I am thinking is to have a controller enabled by the
parent in subtree_control, but then allow the child to either disable it
or set it in pass-through mode by writing to controllers file. IOW, a
child cannot enable a controller without parent's permission. Once a
child has permission, it can do whatever it wants. A parent cannot force
a child to have a controller enabled.
> Another aspect is that most controllers aren't that sensitive to
> nesting several levels. Expensive operations can be and already are
> aggregated and the performance overhead of several levels of nesting
> barely shows up. Skipping levels can be an interesting optimization
> approach and we can definitely support from the core side; however,
> it'd be a lot nicer if we could do that optimization transparently
> (e.g. CPU can skip multi level queueing if there usually is only one
> item at some levels).
The trend that I am seeing is that the total number of controllers is
going to grow over time. New controllers may be sensitive to the level
of nesting like the cpu controller. I am also thinking about how systemd
is using the cgroup filesystem for task classification purpose without
any controller attached to it. With this scheme, we can accommodate all
the different needs without using different cgroup filesystems.
> Hmm... that said, if we can fix the delegation issue in a not-too-ugly
> way, why not? I wonder whether we can still keep the resource control
> knobs attached to the parent and skip in the middle. Topology-wise,
> that'd make more sense too.
Let me know how you think about my proposal above.
Cheers,
Longma
--
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 28/32] x86/mm, kexec: Allow kexec to be used with SME
From: Tom Lendacky @ 2017-05-19 21:07 UTC (permalink / raw)
To: Borislav Petkov
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: <20170519205836.3wvl3nztqyyouje3@pd.tnic>
On 5/19/2017 3:58 PM, Borislav Petkov wrote:
> On Fri, May 19, 2017 at 03:45:28PM -0500, Tom Lendacky wrote:
>> Actually there is. The above will result in data in the cache because
>> halt() turns into a function call if CONFIG_PARAVIRT is defined (refer
>> to the comment above where do_wbinvd_halt is set to true). I could make
>> this a native_wbinvd() and native_halt()
>
> That's why we have the native_* versions - to bypass paravirt crap.
As long as those never change from static inline everything will be
fine. I can change it, but I really like how it explicitly indicates
what is needed in this case. Even if the function gets changed from
static inline the fact that the instructions are sequential in the
function covers that case.
Thanks,
Tom
>
--
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
* [PATCH 3/3] mm/slub: Put tid_to_cpu() and tid_to_event() inside #ifdef block
From: Matthias Kaehlcke @ 2017-05-19 21:00 UTC (permalink / raw)
To: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
Andrew Morton
Cc: linux-mm, linux-kernel, Matthias Kaehlcke
In-Reply-To: <20170519210036.146880-1-mka@chromium.org>
The functions are only used when certain config options are set. Putting
them inside #ifdef fixes the following warnings when building with clang:
mm/slub.c:1759:28: error: unused function 'tid_to_cpu'
[-Werror,-Wunused-function]
^
mm/slub.c:1764:29: error: unused function 'tid_to_event'
[-Werror,-Wunused-function]
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
mm/slub.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/mm/slub.c b/mm/slub.c
index 23a8eb83efff..6df95738420d 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1945,15 +1945,19 @@ static inline unsigned long next_tid(unsigned long tid)
return tid + TID_STEP;
}
+#ifdef SLUB_DEBUG_CMPXCHG
+#ifdef CONFIG_PREEMPT
static inline unsigned int tid_to_cpu(unsigned long tid)
{
return tid % TID_STEP;
}
+#endif
static inline unsigned long tid_to_event(unsigned long tid)
{
return tid / TID_STEP;
}
+#endif
static inline unsigned int init_tid(int cpu)
{
--
2.13.0.303.g4ebf302169-goog
--
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
* [PATCH 2/3] mm/slub: Mark slab_free_hook() as __maybe_unused
From: Matthias Kaehlcke @ 2017-05-19 21:00 UTC (permalink / raw)
To: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
Andrew Morton
Cc: linux-mm, linux-kernel, Matthias Kaehlcke
In-Reply-To: <20170519210036.146880-1-mka@chromium.org>
The function is only used when certain configuration option are enabled.
Adding the attribute fixes the following warning when building with
clang:
mm/slub.c:1258:20: error: unused function 'slab_free_hook'
[-Werror,-Wunused-function]
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
mm/slub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/slub.c b/mm/slub.c
index 66e1046435b7..23a8eb83efff 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1328,7 +1328,7 @@ static inline void kfree_hook(const void *x)
kasan_kfree_large(x);
}
-static inline void *slab_free_hook(struct kmem_cache *s, void *x)
+static inline void *__maybe_unused slab_free_hook(struct kmem_cache *s, void *x)
{
void *freeptr;
--
2.13.0.303.g4ebf302169-goog
--
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
* [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems
From: Matthias Kaehlcke @ 2017-05-19 21:00 UTC (permalink / raw)
To: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
Andrew Morton
Cc: linux-mm, linux-kernel, Matthias Kaehlcke
In-Reply-To: <20170519210036.146880-1-mka@chromium.org>
The function is only used when CONFIG_NUMA=y. Placing it in an #ifdef
block fixes the following warning when building with clang:
mm/slub.c:1246:20: error: unused function 'kmalloc_large_node_hook'
[-Werror,-Wunused-function]
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
mm/slub.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/mm/slub.c b/mm/slub.c
index 57e5156f02be..66e1046435b7 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1313,11 +1313,14 @@ static inline void dec_slabs_node(struct kmem_cache *s, int node,
* Hooks for other subsystems that check memory allocations. In a typical
* production configuration these hooks all should produce no code at all.
*/
+
+#ifdef CONFIG_NUMA
static inline void kmalloc_large_node_hook(void *ptr, size_t size, gfp_t flags)
{
kmemleak_alloc(ptr, size, 1, flags);
kasan_kmalloc_large(ptr, size, flags);
}
+#endif
static inline void kfree_hook(const void *x)
{
--
2.13.0.303.g4ebf302169-goog
--
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
* [PATCH 0/3] mm/slub: Fix unused function warnings
From: Matthias Kaehlcke @ 2017-05-19 21:00 UTC (permalink / raw)
To: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
Andrew Morton
Cc: linux-mm, linux-kernel, Matthias Kaehlcke
This series fixes a bunch of warnings about unused functions in SLUB
Matthias Kaehlcke (3):
mm/slub: Only define kmalloc_large_node_hook() for NUMA systems
mm/slub: Mark slab_free_hook() as __maybe_unused
mm/slub: Put tid_to_cpu() and tid_to_event() inside #ifdef block
mm/slub.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
--
2.13.0.303.g4ebf302169-goog
--
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 28/32] x86/mm, kexec: Allow kexec to be used with SME
From: Borislav Petkov @ 2017-05-19 20:58 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: <1b74e0e6-3dda-f638-461b-f73af9904360@amd.com>
On Fri, May 19, 2017 at 03:45:28PM -0500, Tom Lendacky wrote:
> Actually there is. The above will result in data in the cache because
> halt() turns into a function call if CONFIG_PARAVIRT is defined (refer
> to the comment above where do_wbinvd_halt is set to true). I could make
> this a native_wbinvd() and native_halt()
That's why we have the native_* versions - to bypass paravirt crap.
--
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 PATCH v2 11/17] cgroup: Implement new thread mode semantics
From: Tejun Heo @ 2017-05-19 20:58 UTC (permalink / raw)
To: Waiman Long
Cc: Li Zefan, Johannes Weiner, Peter Zijlstra, Ingo Molnar, cgroups,
linux-kernel, linux-doc, linux-mm, kernel-team, pjt, luto, efault
In-Reply-To: <20170519202624.GA15279@wtj.duckdns.org>
Hello,
On Fri, May 19, 2017 at 04:26:24PM -0400, Tejun Heo wrote:
> (exactly in the way necessary), I wonder whether it'd be better to
> simply allow root to be both domain and thread root.
I'll give this approach a shot early next week.
Thanks.
--
tejun
--
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 13/17] cgroup: Allow fine-grained controllers control in cgroup v2
From: Tejun Heo @ 2017-05-19 20:55 UTC (permalink / raw)
To: Waiman Long
Cc: Li Zefan, Johannes Weiner, Peter Zijlstra, Ingo Molnar, cgroups,
linux-kernel, linux-doc, linux-mm, kernel-team, pjt, luto, efault
In-Reply-To: <1494855256-12558-14-git-send-email-longman@redhat.com>
Hello, Waiman.
On Mon, May 15, 2017 at 09:34:12AM -0400, Waiman Long wrote:
> For cgroup v1, different controllers can be binded to different cgroup
> hierarchies optimized for their own use cases. That is not currently
> the case for cgroup v2 where combining all these controllers into
> the same hierarchy will probably require more levels than is needed
> by each individual controller.
>
> By not enabling a controller in a cgroup and its descendants, we can
> effectively trim the hierarchy as seen by a controller from the leafs
> up. However, there is currently no way to compress the hierarchy in
> the intermediate levels.
>
> This patch implements a fine-grained mechanism to allow a controller to
> skip some intermediate levels in a hierarchy and effectively flatten
> the hierarchy as seen by that controller.
>
> Controllers can now be directly enabled or disabled in a cgroup
> by writing to the "cgroup.controllers" file. The special prefix
> '#' with the controller name is used to set that controller in
> pass-through mode. In that mode, the controller is disabled for that
> cgroup but it allows its children to have that controller enabled or
> in pass-through mode again.
>
> With this change, each controller can now have a unique view of their
> virtual process hierarchy that can be quite different from other
> controllers. We now have the freedom and flexibility to create the
> right hierarchy for each controller to suit their own needs without
> performance loss when compared with cgroup v1.
I can see the appeal but this needs at least more refinements.
This breaks the invariant that in a cgroup its resource control knobs
control distribution of resources from its parent. IOW, the resource
control knobs of a cgroup always belong to the parent. This is also
reflected in how delegation is done. The delegatee assumes ownership
of the cgroup itself and the ability to manage sub-cgroups but doesn't
get the ownership of the resource control knobs as otherwise the
parent would lose control over how it distributes its resources.
Another aspect is that most controllers aren't that sensitive to
nesting several levels. Expensive operations can be and already are
aggregated and the performance overhead of several levels of nesting
barely shows up. Skipping levels can be an interesting optimization
approach and we can definitely support from the core side; however,
it'd be a lot nicer if we could do that optimization transparently
(e.g. CPU can skip multi level queueing if there usually is only one
item at some levels).
Hmm... that said, if we can fix the delegation issue in a not-too-ugly
way, why not? I wonder whether we can still keep the resource control
knobs attached to the parent and skip in the middle. Topology-wise,
that'd make more sense too.
Thanks.
--
tejun
--
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 17/32] x86/mm: Add support to access boot related data in the clear
From: Tom Lendacky @ 2017-05-19 20:50 UTC (permalink / raw)
To: Borislav Petkov
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: <20170518090212.kebstmnjv4h3cjf2@pd.tnic>
On 5/18/2017 4:02 AM, Borislav Petkov wrote:
> On Wed, May 17, 2017 at 01:54:39PM -0500, Tom Lendacky wrote:
>> I was worried what the compiler might do when CONFIG_EFI is not set,
>> but it appears to take care of it. I'll double check though.
>
> There's a efi_enabled() !CONFIG_EFI version too, so should be fine.
>
>> I may introduce a length variable to capture data->len right after
>> paddr_next is set and then have just a single memunmap() call before
>> the if check.
>
> Yap.
>
>> I tried that, but calling an "__init" function (early_memremap()) from
>> a non "__init" function generated warnings. I suppose I can pass in a
>> function for the map and unmap but that looks worse to me (also the
>> unmap functions take different arguments).
>
> No, the other way around: the __init function should call the non-init
> one and you need the non-init one anyway for memremap_is_setup_data().
>
The "worker" function would be doing the loop through the setup data,
but since the setup data is mapped inside the loop I can't do the __init
calling the non-init function and still hope to consolidate the code.
Maybe I'm missing something here...
Thanks,
Tom
>> This is like the chicken and the egg scenario. In order to determine if
>> an address is setup data I have to explicitly map the setup data chain
>> as decrypted. In order to do that I have to supply a flag to explicitly
>> map the data decrypted otherwise I wind up back in the
>> memremap_is_setup_data() function again and again and again...
>
> Oh, fun.
>
--
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 02/11] mm: mempolicy: add queue_pages_node_check()
From: Zi Yan @ 2017-05-19 20:48 UTC (permalink / raw)
To: Mel Gorman
Cc: Anshuman Khandual, n-horiguchi, kirill.shutemov, linux-kernel,
linux-mm, akpm, minchan, vbabka, mhocko, dnellans
In-Reply-To: <20170519202843.lco2rkkivh2a433k@techsingularity.net>
[-- Attachment #1: Type: text/plain, Size: 929 bytes --]
On 19 May 2017, at 16:28, Mel Gorman wrote:
> On Fri, May 19, 2017 at 12:37:38PM -0400, Zi Yan wrote:
>>> As you say, there is no functional change but the helper name is vague
>>> and gives no hint to what's it's checking for. It's somewhat tolerable as
>>> it is as it's obvious what is being checked but the same is not true with
>>> the helper name.
>>>
>>
>> Does queue_pages_invert_nodemask_check() work? I can change the helper name
>> in the next version.
>>
>
> Not particularly, maybe queue_pages_required and invert the check with a
> comment above it explaining what it's checking for would be ok.
>
queue_pages_required() is too broad, I would take queue_pages_page_nid_check()
and invert the check with a comment above saying
/*
* Check if the page's nid is in qp->nmask.
*
* If MPOL_MF_INVERT is set in qp->flags, check if the nid is
* in the invert of qp->nmask.
*/
Does it work?
--
Best Regards
Yan Zi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 496 bytes --]
^ permalink raw reply
* Re: [PATCH v5 28/32] x86/mm, kexec: Allow kexec to be used with SME
From: Tom Lendacky @ 2017-05-19 20:45 UTC (permalink / raw)
To: Borislav Petkov
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: <20170517191755.h2xluopk2p6suw32@pd.tnic>
On 5/17/2017 2:17 PM, Borislav Petkov wrote:
> On Tue, Apr 18, 2017 at 04:21:21PM -0500, Tom Lendacky wrote:
>> Provide support so that kexec can be used to boot a kernel when SME is
>> enabled.
>>
>> Support is needed to allocate pages for kexec without encryption. This
>> is needed in order to be able to reboot in the kernel in the same manner
>> as originally booted.
>>
>> Additionally, when shutting down all of the CPUs we need to be sure to
>> flush the caches and then halt. This is needed when booting from a state
>> where SME was not active into a state where SME is active (or vice-versa).
>> Without these steps, it is possible for cache lines to exist for the same
>> physical location but tagged both with and without the encryption bit. This
>> can cause random memory corruption when caches are flushed depending on
>> which cacheline is written last.
>>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
>> arch/x86/include/asm/init.h | 1 +
>> arch/x86/include/asm/irqflags.h | 5 +++++
>> arch/x86/include/asm/kexec.h | 8 ++++++++
>> arch/x86/include/asm/pgtable_types.h | 1 +
>> arch/x86/kernel/machine_kexec_64.c | 35 +++++++++++++++++++++++++++++++++-
>> arch/x86/kernel/process.c | 26 +++++++++++++++++++++++--
>> arch/x86/mm/ident_map.c | 11 +++++++----
>> include/linux/kexec.h | 14 ++++++++++++++
>> kernel/kexec_core.c | 7 +++++++
>> 9 files changed, 101 insertions(+), 7 deletions(-)
>
> ...
>
>> @@ -86,7 +86,7 @@ static int init_transition_pgtable(struct kimage *image, pgd_t *pgd)
>> set_pmd(pmd, __pmd(__pa(pte) | _KERNPG_TABLE));
>> }
>> pte = pte_offset_kernel(pmd, vaddr);
>> - set_pte(pte, pfn_pte(paddr >> PAGE_SHIFT, PAGE_KERNEL_EXEC));
>> + set_pte(pte, pfn_pte(paddr >> PAGE_SHIFT, PAGE_KERNEL_EXEC_NOENC));
>> return 0;
>> err:
>> free_transition_pgtable(image);
>> @@ -114,6 +114,7 @@ static int init_pgtable(struct kimage *image, unsigned long start_pgtable)
>> .alloc_pgt_page = alloc_pgt_page,
>> .context = image,
>> .pmd_flag = __PAGE_KERNEL_LARGE_EXEC,
>> + .kernpg_flag = _KERNPG_TABLE_NOENC,
>> };
>> unsigned long mstart, mend;
>> pgd_t *level4p;
>> @@ -597,3 +598,35 @@ void arch_kexec_unprotect_crashkres(void)
>> {
>> kexec_mark_crashkres(false);
>> }
>> +
>> +int arch_kexec_post_alloc_pages(void *vaddr, unsigned int pages, gfp_t gfp)
>> +{
>> + int ret;
>> +
>> + if (sme_active()) {
>
> if (!sme_active())
> return 0;
>
> /*
> * If SME...
>
Ok.
>
>> + /*
>> + * If SME is active we need to be sure that kexec pages are
>> + * not encrypted because when we boot to the new kernel the
>> + * pages won't be accessed encrypted (initially).
>> + */
>> + ret = set_memory_decrypted((unsigned long)vaddr, pages);
>> + if (ret)
>> + return ret;
>> +
>> + if (gfp & __GFP_ZERO)
>> + memset(vaddr, 0, pages * PAGE_SIZE);
>
> This function is called after alloc_pages() which already zeroes memory
> when __GFP_ZERO is supplied.
>
> If you need to clear the memory *after* set_memory_encrypted() happens,
> then you should probably mask out __GFP_ZERO before the alloc_pages()
> call so as not to do it twice.
I'll look into that. I could put the memset() at the end of this
function so that it is done here no matter what. And update the
default arch_kexec_post_alloc_pages() to also do the memset(). It
just hides the clearing of the pages a bit though by doing that.
>
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +void arch_kexec_pre_free_pages(void *vaddr, unsigned int pages)
>> +{
>> + if (sme_active()) {
>> + /*
>> + * If SME is active we need to reset the pages back to being
>> + * an encrypted mapping before freeing them.
>> + */
>> + set_memory_encrypted((unsigned long)vaddr, pages);
>> + }
>> +}
>> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
>> index 0bb8842..f4e5de6 100644
>> --- a/arch/x86/kernel/process.c
>> +++ b/arch/x86/kernel/process.c
>> @@ -24,6 +24,7 @@
>> #include <linux/cpuidle.h>
>> #include <trace/events/power.h>
>> #include <linux/hw_breakpoint.h>
>> +#include <linux/kexec.h>
>> #include <asm/cpu.h>
>> #include <asm/apic.h>
>> #include <asm/syscalls.h>
>> @@ -355,8 +356,25 @@ bool xen_set_default_idle(void)
>> return ret;
>> }
>> #endif
>> +
>> void stop_this_cpu(void *dummy)
>> {
>> + bool do_wbinvd_halt = false;
>> +
>> + if (kexec_in_progress && boot_cpu_has(X86_FEATURE_SME)) {
>> + /*
>> + * If we are performing a kexec and the processor supports
>> + * SME then we need to clear out cache information before
>> + * halting. With kexec, going from SME inactive to SME active
>> + * requires clearing cache entries so that addresses without
>> + * the encryption bit set don't corrupt the same physical
>> + * address that has the encryption bit set when caches are
>> + * flushed. Perform a wbinvd followed by a halt to achieve
>> + * this.
>> + */
>> + do_wbinvd_halt = true;
>> + }
>> +
>> local_irq_disable();
>> /*
>> * Remove this CPU:
>> @@ -365,8 +383,12 @@ void stop_this_cpu(void *dummy)
>> disable_local_APIC();
>> mcheck_cpu_clear(this_cpu_ptr(&cpu_info));
>>
>> - for (;;)
>> - halt();
>> + for (;;) {
>> + if (do_wbinvd_halt)
>> + native_wbinvd_halt();
>
> No need for that native_wbinvd_halt() thing:
>
> for (;;) {
> if (do_wbinvd)
> wbinvd();
>
> halt();
> }
>
Actually there is. The above will result in data in the cache because
halt() turns into a function call if CONFIG_PARAVIRT is defined (refer
to the comment above where do_wbinvd_halt is set to true). I could make
this a native_wbinvd() and native_halt() as long as those are
guaranteed to never turn into function calls. But never say never, so
that's why I created native_wbinvd_halt().
Thanks,
Tom
>> /*
>> diff --git a/arch/x86/mm/ident_map.c b/arch/x86/mm/ident_map.c
>> index 04210a2..2c9fd3e 100644
>> --- a/arch/x86/mm/ident_map.c
>> +++ b/arch/x86/mm/ident_map.c
>> @@ -20,6 +20,7 @@ static void ident_pmd_init(struct x86_mapping_info *info, pmd_t *pmd_page,
>> static int ident_pud_init(struct x86_mapping_info *info, pud_t *pud_page,
>> unsigned long addr, unsigned long end)
>> {
>> + unsigned long kernpg_flag = info->kernpg_flag ? : _KERNPG_TABLE;
>
> You're already supplying a x86_mapping_info and thus you can init
> kernpg_flag to default _KERNPG_TABLE and override it in the SME+kexec
> case, as you already do. And this way you can simply do:
>
> set_pud(pud, __pud(__pa(pmd) | info->kernpg_flag));
>
> here and in the other pagetable functions I've snipped below, and save
> yourself some lines.
Ok, I'll check into that.
Thanks,
Tom
>
> ...
>
--
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 12/17] cgroup: Remove cgroup v2 no internal process constraint
From: Tejun Heo @ 2017-05-19 20:38 UTC (permalink / raw)
To: Waiman Long
Cc: Li Zefan, Johannes Weiner, Peter Zijlstra, Ingo Molnar, cgroups,
linux-kernel, linux-doc, linux-mm, kernel-team, pjt, luto, efault
In-Reply-To: <1494855256-12558-13-git-send-email-longman@redhat.com>
Hello, Waiman.
On Mon, May 15, 2017 at 09:34:11AM -0400, Waiman Long wrote:
> The rationale behind the cgroup v2 no internal process constraint is
> to avoid resouorce competition between internal processes and child
> cgroups. However, not all controllers have problem with internal
> process competiton. Enforcing this rule may lead to unnatural process
> hierarchy and unneeded levels for those controllers.
This isn't necessarily something we can determine by looking at the
current state of controllers. It's true that some controllers - pid
and perf - inherently only care about membership of each task but at
the same time neither really suffers from the constraint either. CPU
which is the problematic one here and currently only cares about tasks
actually distributes resources which have parts which are specific to
domain rather than threads and we don't want to declare that CPU isn't
domain aware resource because it inherently is.
> This patch removes the no internal process contraint by enabling those
> controllers that don't like internal process competition to have a
> separate set of control knobs just for internal processes in a cgroup.
>
> A new control file "cgroup.resource_control" is added. Enabling a
> controller with a "+" prefix will create a separate set of control
> knobs for that controller in the special "cgroup.resource_domain"
> sub-directory for all the internal processes. The existing control
> knobs in the cgroup will then be used to manage resource distribution
> between internal processes as a group and other child cgroups.
We would need to declare all major resource controllers to be needing
that special sub-directory. That'd work around the
no-internal-process constraint but I don't think it is solving any
real problems. It's just the kernel doing something that userland can
do with ease and more context.
Thanks.
--
tejun
--
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 32/32] x86/mm: Add support to make use of Secure Memory Encryption
From: Borislav Petkov @ 2017-05-19 20:29 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Tom Lendacky, 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: <20170519201651.dhayf2pwjlsnouz4@treble>
On Fri, May 19, 2017 at 03:16:51PM -0500, Josh Poimboeuf wrote:
> I'm the stack validation guy, not the stack protection guy :-)
LOL. I thought you were *the* stacks guy. :-)))
But once you've validated it, you could protect it then too. :-)
--
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
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