* Re: [PATCH v2] mm: fix the memory leak after collapsing the huge page fails
From: zhong jiang @ 2017-05-09 13:54 UTC (permalink / raw)
To: Vlastimil Babka; +Cc: akpm, kirill.shutemov, hannes, mgorman, linux-mm
In-Reply-To: <0bca4592-efa5-deba-0369-19beacfd2a63@suse.cz>
Hi, Vlastimil
I review the code again. it works well for NUMA. because
khugepaged_prealloc_page will put_page when *hpage is true.
the memory leak will still exist in !NUMA. because it ingore
the put_page. is it right? I miss something.
Thanks
zhongjiang
On 2017/5/9 20:41, Vlastimil Babka wrote:
> On 05/09/2017 02:20 PM, zhong jiang wrote:
>> On 2017/5/9 19:34, Vlastimil Babka wrote:
>>> On 05/09/2017 12:55 PM, zhongjiang wrote:
>>>> From: zhong jiang <zhongjiang@huawei.com>
>>>>
>>>> Current, when we prepare a huge page to collapse, due to some
>>>> reasons, it can fail to collapse. At the moment, we should
>>>> release the preallocate huge page.
>>>>
>>>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>>> Hmm, scratch that, there's no memory leak. The pointer to new_page is
>>> stored in *hpage, and put_page() is called all the way up in
>>> khugepaged_do_scan().
>> I see. I miss it. but why the new_page need to be release all the way.
> AFAIK to support preallocation and reusal of preallocated page for
> collapse attempt in different pmd. It only works for !NUMA so it's
> likely not worth all the trouble and complicated code, so I wouldn't be
> opposed to simplifying this.
>
>> I do not see the count increment when scan success. it save the memory,
>> only when page fault happen.
> I don't understand what you mean here?
>
>> Thanks
>> zhongjiang
>>>> ---
>>>> mm/khugepaged.c | 4 ++++
>>>> 1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>>> index 7cb9c88..586b1f1 100644
>>>> --- a/mm/khugepaged.c
>>>> +++ b/mm/khugepaged.c
>>>> @@ -1082,6 +1082,8 @@ static void collapse_huge_page(struct mm_struct *mm,
>>>> up_write(&mm->mmap_sem);
>>>> out_nolock:
>>>> trace_mm_collapse_huge_page(mm, isolated, result);
>>>> + if (page != NULL && result != SCAN_SUCCEED)
>>>> + put_page(new_page);
>>>> return;
>>>> out:
>>>> mem_cgroup_cancel_charge(new_page, memcg, true);
>>>> @@ -1555,6 +1557,8 @@ static void collapse_shmem(struct mm_struct *mm,
>>>> }
>>>> out:
>>>> VM_BUG_ON(!list_empty(&pagelist));
>>>> + if (page != NULL && result != SCAN_SUCCEED)
>>>> + put_page(new_page);
>>>> /* TODO: tracepoints */
>>>> }
>>>>
>>>>
>>> .
>>>
>>
>
> .
>
--
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 v2] mm: fix the memory leak after collapsing the huge page fails
From: Vlastimil Babka @ 2017-05-09 13:46 UTC (permalink / raw)
To: zhong jiang; +Cc: akpm, kirill.shutemov, hannes, mgorman, linux-mm
In-Reply-To: <5911C4AC.2090402@huawei.com>
On 05/09/2017 03:31 PM, zhong jiang wrote:
> On 2017/5/9 20:41, Vlastimil Babka wrote:
>> On 05/09/2017 02:20 PM, zhong jiang wrote:
>>> On 2017/5/9 19:34, Vlastimil Babka wrote:
>>>> On 05/09/2017 12:55 PM, zhongjiang wrote:
>>>>> From: zhong jiang <zhongjiang@huawei.com>
>>>>>
>>>>> Current, when we prepare a huge page to collapse, due to some
>>>>> reasons, it can fail to collapse. At the moment, we should
>>>>> release the preallocate huge page.
>>>>>
>>>>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>>>> Hmm, scratch that, there's no memory leak. The pointer to new_page is
>>>> stored in *hpage, and put_page() is called all the way up in
>>>> khugepaged_do_scan().
>>> I see. I miss it. but why the new_page need to be release all the way.
>> AFAIK to support preallocation and reusal of preallocated page for
>> collapse attempt in different pmd. It only works for !NUMA so it's
>> likely not worth all the trouble and complicated code, so I wouldn't be
>> opposed to simplifying this.
>>
>>> I do not see the count increment when scan success. it save the memory,
>>> only when page fault happen.
>> I don't understand what you mean here?
> I mean that whether collapse huge page success or nor, preallocate page will
> alway be release. by the above description. I seems to true.
Ah, no. In case of success, collapse_huge_page() does *hpage = NULL; so
the put_page() won't get to it anymore.
>
> Thanks
> zhongjiang
>>> Thanks
>>> zhongjiang
>>>>> ---
>>>>> mm/khugepaged.c | 4 ++++
>>>>> 1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>>>> index 7cb9c88..586b1f1 100644
>>>>> --- a/mm/khugepaged.c
>>>>> +++ b/mm/khugepaged.c
>>>>> @@ -1082,6 +1082,8 @@ static void collapse_huge_page(struct mm_struct *mm,
>>>>> up_write(&mm->mmap_sem);
>>>>> out_nolock:
>>>>> trace_mm_collapse_huge_page(mm, isolated, result);
>>>>> + if (page != NULL && result != SCAN_SUCCEED)
>>>>> + put_page(new_page);
>>>>> return;
>>>>> out:
>>>>> mem_cgroup_cancel_charge(new_page, memcg, true);
>>>>> @@ -1555,6 +1557,8 @@ static void collapse_shmem(struct mm_struct *mm,
>>>>> }
>>>>> out:
>>>>> VM_BUG_ON(!list_empty(&pagelist));
>>>>> + if (page != NULL && result != SCAN_SUCCEED)
>>>>> + put_page(new_page);
>>>>> /* TODO: tracepoints */
>>>>> }
>>>>>
>>>>>
>>>> .
>>>>
>>>
>>
>> .
>>
>
>
--
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 0/4] RFC - Coherent Device Memory (Not for inclusion)
From: Benjamin Herrenschmidt @ 2017-05-09 13:43 UTC (permalink / raw)
To: Michal Hocko
Cc: Balbir Singh, linux-mm, akpm, khandual, aneesh.kumar, paulmck,
srikar, haren, jglisse, mgorman, arbab, vbabka, cl
In-Reply-To: <20170509113638.GJ6481@dhcp22.suse.cz>
On Tue, 2017-05-09 at 13:36 +0200, Michal Hocko wrote:
> But this is not what the CDM as proposed here is about AFAIU. It is
> argued this is not a _normal_ cpuless node and it neads tweak here and
> there. And that is my main objection about. I do not mind if the memory
> is presented as a hotplugable cpuless memory node. I just do not want it
> to be any more special than cpuless nodes are already.
But if you look at where things are going with the new kind of memory
technologies appearing etc... I think the concept of "normal" for
memory is rather fragile.
So I think it makes sense to grow the idea that nodes have "attributes"
that affect the memory policies.
That said, one thing we do need to clarify, especially in the context
of our short term GPU usage model, is of those attributes, what is
inherent to the way the HW works and what is more related to the actual
userspace usage model, the latter possibly being better dealt with with
existing policy mechanisms).
Also maybe understand how much of these things are likely to be shared
with other type of devices such as OpenCAPI or CCIX.
> > > So let me repeat the fundamental question. Is the only difference from
> > > cpuless nodes the fact that the node should be invisible to processes
> > > unless they specify an explicit node mask?
> >
> > It would be *preferable* that it is.
> >
> > It's not necessarily an absolute requirement as long as what lands
> > there can be kicked out. However the system would potentially be
> > performing poorly if too much unrelated stuff lands on the GPU memory
> > as it has a much higher latency.
>
> This is a general concern for many cpuless NUMA node systems. You have
> to pay for the suboptimal performance when accessing that memory. And
> you have means to cope with that.
Yup. However in this case, GPU memory is really bad, so that's one
reason why we want to push the idea of effectively not allowing non-
explicit allocations from it.
Thus, memory would be allocated from that node only if either the
application (or driver) use explicit APIs to grab some of it, or if the
driver migrates pages to it. (Or possibly, if we can make that work,
the memory is provisioned as the result of a page fault by the GPU
itself).
> > Due to the nature of GPUs (and possibly other such accelerators but not
> > necessarily all of them), that memory is also more likely to fail. GPUs
> > crash often. However that isn't necessarily true of OpenCAPI devices or
> > CCIX.
> >
> > This is the kind of attributes of the memory (quality ?) that can be
> > provided by the driver that is putting it online. We can then
> > orthogonally decide how we chose (or not) to take those into account,
> > either in the default mm algorithms or from explicit policy mechanisms
> > set from userspace, but the latter is often awkward and never done
> > right.
>
> The first adds maintain costs all over the place and just looking at
> what become of memory policies and cpusets makes me cry. I definitely do
> not want more special casing on top (and just to make it clear a special
> N_MEMORY_$FOO falls into the same category).
>
> [...]
> > > Moreover cpusets already support exclusive numa nodes AFAIR.
> >
> > Which implies that the user would have to do epxlciit cpuset
> > manipulations for the system to work right ? Most user wouldn't and the
> > rsult is that most user would have badly working systems. That's almost
> > always what happens when we chose to bounce *all* policy decision to
> > the user without the kernel attempting to have some kind of semi-sane
> > default.
>
> I would argue that this is the case for cpuless numa nodes already.
> Users should better know what they are doing when using such a
> specialized HW. And that includes a specialized configuration.
So what you are saying is that users who want to use GPUs or FPGAs or
accelerated devices will need to have intimate knowledge of Linux CPU
and memory policy management at a low level.
That's where I disagree.
People want to throw these things at all sort of problems out there,
hide them behind libraries, and have things "just work".
The user will just use applications normally. Those will be use
more/less standard libraries to perform various computations, these
libraries will know how to take advantage of accelerators, nothing in
that chains knows about memory policies & placement, cpusets etc... and
nothing *should*.
Of course, the special case of the HPC user trying to milk the last
cycle out of the system is probably going to do what you suggest. But
most users won't.
Cheers,
Ben.
--
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 v2] mm: fix the memory leak after collapsing the huge page fails
From: zhong jiang @ 2017-05-09 13:31 UTC (permalink / raw)
To: Vlastimil Babka; +Cc: akpm, kirill.shutemov, hannes, mgorman, linux-mm
In-Reply-To: <0bca4592-efa5-deba-0369-19beacfd2a63@suse.cz>
On 2017/5/9 20:41, Vlastimil Babka wrote:
> On 05/09/2017 02:20 PM, zhong jiang wrote:
>> On 2017/5/9 19:34, Vlastimil Babka wrote:
>>> On 05/09/2017 12:55 PM, zhongjiang wrote:
>>>> From: zhong jiang <zhongjiang@huawei.com>
>>>>
>>>> Current, when we prepare a huge page to collapse, due to some
>>>> reasons, it can fail to collapse. At the moment, we should
>>>> release the preallocate huge page.
>>>>
>>>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>>> Hmm, scratch that, there's no memory leak. The pointer to new_page is
>>> stored in *hpage, and put_page() is called all the way up in
>>> khugepaged_do_scan().
>> I see. I miss it. but why the new_page need to be release all the way.
> AFAIK to support preallocation and reusal of preallocated page for
> collapse attempt in different pmd. It only works for !NUMA so it's
> likely not worth all the trouble and complicated code, so I wouldn't be
> opposed to simplifying this.
>
>> I do not see the count increment when scan success. it save the memory,
>> only when page fault happen.
> I don't understand what you mean here?
I mean that whether collapse huge page success or nor, preallocate page will
alway be release. by the above description. I seems to true.
Thanks
zhongjiang
>> Thanks
>> zhongjiang
>>>> ---
>>>> mm/khugepaged.c | 4 ++++
>>>> 1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>>> index 7cb9c88..586b1f1 100644
>>>> --- a/mm/khugepaged.c
>>>> +++ b/mm/khugepaged.c
>>>> @@ -1082,6 +1082,8 @@ static void collapse_huge_page(struct mm_struct *mm,
>>>> up_write(&mm->mmap_sem);
>>>> out_nolock:
>>>> trace_mm_collapse_huge_page(mm, isolated, result);
>>>> + if (page != NULL && result != SCAN_SUCCEED)
>>>> + put_page(new_page);
>>>> return;
>>>> out:
>>>> mem_cgroup_cancel_charge(new_page, memcg, true);
>>>> @@ -1555,6 +1557,8 @@ static void collapse_shmem(struct mm_struct *mm,
>>>> }
>>>> out:
>>>> VM_BUG_ON(!list_empty(&pagelist));
>>>> + if (page != NULL && result != SCAN_SUCCEED)
>>>> + put_page(new_page);
>>>> /* TODO: tracepoints */
>>>> }
>>>>
>>>>
>>> .
>>>
>>
>
> .
>
--
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 v3 4/4] mm: Adaptive hash table scaling
From: Pasha Tatashin @ 2017-05-09 13:07 UTC (permalink / raw)
To: Michal Hocko, Andrew Morton; +Cc: linux-mm, sparclinux, linux-fsdevel, Al Viro
In-Reply-To: <20170509094607.GG6481@dhcp22.suse.cz>
Thank you Michal for making this change.
Reviewed-by: Pavel Tatashin <pasha.tatashin@oracle.com>
>
> OK, Andrew tends to fold follow up fixes in his mm tree. But anyway, as
> you prefer to have this in a separate patch. Could you add this on top
> Andrew? I believe mnt hash tables need a _reasonable_ upper bound but
> that is for a separate patch I believe.
> ---
> From ac970fdb3e6f5f03a440fdbe6fe09460d99d3557 Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.com>
> Date: Tue, 9 May 2017 11:34:59 +0200
> Subject: [PATCH] mm: drop HASH_ADAPT
>
> "mm: Adaptive hash table scaling" has introduced a new large hash table
> automatic scaling because the previous implementation led to too large
> hashes on TB systems. This is all nice and good but the patch assumes that
> callers of alloc_large_system_hash will opt-in to use this new scaling.
> This makes the API unnecessarily complicated and error prone. The only
> thing that callers should care about is whether they have an upper
> bound for the size or leave it to alloc_large_system_hash to decide (by
> providing high_limit == 0).
>
> As a quick code inspection shows there are users with high_limit == 0
> which do not use the flag already e.g. {dcache,inode}_init_early or
> mnt_init when creating mnt has tables. They certainly have no good
> reason to use a different scaling because the [di]cache was the
> motivation for introducing a different scaling in the first place (we
> just do this attempt and use memblock). It is also hard to imagine why
> we would mnt hash tables need larger hash tables.
>
> Just drop the flag and use the scaling whenever there is no high_limit
> specified.
>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
> fs/dcache.c | 2 +-
> fs/inode.c | 2 +-
> include/linux/bootmem.h | 1 -
> mm/page_alloc.c | 2 +-
> 4 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 808ea99062c2..363502faa328 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -3585,7 +3585,7 @@ static void __init dcache_init(void)
> sizeof(struct hlist_bl_head),
> dhash_entries,
> 13,
> - HASH_ZERO | HASH_ADAPT,
> + HASH_ZERO,
> &d_hash_shift,
> &d_hash_mask,
> 0,
> diff --git a/fs/inode.c b/fs/inode.c
> index 32c8ee454ef0..1b15a7cc78ce 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1953,7 +1953,7 @@ void __init inode_init(void)
> sizeof(struct hlist_head),
> ihash_entries,
> 14,
> - HASH_ZERO | HASH_ADAPT,
> + HASH_ZERO,
> &i_hash_shift,
> &i_hash_mask,
> 0,
> diff --git a/include/linux/bootmem.h b/include/linux/bootmem.h
> index dbaf312b3317..e223d91b6439 100644
> --- a/include/linux/bootmem.h
> +++ b/include/linux/bootmem.h
> @@ -359,7 +359,6 @@ extern void *alloc_large_system_hash(const char *tablename,
> #define HASH_SMALL 0x00000002 /* sub-page allocation allowed, min
> * shift passed via *_hash_shift */
> #define HASH_ZERO 0x00000004 /* Zero allocated hash table */
> -#define HASH_ADAPT 0x00000008 /* Adaptive scale for large memory */
>
> /* Only NUMA needs hash distribution. 64bit NUMA architectures have
> * sufficient vmalloc space.
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index beb2827fd5de..3b840b998c05 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -7213,7 +7213,7 @@ void *__init alloc_large_system_hash(const char *tablename,
> if (PAGE_SHIFT < 20)
> numentries = round_up(numentries, (1<<20)/PAGE_SIZE);
>
> - if (flags & HASH_ADAPT) {
> + if (!high_limit) {
> unsigned long adapt;
>
> for (adapt = ADAPT_SCALE_NPAGES; adapt < numentries;
>
--
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 03/10] x86/mm: Make the batched unmap TLB flush API more generic
From: Andy Lutomirski @ 2017-05-09 13:02 UTC (permalink / raw)
To: Dave Hansen
Cc: Andy Lutomirski, X86 ML, linux-kernel@vger.kernel.org,
Borislav Petkov, Linus Torvalds, Andrew Morton, Mel Gorman,
linux-mm@kvack.org, Rik van Riel, Nadav Amit, Michal Hocko,
Sasha Levin
In-Reply-To: <d36207ef-a4b3-24ef-40e4-9e6a22b092cb@intel.com>
On Mon, May 8, 2017 at 8:34 AM, Dave Hansen <dave.hansen@intel.com> wrote:
> On 05/07/2017 05:38 AM, Andy Lutomirski wrote:
>> diff --git a/mm/rmap.c b/mm/rmap.c
>> index f6838015810f..2e568c82f477 100644
>> --- a/mm/rmap.c
>> +++ b/mm/rmap.c
>> @@ -579,25 +579,12 @@ void page_unlock_anon_vma_read(struct anon_vma *anon_vma)
>> void try_to_unmap_flush(void)
>> {
>> struct tlbflush_unmap_batch *tlb_ubc = ¤t->tlb_ubc;
>> - int cpu;
>>
>> if (!tlb_ubc->flush_required)
>> return;
>>
>> - cpu = get_cpu();
>> -
>> - if (cpumask_test_cpu(cpu, &tlb_ubc->cpumask)) {
>> - count_vm_tlb_event(NR_TLB_LOCAL_FLUSH_ALL);
>> - local_flush_tlb();
>> - trace_tlb_flush(TLB_LOCAL_SHOOTDOWN, TLB_FLUSH_ALL);
>> - }
>> -
>> - if (cpumask_any_but(&tlb_ubc->cpumask, cpu) < nr_cpu_ids)
>> - flush_tlb_others(&tlb_ubc->cpumask, NULL, 0, TLB_FLUSH_ALL);
>> - cpumask_clear(&tlb_ubc->cpumask);
>> tlb_ubc->flush_required = false;
>> tlb_ubc->writable = false;
>> - put_cpu();
>> }
>>
>> /* Flush iff there are potentially writable TLB entries that can race with IO */
>> @@ -613,7 +600,7 @@ static void set_tlb_ubc_flush_pending(struct mm_struct *mm, bool writable)
>> {
>> struct tlbflush_unmap_batch *tlb_ubc = ¤t->tlb_ubc;
>>
>> - cpumask_or(&tlb_ubc->cpumask, &tlb_ubc->cpumask, mm_cpumask(mm));
>> + arch_tlbbatch_add_mm(&tlb_ubc->arch, mm);
>> tlb_ubc->flush_required = true;
>>
>> /*
>
> Looking at this patch in isolation, how can this be safe? It removes
> TLB flushes from the generic code. Do other patches in the series fix
> this up?
Hmm? Unless I totally screwed this up, this patch just moves the
flushes around -- it shouldn't remove any flushes.
--
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 v2] mm: fix the memory leak after collapsing the huge page fails
From: zhong jiang @ 2017-05-09 12:20 UTC (permalink / raw)
To: Vlastimil Babka; +Cc: akpm, kirill.shutemov, hannes, mgorman, linux-mm
In-Reply-To: <442638e9-d6db-2f1c-e260-9290d7524f1d@suse.cz>
On 2017/5/9 19:34, Vlastimil Babka wrote:
> On 05/09/2017 12:55 PM, zhongjiang wrote:
>> From: zhong jiang <zhongjiang@huawei.com>
>>
>> Current, when we prepare a huge page to collapse, due to some
>> reasons, it can fail to collapse. At the moment, we should
>> release the preallocate huge page.
>>
>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> Hmm, scratch that, there's no memory leak. The pointer to new_page is
> stored in *hpage, and put_page() is called all the way up in
> khugepaged_do_scan().
I see. I miss it. but why the new_page need to be release all the way.
I do not see the count increment when scan success. it save the memory,
only when page fault happen.
Thanks
zhongjiang
>> ---
>> mm/khugepaged.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 7cb9c88..586b1f1 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -1082,6 +1082,8 @@ static void collapse_huge_page(struct mm_struct *mm,
>> up_write(&mm->mmap_sem);
>> out_nolock:
>> trace_mm_collapse_huge_page(mm, isolated, result);
>> + if (page != NULL && result != SCAN_SUCCEED)
>> + put_page(new_page);
>> return;
>> out:
>> mem_cgroup_cancel_charge(new_page, memcg, true);
>> @@ -1555,6 +1557,8 @@ static void collapse_shmem(struct mm_struct *mm,
>> }
>> out:
>> VM_BUG_ON(!list_empty(&pagelist));
>> + if (page != NULL && result != SCAN_SUCCEED)
>> + put_page(new_page);
>> /* TODO: tracepoints */
>> }
>>
>>
>
> .
>
--
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 00/10] x86 TLB flush cleanups, moving toward PCID support
From: Andy Lutomirski @ 2017-05-09 12:43 UTC (permalink / raw)
To: Nadav Amit
Cc: Andy Lutomirski, X86 ML, linux-kernel@vger.kernel.org,
Borislav Petkov, Linus Torvalds, Andrew Morton, Mel Gorman,
linux-mm@kvack.org
In-Reply-To: <469C2BEE-5B6C-4351-8BC9-17796A072964@gmail.com>
On Mon, May 8, 2017 at 9:36 AM, Nadav Amit <nadav.amit@gmail.com> wrote:
>
>> On May 7, 2017, at 5:38 AM, Andy Lutomirski <luto@kernel.org> wrote:
>>
>> As I've been working on polishing my PCID code, a major problem I've
>> encountered is that there are too many x86 TLB flushing code paths and
>> that they have too many inconsequential differences. The result was
>> that earlier versions of the PCID code were a colossal mess and very
>> difficult to understand.
>>
>> This series goes a long way toward cleaning up the mess. With all the
>> patches applied, there is a single function that contains the meat of
>> the code to flush the TLB on a given CPU, and all the tlb flushing
>> APIs call it for both local and remote CPUs.
>>
>> This series should only adversely affect the kernel in a couple of
>> minor ways:
>>
>> - It makes smp_mb() unconditional when flushing TLBs. We used to
>> use the TLB flush itself to mostly avoid smp_mb() on the initiating
>> CPU.
>>
>> - On UP kernels, we lose the dubious optimization of inlining nerfed
>> variants of all the TLB flush APIs. This bloats the kernel a tiny
>> bit, although it should increase performance, since the SMP
>> versions were better.
>>
>> Patch 10 in here is a little bit off topic. It's a cleanup that's
>> also needed before PCID can go in, but it's not directly about
>> TLB flushing.
>>
>> Thoughts?
>
> In general I like the changes. I needed to hack Linux TLB shootdowns for
> a research project just because I could not handle the code otherwise.
> I ended up doing some of changes that you have done.
>
> I just have two general comments:
>
> - You may want to consider merging the kernel mappings invalidation
> with the userspace mappings invalidations as well, since there are
> still code redundancies.
>
Hmm. The code for kernel mappings is quite short, and I'm not sure
how well it would fit in if I tried to merge it.
> - Don’t expect too much from concurrent TLB invalidations. In many
> cases the IPI latency dominates the overhead from my experience.
>
Fair enough.
--
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 v2] mm: fix the memory leak after collapsing the huge page fails
From: Vlastimil Babka @ 2017-05-09 12:41 UTC (permalink / raw)
To: zhong jiang; +Cc: akpm, kirill.shutemov, hannes, mgorman, linux-mm
In-Reply-To: <5911B40D.2020007@huawei.com>
On 05/09/2017 02:20 PM, zhong jiang wrote:
> On 2017/5/9 19:34, Vlastimil Babka wrote:
>> On 05/09/2017 12:55 PM, zhongjiang wrote:
>>> From: zhong jiang <zhongjiang@huawei.com>
>>>
>>> Current, when we prepare a huge page to collapse, due to some
>>> reasons, it can fail to collapse. At the moment, we should
>>> release the preallocate huge page.
>>>
>>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>> Hmm, scratch that, there's no memory leak. The pointer to new_page is
>> stored in *hpage, and put_page() is called all the way up in
>> khugepaged_do_scan().
> I see. I miss it. but why the new_page need to be release all the way.
AFAIK to support preallocation and reusal of preallocated page for
collapse attempt in different pmd. It only works for !NUMA so it's
likely not worth all the trouble and complicated code, so I wouldn't be
opposed to simplifying this.
> I do not see the count increment when scan success. it save the memory,
> only when page fault happen.
I don't understand what you mean here?
> Thanks
> zhongjiang
>>> ---
>>> mm/khugepaged.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>> index 7cb9c88..586b1f1 100644
>>> --- a/mm/khugepaged.c
>>> +++ b/mm/khugepaged.c
>>> @@ -1082,6 +1082,8 @@ static void collapse_huge_page(struct mm_struct *mm,
>>> up_write(&mm->mmap_sem);
>>> out_nolock:
>>> trace_mm_collapse_huge_page(mm, isolated, result);
>>> + if (page != NULL && result != SCAN_SUCCEED)
>>> + put_page(new_page);
>>> return;
>>> out:
>>> mem_cgroup_cancel_charge(new_page, memcg, true);
>>> @@ -1555,6 +1557,8 @@ static void collapse_shmem(struct mm_struct *mm,
>>> }
>>> out:
>>> VM_BUG_ON(!list_empty(&pagelist));
>>> + if (page != NULL && result != SCAN_SUCCEED)
>>> + put_page(new_page);
>>> /* TODO: tracepoints */
>>> }
>>>
>>>
>>
>> .
>>
>
>
--
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 4/4] dax: Fix data corruption when fault races with write
From: Jan Kara @ 2017-05-09 12:18 UTC (permalink / raw)
To: Andrew Morton
Cc: Ross Zwisler, Dan Williams, linux-fsdevel, linux-ext4,
linux-nvdimm, linux-mm, Jan Kara, stable
In-Reply-To: <20170509121837.26153-1-jack@suse.cz>
Currently DAX read fault can race with write(2) in the following way:
CPU1 - write(2) CPU2 - read fault
dax_iomap_pte_fault()
->iomap_begin() - sees hole
dax_iomap_rw()
iomap_apply()
->iomap_begin - allocates blocks
dax_iomap_actor()
invalidate_inode_pages2_range()
- there's nothing to invalidate
grab_mapping_entry()
- we add zero page in the radix tree
and map it to page tables
The result is that hole page is mapped into page tables (and thus zeros
are seen in mmap) while file has data written in that place.
Fix the problem by locking exception entry before mapping blocks for the
fault. That way we are sure invalidate_inode_pages2_range() call for
racing write will either block on entry lock waiting for the fault to
finish (and unmap stale page tables after that) or read fault will see
already allocated blocks by write(2).
Fixes: 9f141d6ef6258a3a37a045842d9ba7e68f368956
CC: stable@vger.kernel.org
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/dax.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/fs/dax.c b/fs/dax.c
index 72853669a356..f5071249d456 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -1124,23 +1124,23 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
if ((vmf->flags & FAULT_FLAG_WRITE) && !vmf->cow_page)
flags |= IOMAP_WRITE;
+ entry = grab_mapping_entry(mapping, vmf->pgoff, 0);
+ if (IS_ERR(entry))
+ return dax_fault_return(PTR_ERR(entry));
+
/*
* Note that we don't bother to use iomap_apply here: DAX required
* the file system block size to be equal the page size, which means
* that we never have to deal with more than a single extent here.
*/
error = ops->iomap_begin(inode, pos, PAGE_SIZE, flags, &iomap);
- if (error)
- return dax_fault_return(error);
- if (WARN_ON_ONCE(iomap.offset + iomap.length < pos + PAGE_SIZE)) {
- vmf_ret = dax_fault_return(-EIO); /* fs corruption? */
- goto finish_iomap;
+ if (error) {
+ vmf_ret = dax_fault_return(error);
+ goto unlock_entry;
}
-
- entry = grab_mapping_entry(mapping, vmf->pgoff, 0);
- if (IS_ERR(entry)) {
- vmf_ret = dax_fault_return(PTR_ERR(entry));
- goto finish_iomap;
+ if (WARN_ON_ONCE(iomap.offset + iomap.length < pos + PAGE_SIZE)) {
+ error = -EIO; /* fs corruption? */
+ goto error_finish_iomap;
}
sector = dax_iomap_sector(&iomap, pos);
@@ -1162,13 +1162,13 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
}
if (error)
- goto error_unlock_entry;
+ goto error_finish_iomap;
__SetPageUptodate(vmf->cow_page);
vmf_ret = finish_fault(vmf);
if (!vmf_ret)
vmf_ret = VM_FAULT_DONE_COW;
- goto unlock_entry;
+ goto finish_iomap;
}
switch (iomap.type) {
@@ -1188,7 +1188,7 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
case IOMAP_HOLE:
if (!(vmf->flags & FAULT_FLAG_WRITE)) {
vmf_ret = dax_load_hole(mapping, &entry, vmf);
- goto unlock_entry;
+ goto finish_iomap;
}
/*FALLTHRU*/
default:
@@ -1197,10 +1197,8 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
break;
}
- error_unlock_entry:
+ error_finish_iomap:
vmf_ret = dax_fault_return(error) | major;
- unlock_entry:
- put_locked_mapping_entry(mapping, vmf->pgoff, entry);
finish_iomap:
if (ops->iomap_end) {
int copied = PAGE_SIZE;
@@ -1215,6 +1213,8 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
*/
ops->iomap_end(inode, pos, PAGE_SIZE, copied, flags, &iomap);
}
+ unlock_entry:
+ put_locked_mapping_entry(mapping, vmf->pgoff, entry);
return vmf_ret;
}
--
2.12.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>
^ permalink raw reply related
* [PATCH 1/4] dax: prevent invalidation of mapped DAX entries
From: Jan Kara @ 2017-05-09 12:18 UTC (permalink / raw)
To: Andrew Morton
Cc: Ross Zwisler, Dan Williams, linux-fsdevel, linux-ext4,
linux-nvdimm, linux-mm, [4.10+], Jan Kara
In-Reply-To: <20170509121837.26153-1-jack@suse.cz>
From: Ross Zwisler <ross.zwisler@linux.intel.com>
dax_invalidate_mapping_entry() currently removes DAX exceptional entries
only if they are clean and unlocked. This is done via:
invalidate_mapping_pages()
invalidate_exceptional_entry()
dax_invalidate_mapping_entry()
However, for page cache pages removed in invalidate_mapping_pages() there
is an additional criteria which is that the page must not be mapped. This
is noted in the comments above invalidate_mapping_pages() and is checked in
invalidate_inode_page().
For DAX entries this means that we can can end up in a situation where a
DAX exceptional entry, either a huge zero page or a regular DAX entry,
could end up mapped but without an associated radix tree entry. This is
inconsistent with the rest of the DAX code and with what happens in the
page cache case.
We aren't able to unmap the DAX exceptional entry because according to its
comments invalidate_mapping_pages() isn't allowed to block, and
unmap_mapping_range() takes a write lock on the mapping->i_mmap_rwsem.
Since we essentially never have unmapped DAX entries to evict from the
radix tree, just remove dax_invalidate_mapping_entry().
Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Fixes: c6dcf52c23d2 ("mm: Invalidate DAX radix tree entries only if appropriate")
Reported-by: Jan Kara <jack@suse.cz>
Cc: <stable@vger.kernel.org> [4.10+]
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/dax.c | 29 -----------------------------
include/linux/dax.h | 1 -
mm/truncate.c | 9 +++------
3 files changed, 3 insertions(+), 36 deletions(-)
diff --git a/fs/dax.c b/fs/dax.c
index 6433650be833..565c2ea63135 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -507,35 +507,6 @@ int dax_delete_mapping_entry(struct address_space *mapping, pgoff_t index)
}
/*
- * Invalidate exceptional DAX entry if easily possible. This handles DAX
- * entries for invalidate_inode_pages() so we evict the entry only if we can
- * do so without blocking.
- */
-int dax_invalidate_mapping_entry(struct address_space *mapping, pgoff_t index)
-{
- int ret = 0;
- void *entry, **slot;
- struct radix_tree_root *page_tree = &mapping->page_tree;
-
- spin_lock_irq(&mapping->tree_lock);
- entry = __radix_tree_lookup(page_tree, index, NULL, &slot);
- if (!entry || !radix_tree_exceptional_entry(entry) ||
- slot_locked(mapping, slot))
- goto out;
- if (radix_tree_tag_get(page_tree, index, PAGECACHE_TAG_DIRTY) ||
- radix_tree_tag_get(page_tree, index, PAGECACHE_TAG_TOWRITE))
- goto out;
- radix_tree_delete(page_tree, index);
- mapping->nrexceptional--;
- ret = 1;
-out:
- spin_unlock_irq(&mapping->tree_lock);
- if (ret)
- dax_wake_mapping_entry_waiter(mapping, index, entry, true);
- return ret;
-}
-
-/*
* Invalidate exceptional DAX entry if it is clean.
*/
int dax_invalidate_mapping_entry_sync(struct address_space *mapping,
diff --git a/include/linux/dax.h b/include/linux/dax.h
index d8a3dc042e1c..f8e1833f81f6 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -41,7 +41,6 @@ ssize_t dax_iomap_rw(struct kiocb *iocb, struct iov_iter *iter,
int dax_iomap_fault(struct vm_fault *vmf, enum page_entry_size pe_size,
const struct iomap_ops *ops);
int dax_delete_mapping_entry(struct address_space *mapping, pgoff_t index);
-int dax_invalidate_mapping_entry(struct address_space *mapping, pgoff_t index);
int dax_invalidate_mapping_entry_sync(struct address_space *mapping,
pgoff_t index);
void dax_wake_mapping_entry_waiter(struct address_space *mapping,
diff --git a/mm/truncate.c b/mm/truncate.c
index 83a059e8cd1d..706cff171a15 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -67,17 +67,14 @@ static void truncate_exceptional_entry(struct address_space *mapping,
/*
* Invalidate exceptional entry if easily possible. This handles exceptional
- * entries for invalidate_inode_pages() so for DAX it evicts only unlocked and
- * clean entries.
+ * entries for invalidate_inode_pages().
*/
static int invalidate_exceptional_entry(struct address_space *mapping,
pgoff_t index, void *entry)
{
- /* Handled by shmem itself */
- if (shmem_mapping(mapping))
+ /* Handled by shmem itself, or for DAX we do nothing. */
+ if (shmem_mapping(mapping) || dax_mapping(mapping))
return 1;
- if (dax_mapping(mapping))
- return dax_invalidate_mapping_entry(mapping, index);
clear_shadow_entry(mapping, index, entry);
return 1;
}
--
2.12.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>
^ permalink raw reply related
* [PATCH 2/4] mm: Fix data corruption due to stale mmap reads
From: Jan Kara @ 2017-05-09 12:18 UTC (permalink / raw)
To: Andrew Morton
Cc: Ross Zwisler, Dan Williams, linux-fsdevel, linux-ext4,
linux-nvdimm, linux-mm, Jan Kara, stable
In-Reply-To: <20170509121837.26153-1-jack@suse.cz>
Currently, we didn't invalidate page tables during
invalidate_inode_pages2() for DAX. That could result in e.g. 2MiB zero
page being mapped into page tables while there were already underlying
blocks allocated and thus data seen through mmap were different from
data seen by read(2). The following sequence reproduces the problem:
- open an mmap over a 2MiB hole
- read from a 2MiB hole, faulting in a 2MiB zero page
- write to the hole with write(3p). The write succeeds but we
incorrectly leave the 2MiB zero page mapping intact.
- via the mmap, read the data that was just written. Since the zero
page mapping is still intact we read back zeroes instead of the new
data.
Fix the problem by unconditionally calling
invalidate_inode_pages2_range() in dax_iomap_actor() for new block
allocations and by properly invalidating page tables in
invalidate_inode_pages2_range() for DAX mappings.
Fixes: c6dcf52c23d2d3fb5235cec42d7dd3f786b87d55
CC: stable@vger.kernel.org
Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/dax.c | 2 +-
mm/truncate.c | 12 +++++++++++-
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/fs/dax.c b/fs/dax.c
index 565c2ea63135..72853669a356 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -1003,7 +1003,7 @@ dax_iomap_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
* into page tables. We have to tear down these mappings so that data
* written by write(2) is visible in mmap.
*/
- if ((iomap->flags & IOMAP_F_NEW) && inode->i_mapping->nrpages) {
+ if (iomap->flags & IOMAP_F_NEW) {
invalidate_inode_pages2_range(inode->i_mapping,
pos >> PAGE_SHIFT,
(end - 1) >> PAGE_SHIFT);
diff --git a/mm/truncate.c b/mm/truncate.c
index 706cff171a15..6479ed2afc53 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -686,7 +686,17 @@ int invalidate_inode_pages2_range(struct address_space *mapping,
cond_resched();
index++;
}
-
+ /*
+ * For DAX we invalidate page tables after invalidating radix tree. We
+ * could invalidate page tables while invalidating each entry however
+ * that would be expensive. And doing range unmapping before doesn't
+ * work as we have no cheap way to find whether radix tree entry didn't
+ * get remapped later.
+ */
+ if (dax_mapping(mapping)) {
+ unmap_mapping_range(mapping, (loff_t)start << PAGE_SHIFT,
+ (loff_t)(end - start + 1) << PAGE_SHIFT, 0);
+ }
out:
cleancache_invalidate_inode(mapping);
return ret;
--
2.12.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>
^ permalink raw reply related
* [PATCH 3/4] ext4: Return back to starting transaction in ext4_dax_huge_fault()
From: Jan Kara @ 2017-05-09 12:18 UTC (permalink / raw)
To: Andrew Morton
Cc: Ross Zwisler, Dan Williams, linux-fsdevel, linux-ext4,
linux-nvdimm, linux-mm, Jan Kara, stable
In-Reply-To: <20170509121837.26153-1-jack@suse.cz>
DAX will return to locking exceptional entry before mapping blocks
for a page fault to fix possible races with concurrent writes. To avoid
lock inversion between exceptional entry lock and transaction start,
start the transaction already in ext4_dax_huge_fault().
Fixes: 9f141d6ef6258a3a37a045842d9ba7e68f368956
CC: stable@vger.kernel.org
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/ext4/file.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index cefa9835f275..831fd6beebf0 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -257,6 +257,7 @@ static int ext4_dax_huge_fault(struct vm_fault *vmf,
enum page_entry_size pe_size)
{
int result;
+ handle_t *handle = NULL;
struct inode *inode = file_inode(vmf->vma->vm_file);
struct super_block *sb = inode->i_sb;
bool write = vmf->flags & FAULT_FLAG_WRITE;
@@ -264,12 +265,24 @@ static int ext4_dax_huge_fault(struct vm_fault *vmf,
if (write) {
sb_start_pagefault(sb);
file_update_time(vmf->vma->vm_file);
+ down_read(&EXT4_I(inode)->i_mmap_sem);
+ handle = ext4_journal_start_sb(sb, EXT4_HT_WRITE_PAGE,
+ EXT4_DATA_TRANS_BLOCKS(sb));
+ } else {
+ down_read(&EXT4_I(inode)->i_mmap_sem);
}
- down_read(&EXT4_I(inode)->i_mmap_sem);
- result = dax_iomap_fault(vmf, pe_size, &ext4_iomap_ops);
- up_read(&EXT4_I(inode)->i_mmap_sem);
- if (write)
+ if (!IS_ERR(handle))
+ result = dax_iomap_fault(vmf, pe_size, &ext4_iomap_ops);
+ else
+ result = VM_FAULT_SIGBUS;
+ if (write) {
+ if (!IS_ERR(handle))
+ ext4_journal_stop(handle);
+ up_read(&EXT4_I(inode)->i_mmap_sem);
sb_end_pagefault(sb);
+ } else {
+ up_read(&EXT4_I(inode)->i_mmap_sem);
+ }
return result;
}
--
2.12.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>
^ permalink raw reply related
* [PATCH 0/4 v3] mm,dax: Fix data corruption due to mmap inconsistency
From: Jan Kara @ 2017-05-09 12:18 UTC (permalink / raw)
To: Andrew Morton
Cc: Ross Zwisler, Dan Williams, linux-fsdevel, linux-ext4,
linux-nvdimm, linux-mm, Jan Kara
Hello,
this series fixes data corruption that can happen for DAX mounts when
page faults race with write(2) and as a result page tables get out of sync
with block mappings in the filesystem and thus data seen through mmap is
different from data seen through read(2).
The series passes testing with t_mmap_stale test program from Ross and also
other mmap related tests on DAX filesystem.
Andrew, can you please merge these patches? Thanks!
Changes since v2:
* Added reviewed-by tag from Ross
Changes since v1:
* Improved performance of unmapping pages
* Changed fault locking to fix another write vs fault race
Honza
--
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 0/4] RFC - Coherent Device Memory (Not for inclusion)
From: Michal Hocko @ 2017-05-09 11:36 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Balbir Singh, linux-mm, akpm, khandual, aneesh.kumar, paulmck,
srikar, haren, jglisse, mgorman, arbab, vbabka, cl
In-Reply-To: <1493999822.25766.397.camel@kernel.crashing.org>
On Fri 05-05-17 17:57:02, Benjamin Herrenschmidt wrote:
> On Fri, 2017-05-05 at 16:52 +0200, Michal Hocko wrote:
> >
> > This sounds pretty much like a HW specific details which is not the
> > right criterion to design general CDM around.
>
> Which is why I don't see what's the problem with simply making this
> a hot-plugged NUMA node, since it's basically what it is with a
> "different" kind of CPU, possibly covered with a CMA, which provides
> both some isolation and the ability to do large physical allocations
> for applications who chose to use the legacy programming interfaces and
> manually control the memory.
>
> Then, the "issues" with things like reclaim, autonuma can be handled
> with policy tunables. Possibly node attributes.
>
> It seems to me that such a model fits well in the picture where we are
> heading not just with GPUs, but with OpenCAPI based memory, CCIX or
> other similar technologies that can provide memory possibly with co-
> located acceleration devices.
>
> It also mostly already just work.
But this is not what the CDM as proposed here is about AFAIU. It is
argued this is not a _normal_ cpuless node and it neads tweak here and
there. And that is my main objection about. I do not mind if the memory
is presented as a hotplugable cpuless memory node. I just do not want it
to be any more special than cpuless nodes are already.
> > So let me repeat the fundamental question. Is the only difference from
> > cpuless nodes the fact that the node should be invisible to processes
> > unless they specify an explicit node mask?
>
> It would be *preferable* that it is.
>
> It's not necessarily an absolute requirement as long as what lands
> there can be kicked out. However the system would potentially be
> performing poorly if too much unrelated stuff lands on the GPU memory
> as it has a much higher latency.
This is a general concern for many cpuless NUMA node systems. You have
to pay for the suboptimal performance when accessing that memory. And
you have means to cope with that.
> Due to the nature of GPUs (and possibly other such accelerators but not
> necessarily all of them), that memory is also more likely to fail. GPUs
> crash often. However that isn't necessarily true of OpenCAPI devices or
> CCIX.
>
> This is the kind of attributes of the memory (quality ?) that can be
> provided by the driver that is putting it online. We can then
> orthogonally decide how we chose (or not) to take those into account,
> either in the default mm algorithms or from explicit policy mechanisms
> set from userspace, but the latter is often awkward and never done
> right.
The first adds maintain costs all over the place and just looking at
what become of memory policies and cpusets makes me cry. I definitely do
not want more special casing on top (and just to make it clear a special
N_MEMORY_$FOO falls into the same category).
[...]
> > Moreover cpusets already support exclusive numa nodes AFAIR.
>
> Which implies that the user would have to do epxlciit cpuset
> manipulations for the system to work right ? Most user wouldn't and the
> rsult is that most user would have badly working systems. That's almost
> always what happens when we chose to bounce *all* policy decision to
> the user without the kernel attempting to have some kind of semi-sane
> default.
I would argue that this is the case for cpuless numa nodes already.
Users should better know what they are doing when using such a
specialized HW. And that includes a specialized configuration.
[...]
--
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 v2] mm: fix the memory leak after collapsing the huge page fails
From: Vlastimil Babka @ 2017-05-09 11:34 UTC (permalink / raw)
To: zhongjiang, akpm; +Cc: kirill.shutemov, hannes, mgorman, linux-mm
In-Reply-To: <1494327305-835-1-git-send-email-zhongjiang@huawei.com>
On 05/09/2017 12:55 PM, zhongjiang wrote:
> From: zhong jiang <zhongjiang@huawei.com>
>
> Current, when we prepare a huge page to collapse, due to some
> reasons, it can fail to collapse. At the moment, we should
> release the preallocate huge page.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
Hmm, scratch that, there's no memory leak. The pointer to new_page is
stored in *hpage, and put_page() is called all the way up in
khugepaged_do_scan().
> ---
> mm/khugepaged.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 7cb9c88..586b1f1 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -1082,6 +1082,8 @@ static void collapse_huge_page(struct mm_struct *mm,
> up_write(&mm->mmap_sem);
> out_nolock:
> trace_mm_collapse_huge_page(mm, isolated, result);
> + if (page != NULL && result != SCAN_SUCCEED)
> + put_page(new_page);
> return;
> out:
> mem_cgroup_cancel_charge(new_page, memcg, true);
> @@ -1555,6 +1557,8 @@ static void collapse_shmem(struct mm_struct *mm,
> }
> out:
> VM_BUG_ON(!list_empty(&pagelist));
> + if (page != NULL && result != SCAN_SUCCEED)
> + put_page(new_page);
> /* TODO: tracepoints */
> }
>
>
--
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 v2] mm: fix the memory leak after collapsing the huge page fails
From: zhongjiang @ 2017-05-09 10:55 UTC (permalink / raw)
To: akpm, vbabka; +Cc: kirill.shutemov, hannes, mgorman, linux-mm
From: zhong jiang <zhongjiang@huawei.com>
Current, when we prepare a huge page to collapse, due to some
reasons, it can fail to collapse. At the moment, we should
release the preallocate huge page.
Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
mm/khugepaged.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 7cb9c88..586b1f1 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1082,6 +1082,8 @@ static void collapse_huge_page(struct mm_struct *mm,
up_write(&mm->mmap_sem);
out_nolock:
trace_mm_collapse_huge_page(mm, isolated, result);
+ if (page != NULL && result != SCAN_SUCCEED)
+ put_page(new_page);
return;
out:
mem_cgroup_cancel_charge(new_page, memcg, true);
@@ -1555,6 +1557,8 @@ static void collapse_shmem(struct mm_struct *mm,
}
out:
VM_BUG_ON(!list_empty(&pagelist));
+ if (page != NULL && result != SCAN_SUCCEED)
+ put_page(new_page);
/* TODO: tracepoints */
}
--
1.8.3.1
--
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: [PATCH] mm, vmalloc: properly track vmalloc users
From: Michal Hocko @ 2017-05-09 11:19 UTC (permalink / raw)
To: Andrew Morton
Cc: kbuild-all, kbuild test robot, Vlastimil Babka, linux-mm, LKML
In-Reply-To: <20170503063750.GC1236@dhcp22.suse.cz>
On Wed 03-05-17 08:37:50, Michal Hocko wrote:
> From 33a6239135cb444654f48d5e942e7f34898e24ea Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.com>
> Date: Tue, 2 May 2017 11:18:29 +0200
> Subject: [PATCH] mm, vmalloc: properly track vmalloc users
>
> __vmalloc_node_flags used to be static inline but this has changed by
> "mm: introduce kv[mz]alloc helpers" because kvmalloc_node needs to use
> it as well and the code is outside of the vmalloc proper. I haven't
> realized that changing this will lead to a subtle bug though. The
> function is responsible to track the caller as well. This caller is
> then printed by /proc/vmallocinfo. If __vmalloc_node_flags is not inline
> then we would get only direct users of __vmalloc_node_flags as callers
> (e.g. v[mz]alloc) which reduces usefulness of this debugging feature
> considerably. It simply doesn't help to see that the given range belongs
> to vmalloc as a caller:
> 0xffffc90002c79000-0xffffc90002c7d000 16384 vmalloc+0x16/0x18 pages=3 vmalloc N0=3
> 0xffffc90002c81000-0xffffc90002c85000 16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
> 0xffffc90002c8d000-0xffffc90002c91000 16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
> 0xffffc90002c95000-0xffffc90002c99000 16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
>
> We really want to catch the _caller_ of the vmalloc function. Fix this
> issue by making __vmalloc_node_flags static inline again and export
> __vmalloc_node_flags_caller for kvmalloc_node().
>
The "mm: introduce kv[mz]alloc helpers" got merged in the mean time so I
believe the patch should mention
Fixes: a7c3e901a46f ("mm: introduce kv[mz]alloc helpers")
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
> include/linux/vmalloc.h | 16 +++++++++++++++-
> mm/util.c | 3 ++-
> mm/vmalloc.c | 8 +++++++-
> 3 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index 46991ad3ddd5..4a0fabeb1e92 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -80,7 +80,21 @@ extern void *__vmalloc_node_range(unsigned long size, unsigned long align,
> unsigned long start, unsigned long end, gfp_t gfp_mask,
> pgprot_t prot, unsigned long vm_flags, int node,
> const void *caller);
> -extern void *__vmalloc_node_flags(unsigned long size, int node, gfp_t flags);
> +#ifndef CONFIG_MMU
> +extern void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags);
> +static inline void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags, void* caller)
> +{
> + return __vmalloc_node_flags(size, node, flags);
> +}
> +#else
> +/*
> + * We really want to have this inlined due to caller tracking. This
> + * function is used by the highlevel vmalloc apis and so we want to track
> + * their callers and inlining will achieve that.
> + */
> +extern void *__vmalloc_node_flags_caller(unsigned long size,
> + int node, gfp_t flags, void* caller);
> +#endif
>
> extern void vfree(const void *addr);
> extern void vfree_atomic(const void *addr);
> diff --git a/mm/util.c b/mm/util.c
> index 3022051da938..c35e5870921d 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -380,7 +380,8 @@ void *kvmalloc_node(size_t size, gfp_t flags, int node)
> if (ret || size <= PAGE_SIZE)
> return ret;
>
> - return __vmalloc_node_flags(size, node, flags);
> + return __vmalloc_node_flags_caller(size, node, flags,
> + __builtin_return_address(0));
> }
> EXPORT_SYMBOL(kvmalloc_node);
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 65912eb93a2c..1a97d4a31406 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1809,13 +1809,19 @@ void *__vmalloc(unsigned long size, gfp_t gfp_mask, pgprot_t prot)
> }
> EXPORT_SYMBOL(__vmalloc);
>
> -void *__vmalloc_node_flags(unsigned long size,
> +static inline void *__vmalloc_node_flags(unsigned long size,
> int node, gfp_t flags)
> {
> return __vmalloc_node(size, 1, flags, PAGE_KERNEL,
> node, __builtin_return_address(0));
> }
>
> +
> +void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags, void *caller)
> +{
> + return __vmalloc_node(size, 1, flags, PAGE_KERNEL, node, caller);
> +}
> +
> /**
> * vmalloc - allocate virtually contiguous memory
> * @size: allocation size
> --
> 2.11.0
>
> --
> Michal Hocko
> SUSE Labs
--
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
* [PATCH 1/1 v2] Rework slab bitmasks
From: Igor Stoppa @ 2017-05-09 11:04 UTC (permalink / raw)
To: mhocko; +Cc: linux-mm, Igor Stoppa
In-Reply-To: <20170509110448.7872-1-igor.stoppa@huawei.com>
The bitmasks defined in the slab header can be made more readable by
using the BIT() macro.
Furthermore, several conditional definitions can be collapsed, by
expressing their value as a function of the configuration parameter that
controles them, using the macro IS_ENABLED().
Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>
---
include/linux/slab.h | 71 +++++++++++++++++++++++-----------------------------
1 file changed, 31 insertions(+), 40 deletions(-)
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 3c37a8c..6103bc4 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -15,18 +15,24 @@
#include <linux/types.h>
#include <linux/workqueue.h>
+#define __BIT_VL(vl, shift) (((unsigned long)(vl != 0)) << (shift))
+#define BIT_VL(vl, shift) __BIT_VL(vl, shift)
+#define BIT_CFG(cfg, shift) __BIT_VL(IS_ENABLED(CONFIG_##cfg), shift)
+#define BIT_DBG(dbg, shift) BIT_CFG(DEBUG_##dbg, shift)
+#define BIT_DBG_SLAB(shift) BIT_DBG(SLAB, shift)
/*
* Flags to pass to kmem_cache_create().
* The ones marked DEBUG are only valid if CONFIG_DEBUG_SLAB is set.
*/
-#define SLAB_CONSISTENCY_CHECKS 0x00000100UL /* DEBUG: Perform (expensive) checks on alloc/free */
-#define SLAB_RED_ZONE 0x00000400UL /* DEBUG: Red zone objs in a cache */
-#define SLAB_POISON 0x00000800UL /* DEBUG: Poison objects */
-#define SLAB_HWCACHE_ALIGN 0x00002000UL /* Align objs on cache lines */
-#define SLAB_CACHE_DMA 0x00004000UL /* Use GFP_DMA memory */
-#define SLAB_STORE_USER 0x00010000UL /* DEBUG: Store the last owner for bug hunting */
-#define SLAB_PANIC 0x00040000UL /* Panic if kmem_cache_create() fails */
+
+#define SLAB_CONSISTENCY_CHECKS BIT_DBG_SLAB(8) /* Perform (expensive) checks on alloc/free */
+#define SLAB_RED_ZONE BIT_DBG_SLAB(10) /* Red zone objs in a cache */
+#define SLAB_POISON BIT_DBG_SLAB(11) /* Poison objects */
+#define SLAB_HWCACHE_ALIGN BIT(13) /* Align objs on cache lines */
+#define SLAB_CACHE_DMA BIT(14) /* Use GFP_DMA memory */
+#define SLAB_STORE_USER BIT_DBG_SLAB(16) /* Store the last owner for bug hunting */
+#define SLAB_PANIC BIT(18) /* Panic if kmem_cache_create() fails */
/*
* SLAB_DESTROY_BY_RCU - **WARNING** READ THIS!
*
@@ -62,44 +68,29 @@
* rcu_read_lock before reading the address, then rcu_read_unlock after
* taking the spinlock within the structure expected at that address.
*/
-#define SLAB_DESTROY_BY_RCU 0x00080000UL /* Defer freeing slabs to RCU */
-#define SLAB_MEM_SPREAD 0x00100000UL /* Spread some memory over cpuset */
-#define SLAB_TRACE 0x00200000UL /* Trace allocations and frees */
+#define SLAB_DESTROY_BY_RCU BIT(19) /* Defer freeing slabs to RCU */
+#define SLAB_MEM_SPREAD BIT(20) /* Spread some memory over cpuset */
+#define SLAB_TRACE BIT(21) /* Trace allocations and frees */
/* Flag to prevent checks on free */
-#ifdef CONFIG_DEBUG_OBJECTS
-# define SLAB_DEBUG_OBJECTS 0x00400000UL
-#else
-# define SLAB_DEBUG_OBJECTS 0x00000000UL
-#endif
+# define SLAB_DEBUG_OBJECTS BIT_DBG(OBJECTS, 22)
-#define SLAB_NOLEAKTRACE 0x00800000UL /* Avoid kmemleak tracing */
+#define SLAB_NOLEAKTRACE BIT(23) /* Avoid kmemleak tracing */
/* Don't track use of uninitialized memory */
-#ifdef CONFIG_KMEMCHECK
-# define SLAB_NOTRACK 0x01000000UL
-#else
-# define SLAB_NOTRACK 0x00000000UL
-#endif
-#ifdef CONFIG_FAILSLAB
-# define SLAB_FAILSLAB 0x02000000UL /* Fault injection mark */
-#else
-# define SLAB_FAILSLAB 0x00000000UL
-#endif
-#if defined(CONFIG_MEMCG) && !defined(CONFIG_SLOB)
-# define SLAB_ACCOUNT 0x04000000UL /* Account to memcg */
-#else
-# define SLAB_ACCOUNT 0x00000000UL
-#endif
+# define SLAB_NOTRACK BIT_CFG(KMEMCHECK, 24)
-#ifdef CONFIG_KASAN
-#define SLAB_KASAN 0x08000000UL
-#else
-#define SLAB_KASAN 0x00000000UL
-#endif
+/* Fault injection mark */
+# define SLAB_FAILSLAB BIT_CFG(FAILSLAB, 25)
+
+/* Account to memcg */
+# define SLAB_ACCOUNT BIT_VL(IS_ENABLED(CONFIG_MEMCG) && \
+ !IS_ENABLED(CONFIG_SLOB), 26)
+
+#define SLAB_KASAN BIT_CFG(KASAN, 27)
/* The following flags affect the page allocator grouping pages by mobility */
-#define SLAB_RECLAIM_ACCOUNT 0x00020000UL /* Objects are reclaimable */
+#define SLAB_RECLAIM_ACCOUNT BIT(17) /* Objects are reclaimable */
#define SLAB_TEMPORARY SLAB_RECLAIM_ACCOUNT /* Objects are short-lived */
/*
* ZERO_SIZE_PTR will be returned for zero sized kmalloc requests.
@@ -246,9 +237,9 @@ static inline const char *__check_heap_object(const void *ptr,
#endif
/* Maximum allocatable size */
-#define KMALLOC_MAX_SIZE (1UL << KMALLOC_SHIFT_MAX)
+#define KMALLOC_MAX_SIZE BIT(KMALLOC_SHIFT_MAX)
/* Maximum size for which we actually use a slab cache */
-#define KMALLOC_MAX_CACHE_SIZE (1UL << KMALLOC_SHIFT_HIGH)
+#define KMALLOC_MAX_CACHE_SIZE BIT(KMALLOC_SHIFT_HIGH)
/* Maximum order allocatable via the slab allocagtor */
#define KMALLOC_MAX_ORDER (KMALLOC_SHIFT_MAX - PAGE_SHIFT)
@@ -256,7 +247,7 @@ static inline const char *__check_heap_object(const void *ptr,
* Kmalloc subsystem.
*/
#ifndef KMALLOC_MIN_SIZE
-#define KMALLOC_MIN_SIZE (1 << KMALLOC_SHIFT_LOW)
+#define KMALLOC_MIN_SIZE BIT(KMALLOC_SHIFT_LOW)
#endif
/*
--
2.9.3
--
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/1 v2] mm: Use BIT macro in SLAB bitmaps
From: Igor Stoppa @ 2017-05-09 11:04 UTC (permalink / raw)
To: mhocko; +Cc: linux-mm, Igor Stoppa
The file include/linux/slab.h can be simplified by moving to use the
macro BIT() and making other bitmaps depend on their correspective
master-toggle configuration option.
checkpatch.pl will generate some warnings about line lenght, but I didn't
want to alter the initial layout, which already sufferend from this
problem.
The previous version was redefining a macro that is used by the exinos bsp.
Igor Stoppa (1):
Rework slab bitmasks
include/linux/slab.h | 71 +++++++++++++++++++++++-----------------------------
1 file changed, 31 insertions(+), 40 deletions(-)
--
2.9.3
--
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: fix the memory leak after collapsing the huge page fails
From: zhong jiang @ 2017-05-09 9:57 UTC (permalink / raw)
To: Vlastimil Babka; +Cc: akpm, mgorman, hannes, kirill.shutemov, linux-mm
In-Reply-To: <7d5fd103-f997-e445-2ce6-2e44deed33d8@suse.cz>
On 2017/5/9 16:42, Vlastimil Babka wrote:
> On 05/09/2017 10:12 AM, zhongjiang wrote:
>> From: zhong jiang <zhongjiang@huawei.com>
>>
>> Current, when we prepare a huge page to collapse, due to some
>> reasons, it can fail to collapse. At the moment, we should
>> release the preallocate huge page.
> Yeah, looks like the leak is there...
>
>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>> ---
>> mm/khugepaged.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 7cb9c88..3f5749e 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -1080,6 +1080,7 @@ static void collapse_huge_page(struct mm_struct *mm,
>> result = SCAN_SUCCEED;
>> out_up_write:
>> up_write(&mm->mmap_sem);
>> + put_page(new_page);
> This doesn't seem correct.
> - the put_page() will be called also on success, so a premature free?
> - the out_nolock: case should be also handled
> - collapse_shmem() seems to have the same problem
>
>> out_nolock:
>> trace_mm_collapse_huge_page(mm, isolated, result);
>> return;
>>
>
> .
>
>Subject: [PATCH v2] mm: fix the memory leak after collapsing the huge page
fails
Current, when we prepare a huge page to collapse, due to some
reasons, it can fail to collapse. At the moment, we should
release the preallocate huge page.
Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
mm/khugepaged.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 7cb9c88..586b1f1 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1082,6 +1082,8 @@ static void collapse_huge_page(struct mm_struct *mm,
up_write(&mm->mmap_sem);
out_nolock:
trace_mm_collapse_huge_page(mm, isolated, result);
+ if (page != NULL && result != SCAN_SUCCEED)
+ put_page(new_page);
return;
out:
mem_cgroup_cancel_charge(new_page, memcg, true);
@@ -1555,6 +1557,8 @@ static void collapse_shmem(struct mm_struct *mm,
}
out:
VM_BUG_ON(!list_empty(&pagelist));
+ if (page != NULL && result != SCAN_SUCCEED)
+ put_page(new_page);
/* TODO: tracepoints */
}
--
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: [PATCH v3 4/4] mm: Adaptive hash table scaling
From: Michal Hocko @ 2017-05-09 9:46 UTC (permalink / raw)
To: Andrew Morton, Pasha Tatashin
Cc: linux-mm, sparclinux, linux-fsdevel, Al Viro
In-Reply-To: <e7c61dec-9d57-957b-7ff5-8247fa51eafb@oracle.com>
On Fri 05-05-17 11:33:36, Pasha Tatashin wrote:
>
>
> On 05/05/2017 09:30 AM, Michal Hocko wrote:
> >On Thu 04-05-17 14:28:51, Pasha Tatashin wrote:
> >>BTW, I am OK with your patch on top of this "Adaptive hash table" patch, but
> >>I do not know what high_limit should be from where HASH_ADAPT will kick in.
> >>128M sound reasonable to you?
> >
> >For simplicity I would just use it unconditionally when no high_limit is
> >set. What would be the problem with that?
>
> Sure, that sounds good.
>
> If you look at current users
> >(and there no new users emerging too often) then most of them just want
> >_some_ scaling. The original one obviously doesn't scale with large
> >machines. Are you OK to fold my change to your patch or you want me to
> >send a separate patch? AFAIK Andrew hasn't posted this patch to Linus
> >yet.
> >
>
> I would like a separate patch because mine has soaked in mm tree for a while
> now.
OK, Andrew tends to fold follow up fixes in his mm tree. But anyway, as
you prefer to have this in a separate patch. Could you add this on top
Andrew? I believe mnt hash tables need a _reasonable_ upper bound but
that is for a separate patch I believe.
---
^ permalink raw reply
* Re: RFC v2: post-init-read-only protection for data allocated dynamically
From: Igor Stoppa @ 2017-05-09 9:38 UTC (permalink / raw)
To: Laura Abbott, Michal Hocko
Cc: linux-mm, linux-kernel, kernel-hardening@lists.openwall.com
In-Reply-To: <b3fab9c3-fa35-eb7b-204c-f85a0d392e12@redhat.com>
On 08/05/17 18:25, Laura Abbott wrote:
> On 05/05/2017 03:42 AM, Igor Stoppa wrote:
>> On 04/05/17 19:49, Laura Abbott wrote:
[...]
> PAGE_SIZE is still 4K/16K/64K but the underlying page table mappings
> may use larger mappings (2MB, 32M, 512M, etc.). The ARM architecture
> has a break-before-make requirement which requires old mappings be
> fully torn down and invalidated to avoid TLB conflicts. This is nearly
> impossible to do correctly on live page tables so the current policy
> is to not break down larger mappings.
ok, but if a system integrator chooses to have the mapping set to 512M
(let's consider a case that is definitely unoptimized), this granularity
will apply to anything that needs to be marked as R/O and is allocated
through linear mapping.
If the issue inherently sits with linear allocation, then maybe the
correct approach is to confirm if linear allocation is really needed, in
the first place.
Maybe not, at least for the type of data I have in mind.
However, that would require changes in the users of the interface,
rather than the interface itself.
I don't see this change as a problem, in general, but OTOH I do not know
yet if there are legitimate reasons to use kmalloc for any
post-init-read-only data.
Of course, if you have any proposal that would solve this problem with
large pages, I'd be interested to know.
Also, for me to better understand the magnitude of the problem, do you
know if there is any specific scenario where larger mappings would be
used/recommended?
The major reason for using larger mapping that I can think of, is to
improve the efficiency of the TLB when under pressure, however I
wouldn't be able to judge how much this can affect the overall
performance a big iron or in a small device.
Do you know if there is any similar case that has to deal with locking
down large pages?
Doesn't the kernel text have potentially a similar risks of leaving
large amount of memory unused?
Is rodata only virtual?
> I'd rather see this designed as being mandatory from the start and then
> provide a mechanism to turn it off if necessary. The uptake and
> coverage from opt-in features tends to be very low based on past experience.
I have nothing against such wish, actually I'd love it, but I'm not sure
I have the answer for it.
Yet, a partial solution is better than nothing, if there is no truly
flexible one.
--
igor
--
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: fix the memory leak after collapsing the huge page fails
From: zhong jiang @ 2017-05-09 9:25 UTC (permalink / raw)
To: Vlastimil Babka; +Cc: akpm, mgorman, hannes, kirill.shutemov, linux-mm
In-Reply-To: <7d5fd103-f997-e445-2ce6-2e44deed33d8@suse.cz>
On 2017/5/9 16:42, Vlastimil Babka wrote:
> On 05/09/2017 10:12 AM, zhongjiang wrote:
>> From: zhong jiang <zhongjiang@huawei.com>
>>
>> Current, when we prepare a huge page to collapse, due to some
>> reasons, it can fail to collapse. At the moment, we should
>> release the preallocate huge page.
> Yeah, looks like the leak is there...
>
>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>> ---
>> mm/khugepaged.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 7cb9c88..3f5749e 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -1080,6 +1080,7 @@ static void collapse_huge_page(struct mm_struct *mm,
>> result = SCAN_SUCCEED;
>> out_up_write:
>> up_write(&mm->mmap_sem);
>> + put_page(new_page);
> This doesn't seem correct.
> - the put_page() will be called also on success, so a premature free?
my God, I forget this.
> - the out_nolock: case should be also handled
khugepage_alloc_page fails . it also will reach out_nolock.
The out_nolock is very mess, need to reconsider.
> - collapse_shmem() seems to have the same problem
yes. I will add the path to fix.
>> out_nolock:
>> trace_mm_collapse_huge_page(mm, isolated, result);
>> return;
>>
>
> .
>
--
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 v2 1/2] mm: Uncharge poisoned pages
From: Michal Hocko @ 2017-05-09 9:18 UTC (permalink / raw)
To: Naoya Horiguchi, akpm@linux-foundation.org
Cc: Andi Kleen, Johannes Weiner, Laurent Dufour,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
Vladimir Davydov
In-Reply-To: <20170508025827.GA4913@hori1.linux.bs1.fc.nec.co.jp>
On Mon 08-05-17 02:58:36, Naoya Horiguchi wrote:
> On Tue, May 02, 2017 at 08:55:07PM +0200, Michal Hocko wrote:
> > On Tue 02-05-17 16:59:30, Laurent Dufour wrote:
> > > On 28/04/2017 15:48, Michal Hocko wrote:
> > [...]
> > > > This is getting quite hairy. What is the expected page count of the
> > > > hwpoison page?
> >
> > OK, so from the quick check of the hwpoison code it seems that the ref
> > count will be > 1 (from get_hwpoison_page).
> >
> > > > I guess we would need to update the VM_BUG_ON in the
> > > > memcg uncharge code to ignore the page count of hwpoison pages if it can
> > > > be arbitrary.
> > >
> > > Based on the experiment I did, page count == 2 when isolate_lru_page()
> > > succeeds, even in the case of a poisoned page.
> >
> > that would make some sense to me. The page should have been already
> > unmapped therefore but memory_failure increases the ref count and 1 is
> > for isolate_lru_page().
>
> # sorry for late reply, I was on holidays last week...
>
> Right, and the refcount taken for memory_failure is not freed after
> memory_failure() returns. unpoison_memory() does free the refcount.
OK, from the charge POV this would be safe because we clear page->memcg
so it wouldn't get uncharged more times.
> > > In my case I think this
> > > is because the page is still used by the process which is calling madvise().
> > >
> > > I'm wondering if I'm looking at the right place. May be the poisoned
> > > page should remain attach to the memory_cgroup until no one is using it.
> > > In that case this means that something should be done when the page is
> > > off-lined... I've to dig further here.
> >
> > No, AFAIU the page will not drop the reference count down to 0 in most
> > cases. Maybe there are some scenarios where this can happen but I would
> > expect that the poisoned page will be mapped and in use most of the time
> > and won't drop down 0. And then we should really uncharge it because it
> > will pin the memcg and make it unfreeable which doesn't seem to be what
> > we want. So does the following work reasonable? Andi, Johannes, what do
> > you think? I cannot say I would be really comfortable touching hwpoison
> > code as I really do not understand the workflow. Maybe we want to move
> > this uncharge down to memory_failure() right before we report success?
>
> memory_failure() can be called for any types of page (including slab or
> any kernel/driver pages), and the reported problem seems happen only on
> in-use user pages, so uncharging in delete_from_lru_cache() as done below
> looks better to me.
Yeah, we do see problems only for LRU/page cache pages but my
understanding is that error_states (e.g. me_kernel for the kernel
memory) might change in the future and then we wouldn't catch the same
bug, no?
> > ---
> > From 8bf0791bcf35996a859b6d33fb5494e5b53de49d Mon Sep 17 00:00:00 2001
> > From: Michal Hocko <mhocko@suse.com>
> > Date: Tue, 2 May 2017 20:32:24 +0200
> > Subject: [PATCH] hwpoison, memcg: forcibly uncharge LRU pages
> >
> > Laurent Dufour has noticed that hwpoinsoned pages are kept charged. In
> > his particular case he has hit a bad_page("page still charged to cgroup")
> > when onlining a hwpoison page.
>
> > While this looks like something that shouldn't
> > happen in the first place because onlining hwpages and returning them to
> > the page allocator makes only little sense it shows a real problem.
> >
> > hwpoison pages do not get freed usually so we do not uncharge them (at
> > least not since 0a31bc97c80c ("mm: memcontrol: rewrite uncharge API")).
> > Each charge pins memcg (since e8ea14cc6ead ("mm: memcontrol: take a css
> > reference for each charged page")) as well and so the mem_cgroup and the
> > associated state will never go away. Fix this leak by forcibly
> > uncharging a LRU hwpoisoned page in delete_from_lru_cache(). We also
> > have to tweak uncharge_list because it cannot rely on zero ref count
> > for these pages.
> >
> > Fixes: 0a31bc97c80c ("mm: memcontrol: rewrite uncharge API")
> > Reported-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
> > Signed-off-by: Michal Hocko <mhocko@suse.com>
>
> Reviewed-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Thanks! I will wait a day or two for Johannes and repost the patch.
Andrew could you drop
http://www.ozlabs.org/~akpm/mmotm/broken-out/mm-uncharge-poisoned-pages.patch
in the mean time, 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
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