Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v1 00/11] mm/kasan: support per-page shadow memory to reduce memory consumption
From: Joonsoo Kim @ 2017-05-24  6:04 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Andrey Ryabinin, Andrew Morton, Alexander Potapenko, kasan-dev,
	linux-mm@kvack.org, LKML, Thomas Gleixner, Ingo Molnar,
	H . Peter Anvin, kernel-team
In-Reply-To: <CACT4Y+bZVJpi++kfMkAc-3pXK165ZQyHaEU_6oN94+qQErJd8A@mail.gmail.com>

On Mon, May 22, 2017 at 08:02:36AM +0200, Dmitry Vyukov wrote:
> On Fri, May 19, 2017 at 3:53 AM, Joonsoo Kim <js1304@gmail.com> wrote:
> > On Wed, May 17, 2017 at 03:17:13PM +0300, Andrey Ryabinin wrote:
> >> On 05/16/2017 04:16 AM, js1304@gmail.com wrote:
> >> > From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> >> >
> >> > Hello, all.
> >> >
> >> > This is an attempt to recude memory consumption of KASAN. Please see
> >> > following description to get the more information.
> >> >
> >> > 1. What is per-page shadow memory
> >> >
> >> > This patch introduces infrastructure to support per-page shadow memory.
> >> > Per-page shadow memory is the same with original shadow memory except
> >> > the granualarity. It's one byte shows the shadow value for the page.
> >> > The purpose of introducing this new shadow memory is to save memory
> >> > consumption.
> >> >
> >> > 2. Problem of current approach
> >> >
> >> > Until now, KASAN needs shadow memory for all the range of the memory
> >> > so the amount of statically allocated memory is so large. It causes
> >> > the problem that KASAN cannot run on the system with hard memory
> >> > constraint. Even if KASAN can run, large memory consumption due to
> >> > KASAN changes behaviour of the workload so we cannot validate
> >> > the moment that we want to check.
> >> >
> >> > 3. How does this patch fix the problem
> >> >
> >> > This patch tries to fix the problem by reducing memory consumption for
> >> > the shadow memory. There are two observations.
> >> >
> >>
> >>
> >> I think that the best way to deal with your problem is to increase shadow scale size.
> >>
> >> You'll need to add tunable to gcc to control shadow size. I expect that gcc has some
> >> places where 8-shadow scale size is hardcoded, but it should be fixable.
> >>
> >> The kernel also have some small amount of code written with KASAN_SHADOW_SCALE_SIZE == 8 in mind,
> >> which should be easy to fix.
> >>
> >> Note that bigger shadow scale size requires bigger alignment of allocated memory and variables.
> >> However, according to comments in gcc/asan.c gcc already aligns stack and global variables and at
> >> 32-bytes boundary.
> >> So we could bump shadow scale up to 32 without increasing current stack consumption.
> >>
> >> On a small machine (1Gb) 1/32 of shadow is just 32Mb which is comparable to yours 30Mb, but I expect it to be
> >> much faster. More importantly, this will require only small amount of simple changes in code, which will be
> >> a *lot* more easier to maintain.
> 
> 
> Interesting option. We never considered increasing scale in user space
> due to performance implications. But the algorithm always supported up
> to 128x scale. Definitely worth considering as an option.

Could you explain me how does increasing scale reduce performance? I
tried to guess the reason but failed.

> 
> 
> > I agree that it is also a good option to reduce memory consumption.
> > Nevertheless, there are two reasons that justifies this patchset.
> >
> > 1) With this patchset, memory consumption isn't increased in
> > proportional to total memory size. Please consider my 4Gb system
> > example on the below. With increasing shadow scale size to 32, memory
> > would be consumed by 128M. However, this patchset consumed 50MB. This
> > difference can be larger if we run KASAN with bigger machine.
> >
> > 2) These two optimization can be applied simulatenously. It is just an
> > orthogonal feature. If shadow scale size is increased to 32, memory
> > consumption will be decreased in case of my patchset, too.
> >
> > Therefore, I think that this patchset is useful in any case.
> 
> It is definitely useful all else being equal. But it does considerably
> increase code size and complexity, which is an important aspect.
> 
> Also note that there is also fixed size quarantine (1/32 of RAM) and
> redzones. Reducing shadow overhead beyond some threshold has
> diminishing returns, because overall overhead will be just dominated
> by quarantine/redzones.

My usecase doesn't use quarantine yet since it uses old version kernel
and quarantine isn't back-ported. But, this 1/32 of RAM for quarantine
also could affect the system and I think that we need a switch to
disable it. In our case, making the feature work is more important
than detecting more bugs.

Redzone is also a good target to make selectable since
error pattern could be changed with different object layout. I
sometimes saw that error disappears if KASAN is enabled. I'm not sure
what causes it, but, in some case, it would be helpful that everything
else than something compulsory is the same with non-KASAN build.

> What's your target devices and constraints? We run KASAN on phones
> today without any issues.

My target devices are a smart TV or embedded system on a car. Usually,
these devices have specific use scenario and memory is managed more
tightly than a phone. I have heard that some system with 1GB memory
cannot run if 128MB is used for KASAN. I'm not sure that 1/32 scale
changes the picture, but, yes, I guess that most of problem will disappear.

> 
> > Note that increasing shadow scale has it's own trade-off. It requires
> > that the size of slab object is aligned to shadow scale. It will
> > increase memory consumption due to slab.
> 
> I've tried to retest your latest change on top of
> http://git.cmpxchg.org/cgit.cgi/linux-mmots.git
> d9cd9c95cc3b2fed0f04d233ebf2f7056741858c, but now this version
> https://codereview.appspot.com/325780043 always crashes during boot
> for me. Report points to zero shadow.

Oops... Maybe, it's due to lack of stale TLB handling on double-free
check in kasan_slab_free(). I fixed it on my version 2 patchset.
And, I also fixed performance problem due to memory allocated by early
allocator(memblock or (no)bootmem).

https://github.com/JoonsooKim/linux/tree/kasan-opt-memory-consumption-v2.0-next-20170511

This branch is based on next-20170511.

Thanks.

--
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] mm/zsmalloc: fix -Wunneeded-internal-declaration warning
From: Nick Desaulniers @ 2017-05-24  5:38 UTC (permalink / raw)
  Cc: md, mka, Nick Desaulniers, Minchan Kim, Nitin Gupta,
	Sergey Senozhatsky, linux-mm, linux-kernel

is_first_page() is only called from the macro VM_BUG_ON_PAGE() which is
only compiled in as a runtime check when CONFIG_DEBUG_VM is set,
otherwise is checked at compile time and not actually compiled in.

Fixes the following warning, found with Clang:

mm/zsmalloc.c:472:12: warning: function 'is_first_page' is not needed and
will not be emitted [-Wunneeded-internal-declaration]
static int is_first_page(struct page *page)
           ^

Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com>
---
 mm/zsmalloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index d41edd28298b..15959d35fc26 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -469,7 +469,7 @@ static bool is_zspage_isolated(struct zspage *zspage)
 	return zspage->isolated;
 }
 
-static int is_first_page(struct page *page)
+static __maybe_unused int is_first_page(struct page *page)
 {
 	return PagePrivate(page);
 }
-- 
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>

^ permalink raw reply related

* Re: mm, we use rcu access task_struct in mm_match_cgroup(), but not use rcu free in free_task_struct()
From: Xishi Qiu @ 2017-05-24  4:40 UTC (permalink / raw)
  To: Michal Hocko, Mel Gorman, Hugh Dickins, Vlastimil Babka,
	Minchan Kim, wencongyang (A), Johannes Weiner, Dmitry Vyukov,
	zhong jiang
  Cc: Linux MM, LKML
In-Reply-To: <5924E4A7.7000601@huawei.com>

On 2017/5/24 9:40, Xishi Qiu wrote:

> Hi, I find we use rcu access task_struct in mm_match_cgroup(), but not use
> rcu free in free_task_struct(), is it right?
> 
> Here is the backtrace.
> 
> PID: 2133   TASK: ffff881fe3353300  CPU: 2   COMMAND: "CPU 15/KVM"
>  #0 [ffff881fe276b528] machine_kexec at ffffffff8105280b
>  #1 [ffff881fe276b588] crash_kexec at ffffffff810f5072
>  #2 [ffff881fe276b658] panic at ffffffff8163e23b
>  #3 [ffff881fe276b6d8] oops_end at ffffffff8164d61b
>  #4 [ffff881fe276b700] die at ffffffff8101872b
>  #5 [ffff881fe276b730] do_general_protection at ffffffff8164cefe
>  #6 [ffff881fe276b760] general_protection at ffffffff8164c7a8
>     [exception RIP: mem_cgroup_from_task+22]
>     RIP: ffffffff811db536  RSP: ffff881fe276b810  RFLAGS: 00010286
>     RAX: 6b6b6b6b6b6b6b6b  RBX: ffffea007f988880  RCX: 0000000000020000
>     RDX: 00000007fa607d67  RSI: 00000007fa607d67  RDI: ffff880fe36d72c0
>     RBP: ffff881fe276b880   R8: 00000007fa607600   R9: a801fd67b3000000
>     R10: 57fdec98cc59ecc0  R11: ffff880fe2e8dbd0  R12: ffffc9001cb74000
>     R13: ffff881fdb8cfda0  R14: ffff881fe2581570  R15: 00000007fa607d67
>     ORIG_RAX: ffffffffffffffff  CS: 0010  SS: 0000
>  #7 [ffff881fe276b810] page_referenced at ffffffff811a6b8a
>  #8 [ffff881fe276b888] shrink_page_list at ffffffff81180994
>  #9 [ffff881fe276b9c0] shrink_inactive_list at ffffffff8118166a
> #10 [ffff881fe276ba88] shrink_lruvec at ffffffff81182135
> #11 [ffff881fe276bb88] shrink_zone at ffffffff81182596
> #12 [ffff881fe276bbe0] do_try_to_free_pages at ffffffff81182a90
> #13 [ffff881fe276bc58] try_to_free_mem_cgroup_pages at ffffffff81182fea
> #14 [ffff881fe276bcf0] mem_cgroup_reclaim at ffffffff811dd8de
> #15 [ffff881fe276bd30] __mem_cgroup_try_charge at ffffffff811ddd9c
> #16 [ffff881fe276bdf0] __mem_cgroup_try_charge_swapin at ffffffff811df62b
> #17 [ffff881fe276be28] mem_cgroup_try_charge_swapin at ffffffff811e0537
> #18 [ffff881fe276be38] handle_mm_fault at ffffffff8119abdd
> #19 [ffff881fe276bec8] __do_page_fault at ffffffff816502d6
> #20 [ffff881fe276bf28] do_page_fault at ffffffff81650603
> #21 [ffff881fe276bf50] page_fault at ffffffff8164c808
>     RIP: 00007fdaba456500  RSP: 00007fdaaba6c978  RFLAGS: 00010246
>     RAX: ffffffffffffffff  RBX: 0000000000000000  RCX: fffffffffffffbd0
>     RDX: 0000000000000000  RSI: 000000000000ae80  RDI: 000000000000002c
>     RBP: 00007fdaaba6c9f0   R8: 0000000000840c70   R9: 00000000000000be
>     R10: 000000007fffffff  R11: 0000000000000246  R12: 0000000003622010
>     R13: 000000000000ae80  R14: 00000000008274e0  R15: 0000000003622010
>     ORIG_RAX: ffffffffffffffff  CS: 0033  SS: 002b
> 
> 
> .
> 



--
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 4/6] powerpc/mm: Add devmap support for ppc64
From: Oliver O'Halloran @ 2017-05-24  2:17 UTC (permalink / raw)
  To: Balbir Singh
  Cc: open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), linux-mm,
	Aneesh Kumar K . V
In-Reply-To: <CAKTCnz=tbEYossD8X5z87UEYCLfz4ah+6hZSDRcnXbDmjRqN+Q@mail.gmail.com>

On Tue, May 23, 2017 at 8:40 PM, Balbir Singh <bsingharora@gmail.com> wrote:
> On Tue, May 23, 2017 at 2:05 PM, Oliver O'Halloran <oohall@gmail.com> wrote:
>> Add support for the devmap bit on PTEs and PMDs for PPC64 Book3S.  This
>> is used to differentiate device backed memory from transparent huge
>> pages since they are handled in more or less the same manner by the core
>> mm code.
>>
>> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
>> ---
>> v1 -> v2: Properly differentiate THP and PMD Devmap entries. The
>> mm core assumes that pmd_trans_huge() and pmd_devmap() are mutually
>> exclusive and v1 had pmd_trans_huge() being true on a devmap pmd.
>>
>> Aneesh, this has been fleshed out substantially since v1. Can you
>> re-review it? Also no explicit gup support is required in this patch
>> since devmap support was added generic GUP as a part of making x86 use
>> the generic version.
>> ---
>>  arch/powerpc/include/asm/book3s/64/hash-64k.h |  2 +-
>>  arch/powerpc/include/asm/book3s/64/pgtable.h  | 37 ++++++++++++++++++++++++++-
>>  arch/powerpc/include/asm/book3s/64/radix.h    |  2 +-
>>  arch/powerpc/mm/hugetlbpage.c                 |  2 +-
>>  arch/powerpc/mm/pgtable-book3s64.c            |  4 +--
>>  arch/powerpc/mm/pgtable-hash64.c              |  4 ++-
>>  arch/powerpc/mm/pgtable-radix.c               |  3 ++-
>>  arch/powerpc/mm/pgtable_64.c                  |  2 +-
>>  8 files changed, 47 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/book3s/64/hash-64k.h b/arch/powerpc/include/asm/book3s/64/hash-64k.h
>> index 9732837aaae8..eaaf613c5347 100644
>> --- a/arch/powerpc/include/asm/book3s/64/hash-64k.h
>> +++ b/arch/powerpc/include/asm/book3s/64/hash-64k.h
>> @@ -180,7 +180,7 @@ static inline void mark_hpte_slot_valid(unsigned char *hpte_slot_array,
>>   */
>>  static inline int hash__pmd_trans_huge(pmd_t pmd)
>>  {
>> -       return !!((pmd_val(pmd) & (_PAGE_PTE | H_PAGE_THP_HUGE)) ==
>> +       return !!((pmd_val(pmd) & (_PAGE_PTE | H_PAGE_THP_HUGE | _PAGE_DEVMAP)) ==
>>                   (_PAGE_PTE | H_PAGE_THP_HUGE));
>>  }
>
> Like Aneesh suggested, I think we can probably skip this check here
>
>>
>> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
>> index 85bc9875c3be..24634e92dd0b 100644
>> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
>> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
>> @@ -79,6 +79,9 @@
>>
>>  #define _PAGE_SOFT_DIRTY       _RPAGE_SW3 /* software: software dirty tracking */
>>  #define _PAGE_SPECIAL          _RPAGE_SW2 /* software: special page */
>> +#define _PAGE_DEVMAP           _RPAGE_SW1
>> +#define __HAVE_ARCH_PTE_DEVMAP
>> +
>>  /*
>>   * Drivers request for cache inhibited pte mapping using _PAGE_NO_CACHE
>>   * Instead of fixing all of them, add an alternate define which
>> @@ -599,6 +602,16 @@ static inline pte_t pte_mkhuge(pte_t pte)
>>         return pte;
>>  }
>>
>> +static inline pte_t pte_mkdevmap(pte_t pte)
>> +{
>> +       return __pte(pte_val(pte) | _PAGE_SPECIAL|_PAGE_DEVMAP);
>> +}
>> +
>> +static inline int pte_devmap(pte_t pte)
>> +{
>> +       return !!(pte_raw(pte) & cpu_to_be64(_PAGE_DEVMAP));
>> +}
>> +
>>  static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>>  {
>>         /* FIXME!! check whether this need to be a conditional */
>> @@ -963,6 +976,9 @@ static inline pte_t *pmdp_ptep(pmd_t *pmd)
>>  #define pmd_mk_savedwrite(pmd) pte_pmd(pte_mk_savedwrite(pmd_pte(pmd)))
>>  #define pmd_clear_savedwrite(pmd)      pte_pmd(pte_clear_savedwrite(pmd_pte(pmd)))
>>
>> +#define pud_pfn(...) (0)
>> +#define pgd_pfn(...) (0)
>> +
>
> Don't get these bits.. why are they zero?

I think that was just hacking stuff until it worked. pud_pfn() needs
to exist for the kernel to build when __HAVE_ARCH_PTE_DEVMAP is set,
but we don't need it to do anything (yet) since pud_pfn() is only used
for handing devmap PUD faults. We currently support those so we will
never hit that code path. pgd_pfn() can die though.

>>  #ifdef CONFIG_HAVE_ARCH_SOFT_DIRTY
>>  #define pmd_soft_dirty(pmd)    pte_soft_dirty(pmd_pte(pmd))
>>  #define pmd_mksoft_dirty(pmd)  pte_pmd(pte_mksoft_dirty(pmd_pte(pmd)))
>> @@ -1137,7 +1153,6 @@ static inline int pmd_move_must_withdraw(struct spinlock *new_pmd_ptl,
>>         return true;
>>  }
>>
>> -
>>  #define arch_needs_pgtable_deposit arch_needs_pgtable_deposit
>>  static inline bool arch_needs_pgtable_deposit(void)
>>  {
>> @@ -1146,6 +1161,26 @@ static inline bool arch_needs_pgtable_deposit(void)
>>         return true;
>>  }
>>
>> +static inline pmd_t pmd_mkdevmap(pmd_t pmd)
>> +{
>> +       return pte_pmd(pte_mkdevmap(pmd_pte(pmd)));
>> +}
>> +
>> +static inline int pmd_devmap(pmd_t pmd)
>> +{
>> +       return pte_devmap(pmd_pte(pmd));
>> +}
>
> This should be defined only if #ifdef __HAVE_ARCH_PTE_DEVMAP

ok

>
> The rest looks OK
>
> Balbir Singh.

--
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: [HMM 00/15] HMM (Heterogeneous Memory Management) v22
From: Balbir Singh @ 2017-05-24  1:55 UTC (permalink / raw)
  To: Jérôme Glisse
  Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org, linux-mm,
	John Hubbard, David Nellans
In-Reply-To: <20170522165206.6284-1-jglisse@redhat.com>

On Tue, May 23, 2017 at 2:51 AM, Jérôme Glisse <jglisse@redhat.com> wrote:
> Patchset is on top of mmotm mmotm-2017-05-18, git branch:
>
> https://cgit.freedesktop.org/~glisse/linux/log/?h=hmm-v22
>
> Change since v21 is adding back special refcounting in put_page() to
> catch when a ZONE_DEVICE page is free (refcount going from 2 to 1
> unlike regular page where a refcount of 0 means the page is free).
> See patch 8 of this serie for this refcounting. I did not use static
> keys because it kind of scares me to do that for an inline function.
> If people strongly feel about this i can try to make static key works
> here. Kirill will most likely want to review this.
>
>
> Everything else is the same. Below is the long description of what HMM
> is about and why. At the end of this email i describe briefly each patch
> and suggest reviewers for each of them.
>
>
> Heterogeneous Memory Management (HMM) (description and justification)
>

Thanks for the patches! These patches are very helpful. There are a
few additional things we would need on top of this (once HMM the base
is merged)

1. Support for other architectures, we'd like to make sure we can get
this working for powerpc for example. As a first step we have
ZONE_DEVICE enablement patches, but I think we need some additional
patches for iomem space searching and memory hotplug, IIRC
2. HMM-CDM and physical address based migration bits. In a recent RFC
we decided to try and use the HMM CDM route as a route to implementing
coherent device memory as a starting point. It would be nice to have
those patches on top of these once these make it to mm -
https://lwn.net/Articles/720380/

Balbir Singh.

--
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

* mm, we use rcu access task_struct in mm_match_cgroup(), but not use rcu free in free_task_struct()
From: Xishi Qiu @ 2017-05-24  1:40 UTC (permalink / raw)
  To: Michal Hocko, Mel Gorman, Hugh Dickins, Vlastimil Babka,
	Minchan Kim
  Cc: Linux MM, LKML, wencongyang (A)

Hi, I find we use rcu access task_struct in mm_match_cgroup(), but not use
rcu free in free_task_struct(), is it right?

Here is the backtrace.

PID: 2133   TASK: ffff881fe3353300  CPU: 2   COMMAND: "CPU 15/KVM"
 #0 [ffff881fe276b528] machine_kexec at ffffffff8105280b
 #1 [ffff881fe276b588] crash_kexec at ffffffff810f5072
 #2 [ffff881fe276b658] panic at ffffffff8163e23b
 #3 [ffff881fe276b6d8] oops_end at ffffffff8164d61b
 #4 [ffff881fe276b700] die at ffffffff8101872b
 #5 [ffff881fe276b730] do_general_protection at ffffffff8164cefe
 #6 [ffff881fe276b760] general_protection at ffffffff8164c7a8
    [exception RIP: mem_cgroup_from_task+22]
    RIP: ffffffff811db536  RSP: ffff881fe276b810  RFLAGS: 00010286
    RAX: 6b6b6b6b6b6b6b6b  RBX: ffffea007f988880  RCX: 0000000000020000
    RDX: 00000007fa607d67  RSI: 00000007fa607d67  RDI: ffff880fe36d72c0
    RBP: ffff881fe276b880   R8: 00000007fa607600   R9: a801fd67b3000000
    R10: 57fdec98cc59ecc0  R11: ffff880fe2e8dbd0  R12: ffffc9001cb74000
    R13: ffff881fdb8cfda0  R14: ffff881fe2581570  R15: 00000007fa607d67
    ORIG_RAX: ffffffffffffffff  CS: 0010  SS: 0000
 #7 [ffff881fe276b810] page_referenced at ffffffff811a6b8a
 #8 [ffff881fe276b888] shrink_page_list at ffffffff81180994
 #9 [ffff881fe276b9c0] shrink_inactive_list at ffffffff8118166a
#10 [ffff881fe276ba88] shrink_lruvec at ffffffff81182135
#11 [ffff881fe276bb88] shrink_zone at ffffffff81182596
#12 [ffff881fe276bbe0] do_try_to_free_pages at ffffffff81182a90
#13 [ffff881fe276bc58] try_to_free_mem_cgroup_pages at ffffffff81182fea
#14 [ffff881fe276bcf0] mem_cgroup_reclaim at ffffffff811dd8de
#15 [ffff881fe276bd30] __mem_cgroup_try_charge at ffffffff811ddd9c
#16 [ffff881fe276bdf0] __mem_cgroup_try_charge_swapin at ffffffff811df62b
#17 [ffff881fe276be28] mem_cgroup_try_charge_swapin at ffffffff811e0537
#18 [ffff881fe276be38] handle_mm_fault at ffffffff8119abdd
#19 [ffff881fe276bec8] __do_page_fault at ffffffff816502d6
#20 [ffff881fe276bf28] do_page_fault at ffffffff81650603
#21 [ffff881fe276bf50] page_fault at ffffffff8164c808
    RIP: 00007fdaba456500  RSP: 00007fdaaba6c978  RFLAGS: 00010246
    RAX: ffffffffffffffff  RBX: 0000000000000000  RCX: fffffffffffffbd0
    RDX: 0000000000000000  RSI: 000000000000ae80  RDI: 000000000000002c
    RBP: 00007fdaaba6c9f0   R8: 0000000000840c70   R9: 00000000000000be
    R10: 000000007fffffff  R11: 0000000000000246  R12: 0000000003622010
    R13: 000000000000ae80  R14: 00000000008274e0  R15: 0000000003622010
    ORIG_RAX: ffffffffffffffff  CS: 0033  SS: 002b

^ permalink raw reply

* Re: [RFC PATCH 0/4 v2] mm: give __GFP_REPEAT a better semantic
From: NeilBrown @ 2017-05-24  1:06 UTC (permalink / raw)
  To: Vlastimil Babka, Michal Hocko, linux-mm
  Cc: Johannes Weiner, Mel Gorman, Andrew Morton, LKML, Darrick J. Wong,
	Heiko Carstens, NeilBrown, Jonathan Corbet, Paolo Bonzini,
	Eric W. Biederman
In-Reply-To: <77fdc6db-5cc1-297f-e049-0d6f824e688c@suse.cz>

[-- Attachment #1: Type: text/plain, Size: 7088 bytes --]

On Tue, May 23 2017, Vlastimil Babka wrote:

> On 05/16/2017 11:10 AM, Michal Hocko wrote:
>> So, is there some interest in this? I am not going to push this if there
>> is a general consensus that we do not need to do anything about the
>> current situation or need a different approach.
>
> After the recent LWN article [1] I think that we should really support
> marking allocations as failable, without making them too easily failable
> via __GFP_NORETRY. The __GFP_RETRY_MAY_FAIL flag sounds like a good way
> to do that without introducing a new __GFP_MAYFAIL. We could also
> introduce a wrapper such as GFP_KERNEL_MAYFAIL.
>
> [1] https://lwn.net/Articles/723317/

Yes please!!!

I particularly like:

> - GFP_KERNEL | __GFP_NORETRY - overrides the default allocator behavior and
>   all allocation requests fail early rather than cause disruptive
>   reclaim (one round of reclaim in this implementation). The OOM killer
>   is not invoked.
> - GFP_KERNEL | __GFP_RETRY_MAYFAIL - overrides the default allocator behavior
>   and all allocation requests try really hard. The request will fail if the
>   reclaim cannot make any progress. The OOM killer won't be triggered.
> - GFP_KERNEL | __GFP_NOFAIL - overrides the default allocator behavior
>   and all allocation requests will loop endlessly until they
>   succeed. This might be really dangerous especially for larger orders.

There seems to be a good range here, and the two end points are good
choices.
I like that only __GFP_NOFAIL triggers the OOM.
I would like the middle option to be the default.  I think that is what
many people thought the default was.  I appreciate that making the
transition might be awkward.
Maybe create GFP_DEFAULT which matches the middle option and encourage
that in new code??

We would probably want guidelines on when __GFP_NOFAIL is acceptable.
I assume:
  - no locks held
  - small allocations OK, large allocation need clear justification.
  - error would be exposed to systemcall
???

I think it is important to give kernel developers clear options and make
it easy for them to choose the best option.  This helps to do that.

Thanks,
NeilBrown


>
>> On Tue 07-03-17 16:48:39, Michal Hocko wrote:
>>> Hi,
>>> this is a follow up for __GFP_REPEAT clean up merged in 4.7. The previous
>>> version of this patch series was posted as an RFC
>>> http://lkml.keprnel.org/r/1465212736-14637-1-git-send-email-mhocko@kernel.org
>>> Since then I have reconsidered the semantic and made it a counterpart
>>> to the __GFP_NORETRY and made it the other extreme end of the retry
>>> logic. Both are not invoking the OOM killer so they are suitable
>>> for allocation paths with a fallback. Also a new potential user has
>>> emerged (kvmalloc - see patch 4). I have also renamed the flag from
>>> __GFP_RETRY_HARD to __GFP_RETRY_MAY_FAIL as this should be more clear.
>>>
>>> I have kept the RFC status because of the semantic change. The patch 1
>>> is an exception because it should be merge regardless of the rest.
>>>
>>> The main motivation for the change is that the current implementation of
>>> __GFP_REPEAT is not very much useful.
>>>
>>> The documentation says:
>>>  * __GFP_REPEAT: Try hard to allocate the memory, but the allocation attempt
>>>  *   _might_ fail.  This depends upon the particular VM implementation.
>>>
>>> It just fails to mention that this is true only for large (costly) high
>>> order which has been the case since the flag was introduced. A similar
>>> semantic would be really helpful for smal orders as well, though,
>>> because we have places where a failure with a specific fallback error
>>> handling is preferred to a potential endless loop inside the page
>>> allocator.
>>>
>>> The earlier cleanup dropped __GFP_REPEAT usage for low (!costly) order
>>> users so only those which might use larger orders have stayed. One user
>>> which slipped through cracks is addressed in patch 1.
>>>
>>> Let's rename the flag to something more verbose and use it for existing
>>> users. Semantic for those will not change. Then implement low (!costly)
>>> orders failure path which is hit after the page allocator is about to
>>> invoke the oom killer. Now we have a good counterpart for __GFP_NORETRY
>>> and finally can tell try as hard as possible without the OOM killer.
>>>
>>> Xfs code already has an existing annotation for allocations which are
>>> allowed to fail and we can trivially map them to the new gfp flag
>>> because it will provide the semantic KM_MAYFAIL wants.
>>>
>>> kvmalloc will allow also !costly high order allocations to retry hard
>>> before falling back to the vmalloc.
>>>
>>> The patchset is based on the current linux-next.
>>>
>>> Shortlog
>>> Michal Hocko (4):
>>>       s390: get rid of superfluous __GFP_REPEAT
>>>       mm, tree wide: replace __GFP_REPEAT by __GFP_RETRY_MAYFAIL with more useful semantic
>>>       xfs: map KM_MAYFAIL to __GFP_RETRY_MAYFAIL
>>>       mm: kvmalloc support __GFP_RETRY_MAYFAIL for all sizes
>>>
>>> Diffstat
>>>  Documentation/DMA-ISA-LPC.txt                |  2 +-
>>>  arch/powerpc/include/asm/book3s/64/pgalloc.h |  2 +-
>>>  arch/powerpc/kvm/book3s_64_mmu_hv.c          |  2 +-
>>>  arch/s390/mm/pgalloc.c                       |  2 +-
>>>  drivers/mmc/host/wbsd.c                      |  2 +-
>>>  drivers/s390/char/vmcp.c                     |  2 +-
>>>  drivers/target/target_core_transport.c       |  2 +-
>>>  drivers/vhost/net.c                          |  2 +-
>>>  drivers/vhost/scsi.c                         |  2 +-
>>>  drivers/vhost/vsock.c                        |  2 +-
>>>  fs/btrfs/check-integrity.c                   |  2 +-
>>>  fs/btrfs/raid56.c                            |  2 +-
>>>  fs/xfs/kmem.h                                | 10 +++++++++
>>>  include/linux/gfp.h                          | 32 +++++++++++++++++++---------
>>>  include/linux/slab.h                         |  3 ++-
>>>  include/trace/events/mmflags.h               |  2 +-
>>>  mm/hugetlb.c                                 |  4 ++--
>>>  mm/internal.h                                |  2 +-
>>>  mm/page_alloc.c                              | 14 +++++++++---
>>>  mm/sparse-vmemmap.c                          |  4 ++--
>>>  mm/util.c                                    | 14 ++++--------
>>>  mm/vmalloc.c                                 |  2 +-
>>>  mm/vmscan.c                                  |  8 +++----
>>>  net/core/dev.c                               |  6 +++---
>>>  net/core/skbuff.c                            |  2 +-
>>>  net/sched/sch_fq.c                           |  2 +-
>>>  tools/perf/builtin-kmem.c                    |  2 +-
>>>  27 files changed, 78 insertions(+), 53 deletions(-)
>>>
>>> --
>>> 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>
>> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply

* Re: [HMM 00/15] HMM (Heterogeneous Memory Management) v22
From: Andrew Morton @ 2017-05-23 22:05 UTC (permalink / raw)
  To: Jerome Glisse
  Cc: linux-kernel, linux-mm, John Hubbard, Kirill A. Shutemov,
	Dan Williams
In-Reply-To: <20170523220248.GA23833@redhat.com>

On Tue, 23 May 2017 18:02:49 -0400 Jerome Glisse <jglisse@redhat.com> wrote:

> Andrew i posted updated patch for 0007 0008 and 0009 as reply to orignal
> patches. It includes changes Dan and Kyrill wanted to see. I added the
> device_private_key to page_alloc.c to avoid modify more than 3 patches
> but if you prefer i can repost a v23 serie and move the static key to
> hmm.c
> 
> Also i guess posting a v23 would have it tested against builder as i
> doubt automatic builder are clever enough to understand all this.
> 

Yes please.  22->23 is less than a 5% increment ;)

--
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: [Question] Mlocked count will not be decreased
From: Tetsuo Handa @ 2017-05-23 22:04 UTC (permalink / raw)
  To: Kefeng Wang, linux-mm, linux-kernel, zhongjiang, Qiuxishi,
	Yisheng Xie
In-Reply-To: <a61701d8-3dce-51a2-5eaf-14de84425640@huawei.com>

Kefeng Wang wrote:
> Hi All,
> 
> Mlocked in meminfo will be increasing with an small testcase, and never be released in mainline,
> here is a testcase[1] to reproduce the issue, but the centos7.2/7.3 will not increase.
> 
> Is it normal?

I confirmed your problem also occurs in Linux 4.11 using below testcase.
MemFree is not decreasing while Mlocked is increasing.
Thus, it seems to be statistics accounting bug.

----------
#include <sys/mman.h>
#include <stdlib.h>
#include <unistd.h>

int main(int argc, char ** argv)
{
	int i;
	for (i = 0; i < 128; i++)
		if (fork() == 0) {
			malloc(1048576);
			while (1) {
				mlockall(MCL_CURRENT | MCL_FUTURE);
				munlockall();
			}
		}
	return 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

* Re: [HMM 00/15] HMM (Heterogeneous Memory Management) v22
From: Jerome Glisse @ 2017-05-23 22:02 UTC (permalink / raw)
  To: akpm, linux-kernel, linux-mm
  Cc: John Hubbard, Kirill A. Shutemov, Dan Williams
In-Reply-To: <20170522165206.6284-1-jglisse@redhat.com>

Andrew i posted updated patch for 0007 0008 and 0009 as reply to orignal
patches. It includes changes Dan and Kyrill wanted to see. I added the
device_private_key to page_alloc.c to avoid modify more than 3 patches
but if you prefer i can repost a v23 serie and move the static key to
hmm.c

Also i guess posting a v23 would have it tested against builder as i
doubt automatic builder are clever enough to understand all this.

Cheers,
Jerome

--
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: make kswapd try harder to keep active pages in cache
From: Andrew Morton @ 2017-05-23 21:57 UTC (permalink / raw)
  To: Josef Bacik; +Cc: kernel-team, riel, hannes, linux-mm
In-Reply-To: <1495549403-3719-1-git-send-email-jbacik@fb.com>

On Tue, 23 May 2017 10:23:23 -0400 Josef Bacik <josef@toxicpanda.com> wrote:

> When testing a slab heavy workload I noticed that we often would barely
> reclaim anything at all from slab when kswapd started doing reclaim.
> This is because we use the ratio of nr_scanned / nr_lru to determine how
> much of slab we should reclaim.  But in a slab only/mostly workload we
> will not have much page cache to reclaim, and thus our ratio will be
> really low and not at all related to where the memory on the system is.
> Instead we want to use a ratio of the reclaimable slab to the actual
> reclaimable space on the system.  That way if we are slab heavy we work
> harder to reclaim slab.
> 
> The other part of this that hurts is when we are running close to full
> memory with our working set.  If we start putting a lot of reclaimable
> slab pressure on the system (think find /, or some other silliness), we
> will happily evict the active pages over the slab cache.  This is kind
> of backwards as we want to do all that we can to keep the active working
> set in memory, and instead evict these short lived objects.  The same
> thing occurs when say you do a yum update of a few packages while your
> working set takes up most of RAM, you end up with inactive lists being
> relatively small and so we reclaim active pages even though we could
> reclaim these short lived inactive pages.
> 
> My approach here is twofold.  First, keep track of the difference in
> inactive and slab pages since the last time kswapd ran.  In the first
> run this will just be the overall counts of inactive and slab, but for
> each subsequent run we'll have a good idea of where the memory pressure
> is coming from.  Then we use this information to put pressure on either
> the inactive lists or the slab caches, depending on where the pressure
> is coming from.
>
> ...
>

hm, that's a pretty big change.  I took it, but it will require quite
some reviewing and testing to get further, please.

--
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] ib/core: not to set page dirty bit if it's already set.
From: Qing Huang @ 2017-05-23 21:39 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-rdma, linux-kernel, dledford, sean.hefty, artemyko,
	linux-mm
In-Reply-To: <20170523074234.GE29525@infradead.org>



On 5/23/2017 12:42 AM, Christoph Hellwig wrote:
> On Mon, May 22, 2017 at 04:43:57PM -0700, Qing Huang wrote:
>> On 5/19/2017 6:05 AM, Christoph Hellwig wrote:
>>> On Thu, May 18, 2017 at 04:33:53PM -0700, Qing Huang wrote:
>>>> This change will optimize kernel memory deregistration operations.
>>>> __ib_umem_release() used to call set_page_dirty_lock() against every
>>>> writable page in its memory region. Its purpose is to keep data
>>>> synced between CPU and DMA device when swapping happens after mem
>>>> deregistration ops. Now we choose not to set page dirty bit if it's
>>>> already set by kernel prior to calling __ib_umem_release(). This
>>>> reduces memory deregistration time by half or even more when we ran
>>>> application simulation test program.
>>> As far as I can tell this code doesn't even need set_page_dirty_lock
>>> and could just use set_page_dirty
>> It seems that set_page_dirty_lock has been used here for more than 10 years.
>> Don't know the original purpose. Maybe it was used to prevent races between
>> setting dirty bits and swapping out pages?
> I suspect copy & paste.  Or maybe I don't actually understand the
> explanation of set_page_dirty vs set_page_dirty_lock enough.  But
> I'd rather not hack around the problem.
> --
I think there are two parts here. First part is that we don't need to 
set the dirty bit if it's already set. Second part is whether we use 
set_page_dirty or set_page_dirty_lock to set dirty bits.


--
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

* [HMM 09/18] mm/hmm/devmem: device memory hotplug using ZONE_DEVICE v5
From: Jérôme Glisse @ 2017-05-23 21:37 UTC (permalink / raw)
  To: akpm, linux-kernel, linux-mm
  Cc: Dan Williams, Kirill A . Shutemov, Jérôme Glisse,
	Evgeny Baskakov, John Hubbard, Mark Hairgrove, Sherry Cheung,
	Subhash Gutti
In-Reply-To: <20170522165206.6284-10-jglisse@redhat.com>

This introduce a simple struct and associated helpers for device driver
to use when hotpluging un-addressable device memory as ZONE_DEVICE. It
will find a unuse physical address range and trigger memory hotplug for
it which allocates and initialize struct page for the device memory.

Changed since v4:
  - enable device_private_key static key when adding device memory
Changed since v3:
  - s/device unaddressable/device private/
Changed since v2:
  - s/SECTION_SIZE/PA_SECTION_SIZE
Changed since v1:
  - change to adapt to new add_pages() helper
  - make this x86-64 only for now

Signed-off-by: JA(C)rA'me Glisse <jglisse@redhat.com>
Signed-off-by: Evgeny Baskakov <ebaskakov@nvidia.com>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Mark Hairgrove <mhairgrove@nvidia.com>
Signed-off-by: Sherry Cheung <SCheung@nvidia.com>
Signed-off-by: Subhash Gutti <sgutti@nvidia.com>
---
 include/linux/hmm.h | 114 ++++++++++++++
 mm/Kconfig          |   9 ++
 mm/hmm.c            | 416 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 538 insertions(+), 1 deletion(-)

diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 248a6e0..0865afd 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -72,6 +72,11 @@
 
 #if IS_ENABLED(CONFIG_HMM)
 
+#include <linux/migrate.h>
+#include <linux/memremap.h>
+#include <linux/completion.h>
+
+
 struct hmm;
 
 /*
@@ -322,6 +327,115 @@ int hmm_vma_fault(struct vm_area_struct *vma,
 #endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
 
 
+#if IS_ENABLED(CONFIG_HMM_DEVMEM)
+struct hmm_devmem;
+
+struct page *hmm_vma_alloc_locked_page(struct vm_area_struct *vma,
+				       unsigned long addr);
+
+/*
+ * struct hmm_devmem_ops - callback for ZONE_DEVICE memory events
+ *
+ * @free: call when refcount on page reach 1 and thus is no longer use
+ * @fault: call when there is a page fault to unaddressable memory
+ */
+struct hmm_devmem_ops {
+	void (*free)(struct hmm_devmem *devmem, struct page *page);
+	int (*fault)(struct hmm_devmem *devmem,
+		     struct vm_area_struct *vma,
+		     unsigned long addr,
+		     struct page *page,
+		     unsigned int flags,
+		     pmd_t *pmdp);
+};
+
+/*
+ * struct hmm_devmem - track device memory
+ *
+ * @completion: completion object for device memory
+ * @pfn_first: first pfn for this resource (set by hmm_devmem_add())
+ * @pfn_last: last pfn for this resource (set by hmm_devmem_add())
+ * @resource: IO resource reserved for this chunk of memory
+ * @pagemap: device page map for that chunk
+ * @device: device to bind resource to
+ * @ops: memory operations callback
+ * @ref: per CPU refcount
+ *
+ * This an helper structure for device drivers that do not wish to implement
+ * the gory details related to hotplugging new memoy and allocating struct
+ * pages.
+ *
+ * Device drivers can directly use ZONE_DEVICE memory on their own if they
+ * wish to do so.
+ */
+struct hmm_devmem {
+	struct completion		completion;
+	unsigned long			pfn_first;
+	unsigned long			pfn_last;
+	struct resource			*resource;
+	struct device			*device;
+	struct dev_pagemap		pagemap;
+	const struct hmm_devmem_ops	*ops;
+	struct percpu_ref		ref;
+};
+
+/*
+ * To add (hotplug) device memory, HMM assumes that there is no real resource
+ * that reserves a range in the physical address space (this is intended to be
+ * use by unaddressable device memory). It will reserve a physical range big
+ * enough and allocate struct page for it.
+ *
+ * The device driver can wrap the hmm_devmem struct inside a private device
+ * driver struct. The device driver must call hmm_devmem_remove() before the
+ * device goes away and before freeing the hmm_devmem struct memory.
+ */
+struct hmm_devmem *hmm_devmem_add(const struct hmm_devmem_ops *ops,
+				  struct device *device,
+				  unsigned long size);
+void hmm_devmem_remove(struct hmm_devmem *devmem);
+
+int hmm_devmem_fault_range(struct hmm_devmem *devmem,
+			   struct vm_area_struct *vma,
+			   const struct migrate_vma_ops *ops,
+			   unsigned long *src,
+			   unsigned long *dst,
+			   unsigned long start,
+			   unsigned long addr,
+			   unsigned long end,
+			   void *private);
+
+/*
+ * hmm_devmem_page_set_drvdata - set per-page driver data field
+ *
+ * @page: pointer to struct page
+ * @data: driver data value to set
+ *
+ * Because page can not be on lru we have an unsigned long that driver can use
+ * to store a per page field. This just a simple helper to do that.
+ */
+static inline void hmm_devmem_page_set_drvdata(struct page *page,
+					       unsigned long data)
+{
+	unsigned long *drvdata = (unsigned long *)&page->pgmap;
+
+	drvdata[1] = data;
+}
+
+/*
+ * hmm_devmem_page_get_drvdata - get per page driver data field
+ *
+ * @page: pointer to struct page
+ * Return: driver data value
+ */
+static inline unsigned long hmm_devmem_page_get_drvdata(struct page *page)
+{
+	unsigned long *drvdata = (unsigned long *)&page->pgmap;
+
+	return drvdata[1];
+}
+#endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */
+
+
 /* Below are for HMM internal use only! Not to be used by device driver! */
 void hmm_mm_destroy(struct mm_struct *mm);
 
diff --git a/mm/Kconfig b/mm/Kconfig
index f5357ff..46296d5 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -314,6 +314,15 @@ config HMM_MIRROR
 	  page tables (at PAGE_SIZE granularity), and must be able to recover from
 	  the resulting potential page faults.
 
+config HMM_DEVMEM
+	bool "HMM device memory helpers (to leverage ZONE_DEVICE)"
+	depends on ARCH_HAS_HMM
+	select HMM
+	help
+	  HMM devmem is a set of helper routines to leverage the ZONE_DEVICE
+	  feature. This is just to avoid having device drivers to replicating a lot
+	  of boiler plate code.  See Documentation/vm/hmm.txt.
+
 config PHYS_ADDR_T_64BIT
 	def_bool 64BIT || ARCH_PHYS_ADDR_T_64BIT
 
diff --git a/mm/hmm.c b/mm/hmm.c
index ed97051..87cc130 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -23,9 +23,15 @@
 #include <linux/swap.h>
 #include <linux/slab.h>
 #include <linux/sched.h>
+#include <linux/mmzone.h>
+#include <linux/pagemap.h>
 #include <linux/swapops.h>
 #include <linux/hugetlb.h>
+#include <linux/memremap.h>
 #include <linux/mmu_notifier.h>
+#include <linux/memory_hotplug.h>
+
+#define PA_SECTION_SIZE (1UL << PA_SECTION_SHIFT)
 
 static const struct mmu_notifier_ops hmm_mmu_notifier_ops;
 
@@ -427,7 +433,15 @@ static int hmm_vma_walk_pmd(pmd_t *pmdp,
 			 * This is a special swap entry, ignore migration, use
 			 * device and report anything else as error.
 			 */
-			if (is_migration_entry(entry)) {
+			if (is_device_private_entry(entry)) {
+				pfns[i] = hmm_pfn_t_from_pfn(swp_offset(entry));
+				if (is_write_device_private_entry(entry)) {
+					pfns[i] |= HMM_PFN_WRITE;
+				} else if (write_fault)
+					goto fault;
+				pfns[i] |= HMM_PFN_DEVICE_UNADDRESSABLE;
+				pfns[i] |= flag;
+			} else if (is_migration_entry(entry)) {
 				if (hmm_vma_walk->fault) {
 					pte_unmap(ptep);
 					hmm_vma_walk->last = addr;
@@ -721,3 +735,403 @@ int hmm_vma_fault(struct vm_area_struct *vma,
 }
 EXPORT_SYMBOL(hmm_vma_fault);
 #endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
+
+
+#if IS_ENABLED(CONFIG_HMM_DEVMEM)
+struct page *hmm_vma_alloc_locked_page(struct vm_area_struct *vma,
+				       unsigned long addr)
+{
+	struct page *page;
+
+	page = alloc_page_vma(GFP_HIGHUSER, vma, addr);
+	if (!page)
+		return NULL;
+	lock_page(page);
+	return page;
+}
+EXPORT_SYMBOL(hmm_vma_alloc_locked_page);
+
+
+static void hmm_devmem_ref_release(struct percpu_ref *ref)
+{
+	struct hmm_devmem *devmem;
+
+	devmem = container_of(ref, struct hmm_devmem, ref);
+	complete(&devmem->completion);
+}
+
+static void hmm_devmem_ref_exit(void *data)
+{
+	struct percpu_ref *ref = data;
+	struct hmm_devmem *devmem;
+
+	devmem = container_of(ref, struct hmm_devmem, ref);
+	percpu_ref_exit(ref);
+	devm_remove_action(devmem->device, &hmm_devmem_ref_exit, data);
+}
+
+static void hmm_devmem_ref_kill(void *data)
+{
+	struct percpu_ref *ref = data;
+	struct hmm_devmem *devmem;
+
+	devmem = container_of(ref, struct hmm_devmem, ref);
+	percpu_ref_kill(ref);
+	wait_for_completion(&devmem->completion);
+	devm_remove_action(devmem->device, &hmm_devmem_ref_kill, data);
+}
+
+static int hmm_devmem_fault(struct vm_area_struct *vma,
+			    unsigned long addr,
+			    struct page *page,
+			    unsigned int flags,
+			    pmd_t *pmdp)
+{
+	struct hmm_devmem *devmem = page->pgmap->data;
+
+	return devmem->ops->fault(devmem, vma, addr, page, flags, pmdp);
+}
+
+static void hmm_devmem_free(struct page *page, void *data)
+{
+	struct hmm_devmem *devmem = data;
+
+	devmem->ops->free(devmem, page);
+}
+
+static DEFINE_MUTEX(hmm_devmem_lock);
+static RADIX_TREE(hmm_devmem_radix, GFP_KERNEL);
+
+static void hmm_devmem_radix_release(struct resource *resource)
+{
+	resource_size_t key, align_start, align_size, align_end;
+
+	align_start = resource->start & ~(PA_SECTION_SIZE - 1);
+	align_size = ALIGN(resource_size(resource), PA_SECTION_SIZE);
+	align_end = align_start + align_size - 1;
+
+	mutex_lock(&hmm_devmem_lock);
+	for (key = resource->start;
+	     key <= resource->end;
+	     key += PA_SECTION_SIZE)
+		radix_tree_delete(&hmm_devmem_radix, key >> PA_SECTION_SHIFT);
+	mutex_unlock(&hmm_devmem_lock);
+}
+
+static void hmm_devmem_release(struct device *dev, void *data)
+{
+	struct hmm_devmem *devmem = data;
+	struct resource *resource = devmem->resource;
+	unsigned long start_pfn, npages;
+	struct zone *zone;
+	struct page *page;
+
+	if (percpu_ref_tryget_live(&devmem->ref)) {
+		dev_WARN(dev, "%s: page mapping is still live!\n", __func__);
+		percpu_ref_put(&devmem->ref);
+	}
+
+	/* pages are dead and unused, undo the arch mapping */
+	start_pfn = (resource->start & ~(PA_SECTION_SIZE - 1)) >> PAGE_SHIFT;
+	npages = ALIGN(resource_size(resource), PA_SECTION_SIZE) >> PAGE_SHIFT;
+
+	page = pfn_to_page(start_pfn);
+	zone = page_zone(page);
+
+	mem_hotplug_begin();
+	__remove_pages(zone, start_pfn, npages);
+	mem_hotplug_done();
+
+	hmm_devmem_radix_release(resource);
+}
+
+static struct hmm_devmem *hmm_devmem_find(resource_size_t phys)
+{
+	WARN_ON_ONCE(!rcu_read_lock_held());
+
+	return radix_tree_lookup(&hmm_devmem_radix, phys >> PA_SECTION_SHIFT);
+}
+
+static int hmm_devmem_pages_create(struct hmm_devmem *devmem)
+{
+	resource_size_t key, align_start, align_size, align_end;
+	struct device *device = devmem->device;
+	int ret, nid, is_ram;
+	unsigned long pfn;
+
+	align_start = devmem->resource->start & ~(PA_SECTION_SIZE - 1);
+	align_size = ALIGN(devmem->resource->start +
+			   resource_size(devmem->resource),
+			   PA_SECTION_SIZE) - align_start;
+
+	is_ram = region_intersects(align_start, align_size,
+				   IORESOURCE_SYSTEM_RAM,
+				   IORES_DESC_NONE);
+	if (is_ram == REGION_MIXED) {
+		WARN_ONCE(1, "%s attempted on mixed region %pr\n",
+				__func__, devmem->resource);
+		return -ENXIO;
+	}
+	if (is_ram == REGION_INTERSECTS)
+		return -ENXIO;
+
+	devmem->pagemap.type = MEMORY_DEVICE_PRIVATE;
+	devmem->pagemap.res = devmem->resource;
+	devmem->pagemap.page_fault = hmm_devmem_fault;
+	devmem->pagemap.page_free = hmm_devmem_free;
+	devmem->pagemap.dev = devmem->device;
+	devmem->pagemap.ref = &devmem->ref;
+	devmem->pagemap.data = devmem;
+
+	mutex_lock(&hmm_devmem_lock);
+	align_end = align_start + align_size - 1;
+	for (key = align_start; key <= align_end; key += PA_SECTION_SIZE) {
+		struct hmm_devmem *dup;
+
+		rcu_read_lock();
+		dup = hmm_devmem_find(key);
+		rcu_read_unlock();
+		if (dup) {
+			dev_err(device, "%s: collides with mapping for %s\n",
+				__func__, dev_name(dup->device));
+			mutex_unlock(&hmm_devmem_lock);
+			ret = -EBUSY;
+			goto error;
+		}
+		ret = radix_tree_insert(&hmm_devmem_radix,
+					key >> PA_SECTION_SHIFT,
+					devmem);
+		if (ret) {
+			dev_err(device, "%s: failed: %d\n", __func__, ret);
+			mutex_unlock(&hmm_devmem_lock);
+			goto error_radix;
+		}
+	}
+	mutex_unlock(&hmm_devmem_lock);
+
+	nid = dev_to_node(device);
+	if (nid < 0)
+		nid = numa_mem_id();
+
+	mem_hotplug_begin();
+	ret = add_pages(nid, align_start >> PAGE_SHIFT,
+			align_size >> PAGE_SHIFT, false);
+	if (ret) {
+		mem_hotplug_done();
+		goto error_add_memory;
+	}
+	move_pfn_range_to_zone(&NODE_DATA(nid)->node_zones[ZONE_DEVICE],
+				align_start >> PAGE_SHIFT,
+				align_size >> PAGE_SHIFT);
+	mem_hotplug_done();
+
+	for (pfn = devmem->pfn_first; pfn < devmem->pfn_last; pfn++) {
+		struct page *page = pfn_to_page(pfn);
+
+		/*
+		 * ZONE_DEVICE pages union ->lru with a ->pgmap back
+		 * pointer.  It is a bug if a ZONE_DEVICE page is ever
+		 * freed or placed on a driver-private list. Therefore,
+		 * seed the storage with LIST_POISON* values.
+		 */
+		list_del(&page->lru);
+		page->pgmap = &devmem->pagemap;
+	}
+	return 0;
+
+error_add_memory:
+	untrack_pfn(NULL, PHYS_PFN(align_start), align_size);
+error_radix:
+	hmm_devmem_radix_release(devmem->resource);
+error:
+	return ret;
+}
+
+static int hmm_devmem_match(struct device *dev, void *data, void *match_data)
+{
+	struct hmm_devmem *devmem = data;
+
+	return devmem->resource == match_data;
+}
+
+static void hmm_devmem_pages_remove(struct hmm_devmem *devmem)
+{
+	devres_release(devmem->device, &hmm_devmem_release,
+		       &hmm_devmem_match, devmem->resource);
+}
+
+/*
+ * hmm_devmem_add() - hotplug ZONE_DEVICE memory for device memory
+ *
+ * @ops: memory event device driver callback (see struct hmm_devmem_ops)
+ * @device: device struct to bind the resource too
+ * @size: size in bytes of the device memory to add
+ * Returns: pointer to new hmm_devmem struct ERR_PTR otherwise
+ *
+ * This first finds an empty range of physical address big enough to contain the
+ * new resource, and then hotplugs it as ZONE_DEVICE memory, which in turn
+ * allocates struct pages. It does not do anything beyond that; all events
+ * affecting the memory will go through the various callbacks provided by
+ * hmm_devmem_ops struct.
+ */
+struct hmm_devmem *hmm_devmem_add(const struct hmm_devmem_ops *ops,
+				  struct device *device,
+				  unsigned long size)
+{
+	struct hmm_devmem *devmem;
+	resource_size_t addr;
+	int ret;
+
+	static_branch_enable(&device_private_key);
+
+	devmem = devres_alloc_node(&hmm_devmem_release, sizeof(*devmem),
+				   GFP_KERNEL, dev_to_node(device));
+	if (!devmem)
+		return ERR_PTR(-ENOMEM);
+
+	init_completion(&devmem->completion);
+	devmem->pfn_first = -1UL;
+	devmem->pfn_last = -1UL;
+	devmem->resource = NULL;
+	devmem->device = device;
+	devmem->ops = ops;
+
+	ret = percpu_ref_init(&devmem->ref, &hmm_devmem_ref_release,
+			      0, GFP_KERNEL);
+	if (ret)
+		goto error_percpu_ref;
+
+	ret = devm_add_action(device, hmm_devmem_ref_exit, &devmem->ref);
+	if (ret)
+		goto error_devm_add_action;
+
+	size = ALIGN(size, PA_SECTION_SIZE);
+	addr = min((unsigned long)iomem_resource.end, 1UL << MAX_PHYSMEM_BITS);
+	addr = addr - size + 1UL;
+
+	/*
+	 * FIXME add a new helper to quickly walk resource tree and find free
+	 * range
+	 *
+	 * FIXME what about ioport_resource resource ?
+	 */
+	for (; addr > size && addr >= iomem_resource.start; addr -= size) {
+		ret = region_intersects(addr, size, 0, IORES_DESC_NONE);
+		if (ret != REGION_DISJOINT)
+			continue;
+
+		devmem->resource = devm_request_mem_region(device, addr, size,
+							   dev_name(device));
+		if (!devmem->resource) {
+			ret = -ENOMEM;
+			goto error_no_resource;
+		}
+		break;
+	}
+	if (!devmem->resource) {
+		ret = -ERANGE;
+		goto error_no_resource;
+	}
+
+	devmem->resource->desc = IORES_DESC_DEVICE_PRIVATE_MEMORY;
+	devmem->pfn_first = devmem->resource->start >> PAGE_SHIFT;
+	devmem->pfn_last = devmem->pfn_first +
+			   (resource_size(devmem->resource) >> PAGE_SHIFT);
+
+	ret = hmm_devmem_pages_create(devmem);
+	if (ret)
+		goto error_pages;
+
+	devres_add(device, devmem);
+
+	ret = devm_add_action(device, hmm_devmem_ref_kill, &devmem->ref);
+	if (ret) {
+		hmm_devmem_remove(devmem);
+		return ERR_PTR(ret);
+	}
+
+	return devmem;
+
+error_pages:
+	devm_release_mem_region(device, devmem->resource->start,
+				resource_size(devmem->resource));
+error_no_resource:
+error_devm_add_action:
+	hmm_devmem_ref_kill(&devmem->ref);
+	hmm_devmem_ref_exit(&devmem->ref);
+error_percpu_ref:
+	devres_free(devmem);
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(hmm_devmem_add);
+
+/*
+ * hmm_devmem_remove() - remove device memory (kill and free ZONE_DEVICE)
+ *
+ * @devmem: hmm_devmem struct use to track and manage the ZONE_DEVICE memory
+ *
+ * This will hot-unplug memory that was hotplugged by hmm_devmem_add on behalf
+ * of the device driver. It will free struct page and remove the resource that
+ * reserved the physical address range for this device memory.
+ */
+void hmm_devmem_remove(struct hmm_devmem *devmem)
+{
+	resource_size_t start, size;
+	struct device *device;
+
+	if (!devmem)
+		return;
+
+	device = devmem->device;
+	start = devmem->resource->start;
+	size = resource_size(devmem->resource);
+
+	hmm_devmem_ref_kill(&devmem->ref);
+	hmm_devmem_ref_exit(&devmem->ref);
+	hmm_devmem_pages_remove(devmem);
+
+	devm_release_mem_region(device, start, size);
+}
+EXPORT_SYMBOL(hmm_devmem_remove);
+
+/*
+ * hmm_devmem_fault_range() - migrate back a virtual range of memory
+ *
+ * @devmem: hmm_devmem struct use to track and manage the ZONE_DEVICE memory
+ * @vma: virtual memory area containing the range to be migrated
+ * @ops: migration callback for allocating destination memory and copying
+ * @src: array of unsigned long containing source pfns
+ * @dst: array of unsigned long containing destination pfns
+ * @start: start address of the range to migrate (inclusive)
+ * @addr: fault address (must be inside the range)
+ * @end: end address of the range to migrate (exclusive)
+ * @private: pointer passed back to each of the callback
+ * Returns: 0 on success, VM_FAULT_SIGBUS on error
+ *
+ * This is a wrapper around migrate_vma() which checks the migration status
+ * for a given fault address and returns the corresponding page fault handler
+ * status. That will be 0 on success, or VM_FAULT_SIGBUS if migration failed
+ * for the faulting address.
+ *
+ * This is a helper intendend to be used by the ZONE_DEVICE fault handler.
+ */
+int hmm_devmem_fault_range(struct hmm_devmem *devmem,
+			   struct vm_area_struct *vma,
+			   const struct migrate_vma_ops *ops,
+			   unsigned long *src,
+			   unsigned long *dst,
+			   unsigned long start,
+			   unsigned long addr,
+			   unsigned long end,
+			   void *private)
+{
+	if (migrate_vma(ops, vma, start, end, src, dst, private))
+		return VM_FAULT_SIGBUS;
+
+	if (dst[(addr - start) >> PAGE_SHIFT] & MIGRATE_PFN_ERROR)
+		return VM_FAULT_SIGBUS;
+
+	return 0;
+}
+EXPORT_SYMBOL(hmm_devmem_fault_range);
+#endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */
-- 
2.9.4

--
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

* [HMM 08/18] mm/ZONE_DEVICE: special case put_page() for device private pages v2
From: Jérôme Glisse @ 2017-05-23 21:37 UTC (permalink / raw)
  To: akpm, linux-kernel, linux-mm
  Cc: Dan Williams, Kirill A . Shutemov, Jérôme Glisse,
	Ross Zwisler
In-Reply-To: <20170523132317.rfg33zwfbx3zl6be@node.shutemov.name>

A ZONE_DEVICE page that reach a refcount of 1 is free ie no longer
have any user. For device private pages this is important to catch
and thus we need to special case put_page() for this.

Changed since v1:
  - use static key to disable special code path in put_page() by
    default
  - uninline put_zone_device_private_page()
  - fix build issues with some kernel config related to header
    inter-dependency

Signed-off-by: JA(C)rA'me Glisse <jglisse@redhat.com>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 include/linux/memremap.h | 13 +++++++++++++
 include/linux/mm.h       | 31 ++++++++++++++++++++++---------
 kernel/memremap.c        | 19 ++++++++++++++++++-
 mm/page_alloc.c          |  6 ++++++
 4 files changed, 59 insertions(+), 10 deletions(-)

diff --git a/include/linux/memremap.h b/include/linux/memremap.h
index 0fcf840..0e0d2e6 100644
--- a/include/linux/memremap.h
+++ b/include/linux/memremap.h
@@ -125,6 +125,14 @@ struct dev_pagemap {
 void *devm_memremap_pages(struct device *dev, struct resource *res,
 		struct percpu_ref *ref, struct vmem_altmap *altmap);
 struct dev_pagemap *find_dev_pagemap(resource_size_t phys);
+
+static inline bool is_zone_device_page(const struct page *page);
+
+static inline bool is_device_private_page(const struct page *page)
+{
+	return is_zone_device_page(page) &&
+		page->pgmap->type == MEMORY_DEVICE_PRIVATE;
+}
 #else
 static inline void *devm_memremap_pages(struct device *dev,
 		struct resource *res, struct percpu_ref *ref,
@@ -143,6 +151,11 @@ static inline struct dev_pagemap *find_dev_pagemap(resource_size_t phys)
 {
 	return NULL;
 }
+
+static inline bool is_device_private_page(const struct page *page)
+{
+	return false;
+}
 #endif
 
 /**
diff --git a/include/linux/mm.h b/include/linux/mm.h
index a825dab..7f0656f 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -23,6 +23,7 @@
 #include <linux/page_ext.h>
 #include <linux/err.h>
 #include <linux/page_ref.h>
+#include <linux/memremap.h>
 
 struct mempolicy;
 struct anon_vma;
@@ -788,25 +789,25 @@ static inline bool is_zone_device_page(const struct page *page)
 {
 	return page_zonenum(page) == ZONE_DEVICE;
 }
-
-static inline bool is_device_private_page(const struct page *page)
-{
-	/* See MEMORY_DEVICE_PRIVATE in include/linux/memory_hotplug.h */
-	return ((page_zonenum(page) == ZONE_DEVICE) &&
-		(page->pgmap->type == MEMORY_DEVICE_PRIVATE));
-}
 #else
 static inline bool is_zone_device_page(const struct page *page)
 {
 	return false;
 }
+#endif
 
-static inline bool is_device_private_page(const struct page *page)
+#ifdef CONFIG_DEVICE_PRIVATE
+void put_zone_device_private_page(struct page *page);
+#else
+static inline void put_zone_device_private_page(struct page *page)
 {
-	return false;
 }
 #endif
 
+static inline bool is_device_private_page(const struct page *page);
+
+DECLARE_STATIC_KEY_FALSE(device_private_key);
+
 static inline void get_page(struct page *page)
 {
 	page = compound_head(page);
@@ -822,6 +823,18 @@ static inline void put_page(struct page *page)
 {
 	page = compound_head(page);
 
+	/*
+	 * For private device pages we need to catch refcount transition from
+	 * 2 to 1, when refcount reach one it means the private device page is
+	 * free and we need to inform the device driver through callback. See
+	 * include/linux/memremap.h and HMM for details.
+	 */
+	if (static_branch_unlikely(&device_private_key) &&
+	    unlikely(is_device_private_page(page))) {
+		put_zone_device_private_page(page);
+		return;
+	}
+
 	if (put_page_testzero(page))
 		__put_page(page);
 }
diff --git a/kernel/memremap.c b/kernel/memremap.c
index cd596d4..b9baa6c 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -11,7 +11,6 @@
  * General Public License for more details.
  */
 #include <linux/radix-tree.h>
-#include <linux/memremap.h>
 #include <linux/device.h>
 #include <linux/types.h>
 #include <linux/pfn_t.h>
@@ -464,3 +463,21 @@ struct vmem_altmap *to_vmem_altmap(unsigned long memmap_start)
 	return pgmap ? pgmap->altmap : NULL;
 }
 #endif /* CONFIG_ZONE_DEVICE */
+
+
+#ifdef CONFIG_DEVICE_PRIVATE
+void put_zone_device_private_page(struct page *page)
+{
+	int count = page_ref_dec_return(page);
+
+	/*
+	 * If refcount is 1 then page is freed and refcount is stable as nobody
+	 * holds a reference on the page.
+	 */
+	if (count == 1)
+		page->pgmap->page_free(page, page->pgmap->data);
+	else if (!count)
+		__put_page(page);
+}
+EXPORT_SYMBOL(put_zone_device_private_page);
+#endif /* CONFIG_DEVICE_PRIVATE */
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index e7521ec..2a9dce5 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -72,6 +72,12 @@
 #include <asm/div64.h>
 #include "internal.h"
 
+/*
+ * Device private memory see HMM (Documentation/vm/hmm.txt) or hmm.h
+ */
+DEFINE_STATIC_KEY_FALSE(device_private_key);
+EXPORT_SYMBOL(device_private_key);
+
 /* prevent >1 _updater_ of zone percpu pageset ->high and ->batch fields */
 static DEFINE_MUTEX(pcp_batch_high_lock);
 #define MIN_PERCPU_PAGELIST_FRACTION	(8)
-- 
2.9.4

--
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

* [HMM 07/18] mm/ZONE_DEVICE: new type of ZONE_DEVICE for unaddressable memory v3
From: Jérôme Glisse @ 2017-05-23 21:36 UTC (permalink / raw)
  To: akpm, linux-kernel, linux-mm
  Cc: Dan Williams, Kirill A . Shutemov, Jérôme Glisse,
	Ross Zwisler
In-Reply-To: <CAPcyv4jJh8G7y-Gr-54iBVGrGDQwu=M=FXtkSpXyd=2oNqPcWA@mail.gmail.com>

HMM (heterogeneous memory management) need struct page to support migration
from system main memory to device memory.  Reasons for HMM and migration to
device memory is explained with HMM core patch.

This patch deals with device memory that is un-addressable memory (ie CPU
can not access it). Hence we do not want those struct page to be manage
like regular memory. That is why we extend ZONE_DEVICE to support different
types of memory.

A persistent memory type is define for existing user of ZONE_DEVICE and a
new device un-addressable type is added for the un-addressable memory type.
There is a clear separation between what is expected from each memory type
and existing user of ZONE_DEVICE are un-affected by new requirement and new
use of the un-addressable type. All specific code path are protect with
test against the memory type.

Because memory is un-addressable we use a new special swap type for when
a page is migrated to device memory (this reduces the number of maximum
swap file).

The main two additions beside memory type to ZONE_DEVICE is two callbacks.
First one, page_free() is call whenever page refcount reach 1 (which means
the page is free as ZONE_DEVICE page never reach a refcount of 0). This
allow device driver to manage its memory and associated struct page.

The second callback page_fault() happens when there is a CPU access to
an address that is back by a device page (which are un-addressable by the
CPU). This callback is responsible to migrate the page back to system
main memory. Device driver can not block migration back to system memory,
HMM make sure that such page can not be pin into device memory.

If device is in some error condition and can not migrate memory back then
a CPU page fault to device memory should end with SIGBUS.

Changed since v2:
  - s/DEVICE_UNADDRESSABLE/DEVICE_PRIVATE
Changed since v1:
  - rename to device private memory (from device unaddressable)

Signed-off-by: JA(C)rA'me Glisse <jglisse@redhat.com>
Acked-by: Dan Williams <dan.j.williams@intel.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 fs/proc/task_mmu.c       |  7 +++++
 include/linux/ioport.h   |  1 +
 include/linux/memremap.h | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mm.h       | 12 ++++++++
 include/linux/swap.h     | 24 ++++++++++++++--
 include/linux/swapops.h  | 68 +++++++++++++++++++++++++++++++++++++++++++++
 kernel/memremap.c        | 34 +++++++++++++++++++++++
 mm/Kconfig               | 13 +++++++++
 mm/memory.c              | 61 ++++++++++++++++++++++++++++++++++++++++
 mm/memory_hotplug.c      | 10 +++++--
 mm/mprotect.c            | 14 ++++++++++
 11 files changed, 311 insertions(+), 5 deletions(-)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index f0c8b33..90b2fa4 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -542,6 +542,8 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr,
 			}
 		} else if (is_migration_entry(swpent))
 			page = migration_entry_to_page(swpent);
+		else if (is_device_private_entry(swpent))
+			page = device_private_entry_to_page(swpent);
 	} else if (unlikely(IS_ENABLED(CONFIG_SHMEM) && mss->check_shmem_swap
 							&& pte_none(*pte))) {
 		page = find_get_entry(vma->vm_file->f_mapping,
@@ -704,6 +706,8 @@ static int smaps_hugetlb_range(pte_t *pte, unsigned long hmask,
 
 		if (is_migration_entry(swpent))
 			page = migration_entry_to_page(swpent);
+		else if (is_device_private_entry(swpent))
+			page = device_private_entry_to_page(swpent);
 	}
 	if (page) {
 		int mapcount = page_mapcount(page);
@@ -1196,6 +1200,9 @@ static pagemap_entry_t pte_to_pagemap_entry(struct pagemapread *pm,
 		flags |= PM_SWAP;
 		if (is_migration_entry(entry))
 			page = migration_entry_to_page(entry);
+
+		if (is_device_private_entry(entry))
+			page = device_private_entry_to_page(entry);
 	}
 
 	if (page && !PageAnon(page))
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 6230064..3a4f691 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -130,6 +130,7 @@ enum {
 	IORES_DESC_ACPI_NV_STORAGE		= 3,
 	IORES_DESC_PERSISTENT_MEMORY		= 4,
 	IORES_DESC_PERSISTENT_MEMORY_LEGACY	= 5,
+	IORES_DESC_DEVICE_PRIVATE_MEMORY	= 6,
 };
 
 /* helpers to define resources */
diff --git a/include/linux/memremap.h b/include/linux/memremap.h
index 9341619..0fcf840 100644
--- a/include/linux/memremap.h
+++ b/include/linux/memremap.h
@@ -4,6 +4,8 @@
 #include <linux/ioport.h>
 #include <linux/percpu-refcount.h>
 
+#include <asm/pgtable.h>
+
 struct resource;
 struct device;
 
@@ -35,18 +37,88 @@ static inline struct vmem_altmap *to_vmem_altmap(unsigned long memmap_start)
 }
 #endif
 
+/*
+ * Specialize ZONE_DEVICE memory into multiple types each having differents
+ * usage.
+ *
+ * MEMORY_DEVICE_PUBLIC:
+ * Persistent device memory (pmem): struct page might be allocated in different
+ * memory and architecture might want to perform special actions. It is similar
+ * to regular memory, in that the CPU can access it transparently. However,
+ * it is likely to have different bandwidth and latency than regular memory.
+ * See Documentation/nvdimm/nvdimm.txt for more information.
+ *
+ * MEMORY_DEVICE_PRIVATE:
+ * Device memory that is not directly addressable by the CPU: CPU can neither
+ * read nor write _UNADDRESSABLE memory. In this case, we do still have struct
+ * pages backing the device memory. Doing so simplifies the implementation, but
+ * it is important to remember that there are certain points at which the struct
+ * page must be treated as an opaque object, rather than a "normal" struct page.
+ * A more complete discussion of unaddressable memory may be found in
+ * include/linux/hmm.h and Documentation/vm/hmm.txt.
+ */
+enum memory_type {
+	MEMORY_DEVICE_PUBLIC = 0,
+	MEMORY_DEVICE_PRIVATE,
+};
+
+/*
+ * For MEMORY_DEVICE_PRIVATE we use ZONE_DEVICE and extend it with two
+ * callbacks:
+ *   page_fault()
+ *   page_free()
+ *
+ * Additional notes about MEMORY_DEVICE_PRIVATE may be found in
+ * include/linux/hmm.h and Documentation/vm/hmm.txt. There is also a brief
+ * explanation in include/linux/memory_hotplug.h.
+ *
+ * The page_fault() callback must migrate page back, from device memory to
+ * system memory, so that the CPU can access it. This might fail for various
+ * reasons (device issues,  device have been unplugged, ...). When such error
+ * conditions happen, the page_fault() callback must return VM_FAULT_SIGBUS and
+ * set the CPU page table entry to "poisoned".
+ *
+ * Note that because memory cgroup charges are transferred to the device memory,
+ * this should never fail due to memory restrictions. However, allocation
+ * of a regular system page might still fail because we are out of memory. If
+ * that happens, the page_fault() callback must return VM_FAULT_OOM.
+ *
+ * The page_fault() callback can also try to migrate back multiple pages in one
+ * chunk, as an optimization. It must, however, prioritize the faulting address
+ * over all the others.
+ *
+ *
+ * The page_free() callback is called once the page refcount reaches 1
+ * (ZONE_DEVICE pages never reach 0 refcount unless there is a refcount bug.
+ * This allows the device driver to implement its own memory management.)
+ */
+typedef int (*dev_page_fault_t)(struct vm_area_struct *vma,
+				unsigned long addr,
+				struct page *page,
+				unsigned int flags,
+				pmd_t *pmdp);
+typedef void (*dev_page_free_t)(struct page *page, void *data);
+
 /**
  * struct dev_pagemap - metadata for ZONE_DEVICE mappings
+ * @page_fault: callback when CPU fault on an unaddressable device page
+ * @page_free: free page callback when page refcount reaches 1
  * @altmap: pre-allocated/reserved memory for vmemmap allocations
  * @res: physical address range covered by @ref
  * @ref: reference count that pins the devm_memremap_pages() mapping
  * @dev: host device of the mapping for debug
+ * @data: private data pointer for page_free()
+ * @type: memory type: see MEMORY_* in memory_hotplug.h
  */
 struct dev_pagemap {
+	dev_page_fault_t page_fault;
+	dev_page_free_t page_free;
 	struct vmem_altmap *altmap;
 	const struct resource *res;
 	struct percpu_ref *ref;
 	struct device *dev;
+	void *data;
+	enum memory_type type;
 };
 
 #ifdef CONFIG_ZONE_DEVICE
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 7cb17c6..a825dab 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -788,11 +788,23 @@ static inline bool is_zone_device_page(const struct page *page)
 {
 	return page_zonenum(page) == ZONE_DEVICE;
 }
+
+static inline bool is_device_private_page(const struct page *page)
+{
+	/* See MEMORY_DEVICE_PRIVATE in include/linux/memory_hotplug.h */
+	return ((page_zonenum(page) == ZONE_DEVICE) &&
+		(page->pgmap->type == MEMORY_DEVICE_PRIVATE));
+}
 #else
 static inline bool is_zone_device_page(const struct page *page)
 {
 	return false;
 }
+
+static inline bool is_device_private_page(const struct page *page)
+{
+	return false;
+}
 #endif
 
 static inline void get_page(struct page *page)
diff --git a/include/linux/swap.h b/include/linux/swap.h
index 5ab1c98..ab6c20b 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -51,6 +51,23 @@ static inline int current_is_kswapd(void)
  */
 
 /*
+ * Unaddressable device memory support. See include/linux/hmm.h and
+ * Documentation/vm/hmm.txt. Short description is we need struct pages for
+ * device memory that is unaddressable (inaccessible) by CPU, so that we can
+ * migrate part of a process memory to device memory.
+ *
+ * When a page is migrated from CPU to device, we set the CPU page table entry
+ * to a special SWP_DEVICE_* entry.
+ */
+#ifdef CONFIG_DEVICE_PRIVATE
+#define SWP_DEVICE_NUM 2
+#define SWP_DEVICE_WRITE (MAX_SWAPFILES+SWP_HWPOISON_NUM+SWP_MIGRATION_NUM)
+#define SWP_DEVICE_READ (MAX_SWAPFILES+SWP_HWPOISON_NUM+SWP_MIGRATION_NUM+1)
+#else
+#define SWP_DEVICE_NUM 0
+#endif
+
+/*
  * NUMA node memory migration support
  */
 #ifdef CONFIG_MIGRATION
@@ -72,7 +89,8 @@ static inline int current_is_kswapd(void)
 #endif
 
 #define MAX_SWAPFILES \
-	((1 << MAX_SWAPFILES_SHIFT) - SWP_MIGRATION_NUM - SWP_HWPOISON_NUM)
+	((1 << MAX_SWAPFILES_SHIFT) - SWP_DEVICE_NUM - \
+	SWP_MIGRATION_NUM - SWP_HWPOISON_NUM)
 
 /*
  * Magic header for a swap area. The first part of the union is
@@ -432,8 +450,8 @@ static inline void show_swap_cache_info(void)
 {
 }
 
-#define free_swap_and_cache(swp)	is_migration_entry(swp)
-#define swapcache_prepare(swp)		is_migration_entry(swp)
+#define free_swap_and_cache(e) (is_migration_entry(e) || is_device_private_entry(e))
+#define swapcache_prepare(e) (is_migration_entry(e) || is_device_private_entry(e))
 
 static inline int add_swap_count_continuation(swp_entry_t swp, gfp_t gfp_mask)
 {
diff --git a/include/linux/swapops.h b/include/linux/swapops.h
index 5c3a5f3..361090c 100644
--- a/include/linux/swapops.h
+++ b/include/linux/swapops.h
@@ -100,6 +100,74 @@ static inline void *swp_to_radix_entry(swp_entry_t entry)
 	return (void *)(value | RADIX_TREE_EXCEPTIONAL_ENTRY);
 }
 
+#if IS_ENABLED(CONFIG_DEVICE_PRIVATE)
+static inline swp_entry_t make_device_private_entry(struct page *page, bool write)
+{
+	return swp_entry(write ? SWP_DEVICE_WRITE : SWP_DEVICE_READ,
+			 page_to_pfn(page));
+}
+
+static inline bool is_device_private_entry(swp_entry_t entry)
+{
+	int type = swp_type(entry);
+	return type == SWP_DEVICE_READ || type == SWP_DEVICE_WRITE;
+}
+
+static inline void make_device_private_entry_read(swp_entry_t *entry)
+{
+	*entry = swp_entry(SWP_DEVICE_READ, swp_offset(*entry));
+}
+
+static inline bool is_write_device_private_entry(swp_entry_t entry)
+{
+	return unlikely(swp_type(entry) == SWP_DEVICE_WRITE);
+}
+
+static inline struct page *device_private_entry_to_page(swp_entry_t entry)
+{
+	return pfn_to_page(swp_offset(entry));
+}
+
+int device_private_entry_fault(struct vm_area_struct *vma,
+		       unsigned long addr,
+		       swp_entry_t entry,
+		       unsigned int flags,
+		       pmd_t *pmdp);
+#else /* CONFIG_DEVICE_PRIVATE */
+static inline swp_entry_t make_device_private_entry(struct page *page, bool write)
+{
+	return swp_entry(0, 0);
+}
+
+static inline void make_device_private_entry_read(swp_entry_t *entry)
+{
+}
+
+static inline bool is_device_private_entry(swp_entry_t entry)
+{
+	return false;
+}
+
+static inline bool is_write_device_private_entry(swp_entry_t entry)
+{
+	return false;
+}
+
+static inline struct page *device_private_entry_to_page(swp_entry_t entry)
+{
+	return NULL;
+}
+
+static inline int device_private_entry_fault(struct vm_area_struct *vma,
+				     unsigned long addr,
+				     swp_entry_t entry,
+				     unsigned int flags,
+				     pmd_t *pmdp)
+{
+	return VM_FAULT_SIGBUS;
+}
+#endif /* CONFIG_DEVICE_PRIVATE */
+
 #ifdef CONFIG_MIGRATION
 static inline swp_entry_t make_migration_entry(struct page *page, int write)
 {
diff --git a/kernel/memremap.c b/kernel/memremap.c
index 124bed7..cd596d4 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -18,6 +18,8 @@
 #include <linux/io.h>
 #include <linux/mm.h>
 #include <linux/memory_hotplug.h>
+#include <linux/swap.h>
+#include <linux/swapops.h>
 
 #ifndef ioremap_cache
 /* temporary while we convert existing ioremap_cache users to memremap */
@@ -182,6 +184,34 @@ struct page_map {
 	struct vmem_altmap altmap;
 };
 
+#if IS_ENABLED(CONFIG_DEVICE_PRIVATE)
+int device_private_entry_fault(struct vm_area_struct *vma,
+		       unsigned long addr,
+		       swp_entry_t entry,
+		       unsigned int flags,
+		       pmd_t *pmdp)
+{
+	struct page *page = device_private_entry_to_page(entry);
+
+	/*
+	 * The page_fault() callback must migrate page back to system memory
+	 * so that CPU can access it. This might fail for various reasons
+	 * (device issue, device was unsafely unplugged, ...). When such
+	 * error conditions happen, the callback must return VM_FAULT_SIGBUS.
+	 *
+	 * Note that because memory cgroup charges are accounted to the device
+	 * memory, this should never fail because of memory restrictions (but
+	 * allocation of regular system page might still fail because we are
+	 * out of memory).
+	 *
+	 * There is a more in-depth description of what that callback can and
+	 * cannot do, in include/linux/memremap.h
+	 */
+	return page->pgmap->page_fault(vma, addr, page, flags, pmdp);
+}
+EXPORT_SYMBOL(device_private_entry_fault);
+#endif /* CONFIG_DEVICE_PRIVATE */
+
 static void pgmap_radix_release(struct resource *res)
 {
 	resource_size_t key, align_start, align_size, align_end;
@@ -321,6 +351,10 @@ void *devm_memremap_pages(struct device *dev, struct resource *res,
 	}
 	pgmap->ref = ref;
 	pgmap->res = &page_map->res;
+	pgmap->type = MEMORY_DEVICE_PUBLIC;
+	pgmap->page_fault = NULL;
+	pgmap->page_free = NULL;
+	pgmap->data = NULL;
 
 	mutex_lock(&pgmap_lock);
 	error = 0;
diff --git a/mm/Kconfig b/mm/Kconfig
index d744cff..f5357ff 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -736,6 +736,19 @@ config ZONE_DEVICE
 
 	  If FS_DAX is enabled, then say Y.
 
+config DEVICE_PRIVATE
+	bool "Unaddressable device memory (GPU memory, ...)"
+	depends on X86_64
+	depends on ZONE_DEVICE
+	depends on MEMORY_HOTPLUG
+	depends on MEMORY_HOTREMOVE
+	depends on SPARSEMEM_VMEMMAP
+
+	help
+	  Allows creation of struct pages to represent unaddressable device
+	  memory; i.e., memory that is only accessible from the device (or
+	  group of devices).
+
 config FRAME_VECTOR
 	bool
 
diff --git a/mm/memory.c b/mm/memory.c
index d320b4e..eba61dd 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -49,6 +49,7 @@
 #include <linux/swap.h>
 #include <linux/highmem.h>
 #include <linux/pagemap.h>
+#include <linux/memremap.h>
 #include <linux/ksm.h>
 #include <linux/rmap.h>
 #include <linux/export.h>
@@ -927,6 +928,35 @@ copy_one_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm,
 					pte = pte_swp_mksoft_dirty(pte);
 				set_pte_at(src_mm, addr, src_pte, pte);
 			}
+		} else if (is_device_private_entry(entry)) {
+			page = device_private_entry_to_page(entry);
+
+			/*
+			 * Update rss count even for unaddressable pages, as
+			 * they should treated just like normal pages in this
+			 * respect.
+			 *
+			 * We will likely want to have some new rss counters
+			 * for unaddressable pages, at some point. But for now
+			 * keep things as they are.
+			 */
+			get_page(page);
+			rss[mm_counter(page)]++;
+			page_dup_rmap(page, false);
+
+			/*
+			 * We do not preserve soft-dirty information, because so
+			 * far, checkpoint/restore is the only feature that
+			 * requires that. And checkpoint/restore does not work
+			 * when a device driver is involved (you cannot easily
+			 * save and restore device driver state).
+			 */
+			if (is_write_device_private_entry(entry) &&
+			    is_cow_mapping(vm_flags)) {
+				make_device_private_entry_read(&entry);
+				pte = swp_entry_to_pte(entry);
+				set_pte_at(src_mm, addr, src_pte, pte);
+			}
 		}
 		goto out_set_pte;
 	}
@@ -1243,6 +1273,29 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
 			}
 			continue;
 		}
+
+		entry = pte_to_swp_entry(ptent);
+		if (non_swap_entry(entry) && is_device_private_entry(entry)) {
+			struct page *page = device_private_entry_to_page(entry);
+
+			if (unlikely(details && details->check_mapping)) {
+				/*
+				 * unmap_shared_mapping_pages() wants to
+				 * invalidate cache without truncating:
+				 * unmap shared but keep private pages.
+				 */
+				if (details->check_mapping !=
+				    page_rmapping(page))
+					continue;
+			}
+
+			pte_clear_not_present_full(mm, addr, pte, tlb->fullmm);
+			rss[mm_counter(page)]--;
+			page_remove_rmap(page, false);
+			put_page(page);
+			continue;
+		}
+
 		/* If details->check_mapping, we leave swap entries. */
 		if (unlikely(details))
 			continue;
@@ -2690,6 +2743,14 @@ int do_swap_page(struct vm_fault *vmf)
 		if (is_migration_entry(entry)) {
 			migration_entry_wait(vma->vm_mm, vmf->pmd,
 					     vmf->address);
+		} else if (is_device_private_entry(entry)) {
+			/*
+			 * For un-addressable device memory we call the pgmap
+			 * fault handler callback. The callback must migrate
+			 * the page back to some CPU accessible page.
+			 */
+			ret = device_private_entry_fault(vma, vmf->address, entry,
+						 vmf->flags, vmf->pmd);
 		} else if (is_hwpoison_entry(entry)) {
 			ret = VM_FAULT_HWPOISON;
 		} else {
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 599c675..0a9f690 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -156,7 +156,7 @@ void mem_hotplug_done(void)
 /* add this memory to iomem resource */
 static struct resource *register_memory_resource(u64 start, u64 size)
 {
-	struct resource *res;
+	struct resource *res, *conflict;
 	res = kzalloc(sizeof(struct resource), GFP_KERNEL);
 	if (!res)
 		return ERR_PTR(-ENOMEM);
@@ -165,7 +165,13 @@ static struct resource *register_memory_resource(u64 start, u64 size)
 	res->start = start;
 	res->end = start + size - 1;
 	res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
-	if (request_resource(&iomem_resource, res) < 0) {
+	conflict =  request_resource_conflict(&iomem_resource, res);
+	if (conflict) {
+		if (conflict->desc == IORES_DESC_DEVICE_PRIVATE_MEMORY) {
+			pr_debug("Device unaddressable memory block "
+				 "memory hotplug at %#010llx !\n",
+				 (unsigned long long)start);
+		}
 		pr_debug("System RAM resource %pR cannot be added\n", res);
 		kfree(res);
 		return ERR_PTR(-EEXIST);
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 1a8c9ca..868d0ed 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -124,6 +124,20 @@ static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
 
 				pages++;
 			}
+
+			if (is_write_device_private_entry(entry)) {
+				pte_t newpte;
+
+				/*
+				 * We do not preserve soft-dirtiness. See
+				 * copy_one_pte() for explanation.
+				 */
+				make_device_private_entry_read(&entry);
+				newpte = swp_entry_to_pte(entry);
+				set_pte_at(mm, addr, pte, newpte);
+
+				pages++;
+			}
 		}
 	} while (pte++, addr += PAGE_SIZE, addr != end);
 	arch_leave_lazy_mmu_mode();
-- 
2.9.4

--
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: kmemleak: Treat vm_struct as alternative reference to vmalloc'ed objects
From: Luis R. Rodriguez @ 2017-05-23 20:37 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: linux-mm, linux-kernel, Michal Hocko, Andy Lutomirski,
	Luis R. Rodriguez
In-Reply-To: <1495474514-24425-1-git-send-email-catalin.marinas@arm.com>

On Mon, May 22, 2017 at 06:35:14PM +0100, Catalin Marinas wrote:
> Kmemleak requires that vmalloc'ed objects have a minimum reference count
> of 2: one in the corresponding vm_struct object and the other owned by
> the vmalloc() caller. There are cases, however, where the original
> vmalloc() returned pointer is lost and, instead, a pointer to vm_struct
> is stored (see free_thread_stack()). Kmemleak currently reports such
> objects as leaks.
> 
> This patch adds support for treating any surplus references to an object
> as additional references to a specified object. It introduces the
> kmemleak_vmalloc() API function which takes a vm_struct pointer and sets
> its surplus reference passing to the actual vmalloc() returned pointer.
> The __vmalloc_node_range() calling site has been modified accordingly.
> 
> An unrelated minor change is included in this patch to change the type
> of kmemleak_object.flags to unsigned int (previously unsigned long).
> 
> Reported-by: "Luis R. Rodriguez" <mcgrof@kernel.org>

Tested-by: Luis R. Rodriguez <mcgrof@kernel.org>

> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index 20036d4f9f13..11ab654502fd 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -1188,6 +1249,30 @@ static bool update_checksum(struct kmemleak_object *object)
>  }
>  
>  /*
> + * Update an object's references. object->lock must be held by the caller.
> + */
> +static void update_refs(struct kmemleak_object *object)
> +{
> +	if (!color_white(object)) {
> +		/* non-orphan, ignored or new */
> +		return;
> +	}
> +
> +	/*
> +	 * Increase the object's reference count (number of pointers to the
> +	 * memory block). If this count reaches the required minimum, the
> +	 * object's color will become gray and it will be added to the
> +	 * gray_list.
> +	 */
> +	object->count++;
> +	if (color_gray(object)) {
> +		/* put_object() called when removing from gray_list */
> +		WARN_ON(!get_object(object));
> +		list_add_tail(&object->gray_list, &gray_list);
> +	}
> +}
> +
> +/*

This an initial use of it seems to be very possible and likely without the
vmalloc special case, ie, can this be added as a separate patch to make the
actual functional change easier to read ?

  Luis

>   * Memory scanning is a long process and it needs to be interruptable. This
>   * function checks whether such interrupt condition occurred.
>   */
> @@ -1224,6 +1309,7 @@ static void scan_block(void *_start, void *_end,
>  	for (ptr = start; ptr < end; ptr++) {
>  		struct kmemleak_object *object;
>  		unsigned long pointer;
> +		unsigned long excess_ref;
>  
>  		if (scan_should_stop())
>  			break;
> @@ -1259,25 +1345,25 @@ static void scan_block(void *_start, void *_end,
>  		 * enclosed by scan_mutex.
>  		 */
>  		spin_lock_nested(&object->lock, SINGLE_DEPTH_NESTING);
> -		if (!color_white(object)) {
> -			/* non-orphan, ignored or new */
> -			spin_unlock(&object->lock);
> -			continue;
> -		}
> +		/* only pass surplus references (object already gray) */
> +		if (color_gray(object))
> +			excess_ref = object->excess_ref;
> +		else
> +			excess_ref = 0;
> +		update_refs(object);
> +		spin_unlock(&object->lock);
>  
> -		/*
> -		 * Increase the object's reference count (number of pointers
> -		 * to the memory block). If this count reaches the required
> -		 * minimum, the object's color will become gray and it will be
> -		 * added to the gray_list.
> -		 */
> -		object->count++;
> -		if (color_gray(object)) {
> -			/* put_object() called when removing from gray_list */
> -			WARN_ON(!get_object(object));
> -			list_add_tail(&object->gray_list, &gray_list);
> +		if (excess_ref) {
> +			object = lookup_object(excess_ref, 0);
> +			if (!object)
> +				continue;
> +			if (object == scanned)
> +				/* circular reference, ignore */
> +				continue;
> +			spin_lock_nested(&object->lock, SINGLE_DEPTH_NESTING);
> +			update_refs(object);
> +			spin_unlock(&object->lock);
>  		}
> -		spin_unlock(&object->lock);
>  	}
>  	read_unlock_irqrestore(&kmemleak_lock, flags);
>  }
> @@ -1980,6 +2066,10 @@ void __init kmemleak_init(void)
>  		case KMEMLEAK_NO_SCAN:
>  			kmemleak_no_scan(log->ptr);
>  			break;
> +		case KMEMLEAK_SET_EXCESS_REF:
> +			object_set_excess_ref((unsigned long)log->ptr,
> +					      log->excess_ref);
> +			break;
>  		default:
>  			kmemleak_warn("Unknown early log operation: %d\n",
>  				      log->op_type);
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 34a1c3e46ed7..b805cc5ecca0 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1759,12 +1759,7 @@ void *__vmalloc_node_range(unsigned long size, unsigned long align,
>  	 */
>  	clear_vm_uninitialized_flag(area);
>  
> -	/*
> -	 * A ref_count = 2 is needed because vm_struct allocated in
> -	 * __get_vm_area_node() contains a reference to the virtual address of
> -	 * the vmalloc'ed block.
> -	 */
> -	kmemleak_alloc(addr, real_size, 2, gfp_mask);
> +	kmemleak_vmalloc(area, size, gfp_mask);
>  
>  	return addr;
>  
> 

-- 
Luis Rodriguez, SUSE LINUX GmbH
Maxfeldstrasse 5; D-90409 Nuernberg

--
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 1/1] Sealable memory support
From: Kees Cook @ 2017-05-23 20:11 UTC (permalink / raw)
  To: Igor Stoppa
  Cc: Casey Schaufler, Michal Hocko, Dave Hansen, Laura Abbott,
	Linux-MM, kernel-hardening@lists.openwall.com, LKML, Daniel Micay,
	Greg KH, James Morris, Stephen Smalley
In-Reply-To: <656b6465-16cd-ab0a-b439-ab5bea42006d@huawei.com>

On Tue, May 23, 2017 at 2:43 AM, Igor Stoppa <igor.stoppa@huawei.com> wrote:
> On 23/05/17 00:38, Kees Cook wrote:
>> On Fri, May 19, 2017 at 3:38 AM, Igor Stoppa <igor.stoppa@huawei.com> wrote:
>
> [...]
>
>> For the first bit of bikeshedding, should this really be called
>> seal/unseal? My mind is probably just broken from having read TPM
>> documentation, but this isn't really "sealing" as I'd understand it
>> (it's not tied to a credential, for example). It's "only" rw/ro.
>> Perhaps "protect/unprotect" or just simply "readonly/writable", and
>> call the base function "romalloc"?
>
> I was not aware of the specific mean of "seal", in this context.
> The term was implicitly proposed by Michal Hocko, while discussing about
> the mechanism and I liked it more than what I was using initially:
> "lockable".
>
> tbh I like the sound of "smalloc" better than "romalloc"
>
> But this is really the least of my worries :-P
>
>> This is fundamentally a heap allocator, with linked lists, etc. I'd
>> like to see as much attention as possible given to hardening it
>> against attacks, especially adding redzoning around the metadata at
>> least, and perhaps requiring that CONFIG_DEBUG_LIST be enabled.
>
> My initial goal was to provide something that is useful without
> affecting performance.
>
> You seem to be pushing for a more extreme approach.

I don't know about pushing, but it seemed like the API provided for
arbitrary unsealing, etc. More below...

> While I have nothing against it and I actually agree that it can be
> useful, I would not make it mandatory.
>
> More on this later.
>
>> And as
>> part of that, I'd like hardened usercopy to grow knowledge of these
>> allocations so we can bounds-check objects. Right now, mm/usercopy.c
>> just looks at PageSlab(page) to decide if it should do slab checks. I
>> think adding a check for this type of object would be very important
>> there.
>
> I am not familiar with this and I need to study it, however I still
> think that if there is a significant trade-off in terms of performance
> vs resilience, it should be optional, for those who want it.

I would want hardened usercopy support as a requirement for using
smalloc(). Without it, we're regressing the over-read protection that
already exists for slab objects, if kernel code switched from slab to
smalloc. It should be very similar to the existing slab checks. "Is
this a smalloc object? Have we read beyond the end of a given object?"
etc. The metadata is all there, except for an efficient way to mark a
page as a smalloc page, but I think that just requires a new Page$Name
bit test, as done for slab.

> Maybe there could be a master toggle for these options, if it makes
> sense to group them logically. How does this sound?
>
>> The ro/rw granularity here is the _entire_ pool, not a specific
>> allocation (or page containing the allocation). I'm concerned that
>> makes this very open to race conditions where, especially in the
>> global pool, one thread can be trying to write to ro data in a pool
>> and another has made the pool writable.
>
> I have the impression we are thinking to something different.
> Close, but different enough.
>
> First of all, how using a mutex can create races?
> Do you mean with respect of other resources that might be held by
> competing users of the pool?

I meant this:

CPU 1     CPU 2
create
alloc
write
seal
...
unseal
                write
write
seal

The CPU 2 write would be, for example, an attacker using a
vulnerability to attempt to write to memory in the sealed area. All it
would need to do to succeed would be to trigger an action in the
kernel that would do a "legitimate" write (which requires the unseal),
and race it. Unsealing should be CPU-local, if the API is going to
support this kind of access.

> That, imho, is a locking problem that cannot be solved here.
> You can try to mitigate it, by reducing the chances it will happen, but
> basically you are trying to make do with an user of the API that is not
> implementing locking correctly.
> I'd say that it's better to fix the user.
>
> If you meant something else, I really didn't get it :-)
>
> More about the frequency of the access: you seem to expect very often
> seal/unseal - or lock/unlock, while I don't.

I am more concerned about _any_ unseal after initial seal. And even
then, it'd be nice to keep things CPU-local. My concerns are related
to the write-rarely proposal (https://lkml.org/lkml/2017/3/29/704)
which is kind of like this, but focused on the .data section, not
dynamic memory. It has similar concerns about CPU-locality.
Additionally, even writing to memory and then making it read-only
later runs risks (see threads about BPF JIT races vs making things
read-only: https://patchwork.kernel.org/patch/9662653/ Alexei's NAK
doesn't change the risk this series is fixing: races with attacker
writes during assignment but before read-only marking).

So, while smalloc would hugely reduce the window an attacker has
available to change data contents, this API doesn't eliminate it. (To
eliminate it, there would need to be a CPU-local page permission view
that let only the current CPU to the page, and then restore it to
read-only to match the global read-only view.)

> What I envision as primary case for unlocking is the tear down of the
> entire pool, for example preceding the unloading of the module.
>
> Think about __ro_after_init : once it is r/o, it stays r/o.

Ah! In that case, sure. This isn't what the proposed API provided,
though, so let's adjust it to only perform the unseal at destroy time.
That makes it much saner, IMO. "Write once" dynamic allocations, or
"read-only after seal". woalloc? :P yay naming

> Which also means that there would be possibly a busy transient, when
> allocations are made. That is true.
> But only for shared pools. If a module uses one pool, I would expect the
> initialization of the module to be mostly sequential.
> So, no chance of races. Do you agree?

I think a shared global pool would need to be eliminated for true
write-once semantics.

> Furthermore, if a module _really_ wants to do parallel allocation, then
> maybe it's simpler and cleaner to have one pool per "thread" or whatever
> is the mechanism used.

Right.

> The global pool is mostly there for completing the offering of
> __ro_after_init. If one wants ot use it, it's available.
> It saves the trouble of having own pool, it means competing with the
> rest of the kernel for locking.
>
> If it seems a bad idea, it can be easily removed.

I think they need to be explicit, yes.

> I'd rather not add extra locking to something that doesn't need it:
> Allocate - write - seal - read, read, read, ... - unseal - destroy.

Yup, I would be totally fine with this. It still has a race between
allocate and seal, but it's a huge improvement over the existing state
of the world where all dynamic memory is writable. :)

>> Finding a user for this would help clarify its protection properties,
>> too. (The LSM example is likely not the best starting point for that,
>> as it would depend on other changes that are under discussion.)
>
> Well, I *have* to do the LSM - and SE Linux too.
>
> That's where this started and why I'm here with this patch.

Ah, okay. Most of the LSM is happily covered by __ro_after_init. If we
could just drop the runtime disabling of SELinux, we'd be fine.

I'd like to see what you have in mind for LSM, since the only runtime
change would be the SELinux piece, and that would violate the
alloc-write-seal-read-read-destroy lifetime (since you'd need to
unseal-write-seal to change the state of SELinux during runtime).

> From what I could understand about the discussion, the debate about the
> changes is settled (is it?).
>
> Casey seemed to be ok with this.
>
> If something else is needed, ok, I can do that too, but I didn't see any
> specific reason why LSM should not be good enough as first example.
>
> If you can provide a specific reason why it's not suitable, I can
> reconsider.

Hopefully we're on the same page now? I love being able to work off a
patch example, so if you agree about the changes in lifetime
management, and you've got some examples of how/where to use smalloc,
I'd love to review those too. I think there are a LOT of places this
could get used.

> [...]
>
>>> +       if (!pool) {
>>> +               pr_err("No memory for allocating pool.");
>>
>> It might be handy to have pools named like they are for the slab allocator.
>
> I considered this.
> The only reason I could come up with why it might be desirable is if the
> same pool needs to be accessed from two or more places that do not share
> the pointer. It doesn't seem particularly useful.
> The only upside I can think of is that it would save a memory page, vs
> the case of creating 2 separate pools.
> It would also introduce locking cross-dependency.
>
> Did I overlook some reason why it would be desirable?

Hm, I just meant add a char[] to the metadata and pass it in during
create(). Then it's possible to report which smalloc cache is being
examined during hardened usercopy checks.

>
> [...]
>
>>> +int smalloc_destroy(struct smalloc_pool *pool)
>>> +{
>>> +       struct list_head *pos, *q;
>>> +       struct smalloc_node *node;
>>> +
>>> +       if (!pool)
>>> +               return -EINVAL;
>
> locking was missing, I added it in the new version
> also I moved to goto-like error handling, since there were several
> similar exit paths.

It seems like smalloc pools could also be refcounted?

> [...]
>
>>> +typedef uint64_t align_t;
>>> +
>>> +enum seal_t {
>>> +       SMALLOC_UNSEALED,
>>> +       SMALLOC_SEALED,
>>> +};
>>> +
>>> +#define __SMALLOC_ALIGNED__ __aligned(sizeof(align_t))
>
> How about the alignment? Is it a desirable feature?
> Did I overlook some reason why it would not work?

I think alignment makes sense. It makes sure there aren't crazy
problems with structs allocated after the header.

>>> +#define NODE_HEADER                                    \
>>> +       struct {                                        \
>>> +               __SMALLOC_ALIGNED__ struct {            \
>>> +                       struct list_head list;          \
>>> +                       align_t *free;                  \
>>> +                       unsigned long available_words;  \
>>> +               };                                      \
>>> +       }
>
> Does this look ok? ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

It's probably a sufficient starting point, depending on how the API
shakes out. Without unseal-write-seal properties, I case much less
about redzoning, etc.

> [...]
>
>> I'd really like to see kernel-doc for the API functions (likely in the .c file).
>
> Yes, I just see no point right now, since the API doesn't seem to be
> agreed/finalized yet.

Sure thing. I guess I was looking for more docs because of my
questions/concerns about lifetime management, etc. :)

>> Thanks again for working on this! If you can find examples of file
>> operations living in the heap, those would be great examples for using
>> this API (assuming the other properties can be improved).
>
> As I explained above, I would prefer to continue with LSM, unless you
> have some specific reason against it.

I think I just lack the imagination for what you're trying to do that
isn't already covered by the existing __ro_after_init changes. :)

> If you have some example in mind about file operations living on the
> heap - which I suspect you do have :) - we could discuss also about
> those and if the locking needs to be modified, but from my perspective
> the use case is significantly different and I wouldn't pile it up with this.
> Of course I might be missing the point you are trying to make.
> In that case, I'm afraid that further explanation is needed from you, as
> I do not get it :-)

Well, a poor example would be struct sock, since it needs to be
regularly written to, but it has function pointers near the end which
have been a very common target for attackers. (Though this is less so
now that INET_DIAG no longer exposes the kernel addresses to allocated
struct socks.)

> Thanks a lot for the review, this is exactly the sort of early feedback
> I was hoping to receive.

Great! Gaining a ro_after_init-like API for dynamic memory would be
quite valuable, I think. And once we have a write-rarely
infrastructure for the .rodata segment, perhaps we can extend it to
the dynamic memory areas too (in the future).

Thanks!

-Kees

-- 
Kees Cook
Pixel Security

--
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: [HMM 12/15] mm/migrate: new memory migration helper for use with device memory v4
From: Reza Arbab @ 2017-05-23 18:07 UTC (permalink / raw)
  To: Jérôme Glisse
  Cc: akpm, linux-kernel, linux-mm, John Hubbard, David Nellans,
	Evgeny Baskakov, Mark Hairgrove, Sherry Cheung, Subhash Gutti
In-Reply-To: <20170522165206.6284-13-jglisse@redhat.com>

On Mon, May 22, 2017 at 12:52:03PM -0400, Jerome Glisse wrote:
>This patch add a new memory migration helpers, which migrate memory
>backing a range of virtual address of a process to different memory
>(which can be allocated through special allocator). It differs from
>numa migration by working on a range of virtual address and thus by
>doing migration in chunk that can be large enough to use DMA engine
>or special copy offloading engine.
>
>Expected users are any one with heterogeneous memory where different
>memory have different characteristics (latency, bandwidth, ...). As
>an example IBM platform with CAPI bus can make use of this feature
>to migrate between regular memory and CAPI device memory. New CPU
>architecture with a pool of high performance memory not manage as
>cache but presented as regular memory (while being faster and with
>lower latency than DDR) will also be prime user of this patch.
>
>Migration to private device memory will be useful for device that
>have large pool of such like GPU, NVidia plans to use HMM for that.

Acked-by: Reza Arbab <arbab@linux.vnet.ibm.com>

-- 
Reza Arbab

--
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: make kswapd try harder to keep active pages in cache
From: Rik van Riel @ 2017-05-23 17:14 UTC (permalink / raw)
  To: Josef Bacik, akpm, kernel-team, hannes, linux-mm
In-Reply-To: <1495549403-3719-1-git-send-email-jbacik@fb.com>

[-- Attachment #1: Type: text/plain, Size: 667 bytes --]

On Tue, 2017-05-23 at 10:23 -0400, Josef Bacik wrote:

> My approach here is twofold.  First, keep track of the difference in
> inactive and slab pages since the last time kswapd ran.  In the first
> run this will just be the overall counts of inactive and slab, but
> for
> each subsequent run we'll have a good idea of where the memory
> pressure
> is coming from.  Then we use this information to put pressure on
> either
> the inactive lists or the slab caches, depending on where the
> pressure
> is coming from.

> Signed-off-by: Josef Bacik <jbacik@fb.com>

This looks totally reasonable to me.

Acked-by: Rik van Riel <riel@redhat.com>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply

* Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems
From: Doug Anderson @ 2017-05-23 17:12 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: David Rientjes, Andrew Morton, Christoph Lameter, Pekka Enberg,
	Joonsoo Kim, linux-mm, linux-kernel@vger.kernel.org
In-Reply-To: <20170523165608.GN141096@google.com>

Hi,

On Tue, May 23, 2017 at 9:56 AM, Matthias Kaehlcke <mka@chromium.org> wrote:
> Hi David,
>
> El Mon, May 22, 2017 at 06:35:23PM -0700 David Rientjes ha dit:
>
>> On Mon, 22 May 2017, Andrew Morton wrote:
>>
>> > > > Is clang not inlining kmalloc_large_node_hook() for some reason?  I don't
>> > > > think this should ever warn on gcc.
>> > >
>> > > clang warns about unused static inline functions outside of header
>> > > files, in difference to gcc.
>> >
>> > I wish it wouldn't.  These patches just add clutter.
>> >
>>
>> Matthias, what breaks if you do this?
>>
>> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
>> index de179993e039..e1895ce6fa1b 100644
>> --- a/include/linux/compiler-clang.h
>> +++ b/include/linux/compiler-clang.h
>> @@ -15,3 +15,8 @@
>>   * with any version that can compile the kernel
>>   */
>>  #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
>> +
>> +#ifdef inline
>> +#undef inline
>> +#define inline __attribute__((unused))
>> +#endif
>
> Thanks for the suggestion!

Wow, I totally didn't think of this either when talking with Matthias
about this.  I guess it makes the assumption that nobody else is
hacking "inline" like we are, but "what are the chances of someone
doing that (TM)"  ;-)


> Nothing breaks and the warnings are silenced. It seems we could use
> this if there is a stong opposition against having warnings on unused
> static inline functions in .c files.
>
> Still I am not convinced that gcc's behavior is preferable in this
> case. True, it saves us from adding a bunch of __maybe_unused or
> #ifdefs, on the other hand the warning is a useful tool to spot truly
> unused code. So far about 50% of the warnings I looked into fall into
> this category.

Personally I actually prefer clang's behavior to gcc's here and I
think Matthias's patches are actually doing a service to the
community.  As he points out, many of the patches he's posted have
removed totally dead code from the kernel.  This code has often
existed for many years but never been noticed.  While the code wasn't
hurting anyone (it was dead and the compiler wasn't generating any
code for it), it would certainly add confusion to anyone reading the
source code.

Obviously my opinion isn't as important as the opinion of upstream
maintainers, but hopefully you'll consider it anyway.  :)  If you
still want to just make clang behave like gcc then I think David's
suggestion is a great one.


-Doug

--
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 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems
From: Matthias Kaehlcke @ 2017-05-23 16:56 UTC (permalink / raw)
  To: David Rientjes
  Cc: Andrew Morton, Christoph Lameter, Pekka Enberg, Joonsoo Kim,
	linux-mm, linux-kernel, Douglas Anderson
In-Reply-To: <alpine.DEB.2.10.1705221834440.13805@chino.kir.corp.google.com>

Hi David,

El Mon, May 22, 2017 at 06:35:23PM -0700 David Rientjes ha dit:

> On Mon, 22 May 2017, Andrew Morton wrote:
> 
> > > > Is clang not inlining kmalloc_large_node_hook() for some reason?  I don't 
> > > > think this should ever warn on gcc.
> > > 
> > > clang warns about unused static inline functions outside of header
> > > files, in difference to gcc.
> > 
> > I wish it wouldn't.  These patches just add clutter.
> > 
> 
> Matthias, what breaks if you do this?
> 
> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> index de179993e039..e1895ce6fa1b 100644
> --- a/include/linux/compiler-clang.h
> +++ b/include/linux/compiler-clang.h
> @@ -15,3 +15,8 @@
>   * with any version that can compile the kernel
>   */
>  #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
> +
> +#ifdef inline
> +#undef inline
> +#define inline __attribute__((unused))
> +#endif

Thanks for the suggestion!

Nothing breaks and the warnings are silenced. It seems we could use
this if there is a stong opposition against having warnings on unused
static inline functions in .c files.

Still I am not convinced that gcc's behavior is preferable in this
case. True, it saves us from adding a bunch of __maybe_unused or
#ifdefs, on the other hand the warning is a useful tool to spot truly
unused code. So far about 50% of the warnings I looked into fall into
this category.

--
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: dm ioctl: Restore __GFP_HIGH in copy_params()
From: Mikulas Patocka @ 2017-05-23 16:44 UTC (permalink / raw)
  To: Michal Hocko
  Cc: David Rientjes, Mike Snitzer, Junaid Shahid, Alasdair Kergon,
	Andrew Morton, linux-mm, andreslc, gthelen, vbabka, linux-kernel
In-Reply-To: <20170523060534.GA12813@dhcp22.suse.cz>



On Tue, 23 May 2017, Michal Hocko wrote:

> On Mon 22-05-17 13:35:41, David Rientjes wrote:
> > On Mon, 22 May 2017, Mike Snitzer wrote:
> [...]
> > > While adding the __GFP_NOFAIL flag would serve to document expectations
> > > I'm left unconvinced that the memory allocator will _not fail_ for an
> > > order-0 page -- as Mikulas said most ioctls don't need more than 4K.
> > 
> > __GFP_NOFAIL would make no sense in kvmalloc() calls, ever, it would never 
> > fallback to vmalloc :)
> 
> Sorry, I could have been more specific. You would have to opencode
> kvmalloc obviously. It is documented to not support this flag for the
> reasons you have mentioned above.
> 
> > I'm hoping this can get merged during the 4.12 window to fix the broken 
> > commit d224e9381897.
> 
> I obviously disagree. Relying on memory reserves for _correctness_ is
> clearly broken by design, full stop. But it is dm code and you are going
> it is responsibility of the respective maintainers to support this code.

Block loop device is broken in the same way - it converts block requests 
to filesystem reads and writes and those FS reads and writes allocate 
memory.

Network block device needs an userspace daemon to perform I/O.

iSCSI also needs to allocate memory to perform I/O.

NFS and other networking filesystems are also broken in the same way (they 
need to receive a packet to acknowledge a write and packet reception needs 
to allocate memory).

So - what should these *broken* drivers do to reduce the possibility of 
the deadlock?

Mikulas

> -- 
> 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 v3 3/6] mm/hugetlb: add size parameter to huge_pte_offset()
From: Punit Agrawal @ 2017-05-23 16:13 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, akpm, linux-mm, linux-kernel, linux-arm-kernel,
	catalin.marinas, will.deacon, n-horiguchi, kirill.shutemov,
	mike.kravetz, steve.capper, mark.rutland, hillf.zj, linux-arch,
	aneesh.kumar, Tony Luck, Fenghua Yu, James Hogan, Ralf Baechle,
	James E.J. Bottomley, Helge Deller, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Martin Schwidefsky,
	Heiko Carstens, Yoshinori Sato, Rich Felker, David S. Miller,
	Chris Metcalf, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Alexander Viro, Michal Hocko
In-Reply-To: <201705231817.OSRI6iib%fengguang.wu@intel.com>

kbuild test robot <lkp@intel.com> writes:

> Hi Punit,
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.12-rc2 next-20170523]
> [cannot apply to mmotm/master]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Punit-Agrawal/Support-for-contiguous-pte-hugepages/20170523-142407
> config: arm64-defconfig (attached as .config)
> compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
>         wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=arm64 
>
> All errors (new ones prefixed by >>):
>
>    arch/arm64/mm/hugetlbpage.c: In function 'huge_ptep_get_and_clear':
>>> arch/arm64/mm/hugetlbpage.c:200:10: error: too few arguments to function 'huge_pte_offset'
>       cpte = huge_pte_offset(mm, addr);
>              ^~~~~~~~~~~~~~~
>    arch/arm64/mm/hugetlbpage.c:135:8: note: declared here
>     pte_t *huge_pte_offset(struct mm_struct *mm,
>            ^~~~~~~~~~~~~~~
>    arch/arm64/mm/hugetlbpage.c: In function 'huge_ptep_set_access_flags':
>    arch/arm64/mm/hugetlbpage.c:238:10: error: too few arguments to function 'huge_pte_offset'
>       cpte = huge_pte_offset(vma->vm_mm, addr);
>              ^~~~~~~~~~~~~~~
>    arch/arm64/mm/hugetlbpage.c:135:8: note: declared here
>     pte_t *huge_pte_offset(struct mm_struct *mm,
>            ^~~~~~~~~~~~~~~
>    arch/arm64/mm/hugetlbpage.c: In function 'huge_ptep_set_wrprotect':
>    arch/arm64/mm/hugetlbpage.c:263:10: error: too few arguments to function 'huge_pte_offset'
>       cpte = huge_pte_offset(mm, addr);
>              ^~~~~~~~~~~~~~~
>    arch/arm64/mm/hugetlbpage.c:135:8: note: declared here
>     pte_t *huge_pte_offset(struct mm_struct *mm,
>            ^~~~~~~~~~~~~~~
>    arch/arm64/mm/hugetlbpage.c: In function 'huge_ptep_clear_flush':
>    arch/arm64/mm/hugetlbpage.c:280:10: error: too few arguments to function 'huge_pte_offset'
>       cpte = huge_pte_offset(vma->vm_mm, addr);
>              ^~~~~~~~~~~~~~~
>    arch/arm64/mm/hugetlbpage.c:135:8: note: declared here
>     pte_t *huge_pte_offset(struct mm_struct *mm,
>            ^~~~~~~~~~~~~~~

Ok, so we haven't quite managed to remove the dependency of this patch
on the following arm64 changes[0].

I'll post a new version fixing this failure soon.

[0] https://www.spinics.net/lists/arm-kernel/msg582758.html

>
> vim +/huge_pte_offset +200 arch/arm64/mm/hugetlbpage.c
>
> 66b3923a David Woods 2015-12-17  194  	if (pte_cont(*ptep)) {
> 66b3923a David Woods 2015-12-17  195  		int ncontig, i;
> 66b3923a David Woods 2015-12-17  196  		size_t pgsize;
> 66b3923a David Woods 2015-12-17  197  		pte_t *cpte;
> 66b3923a David Woods 2015-12-17  198  		bool is_dirty = false;
> 66b3923a David Woods 2015-12-17  199  
> 66b3923a David Woods 2015-12-17 @200  		cpte = huge_pte_offset(mm, addr);
> 66b3923a David Woods 2015-12-17  201  		ncontig = find_num_contig(mm, addr, cpte, *cpte, &pgsize);
> 66b3923a David Woods 2015-12-17  202  		/* save the 1st pte to return */
> 66b3923a David Woods 2015-12-17  203  		pte = ptep_get_and_clear(mm, addr, cpte);
>
> :::::: The code at line 200 was first introduced by commit
> :::::: 66b3923a1a0f77a563b43f43f6ad091354abbfe9 arm64: hugetlb: add support for PTE contiguous bit
>
> :::::: TO: David Woods <dwoods@ezchip.com>
> :::::: CC: Will Deacon <will.deacon@arm.com>
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

--
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 0/6] refine and rename slub sysfs
From: Christoph Lameter @ 2017-05-23 16:07 UTC (permalink / raw)
  To: Michal Hocko; +Cc: Wei Yang, penberg, rientjes, akpm, linux-mm, linux-kernel
In-Reply-To: <20170523063911.GC12813@dhcp22.suse.cz>

On Tue, 23 May 2017, Michal Hocko wrote:

> > >_Why_ do we need all this?
> >
> > To have a clear statistics for each slab level.
>
> Is this worth risking breakage of the userspace which consume this data
> now? Do you have any user space code which will greatly benefit from the
> new data and which couldn't do the same with the current format/output?
>
> If yes this all should be in the changelog.

And the patchset would also need to update the user space tool that is in
the kernel tree...

Again Wei please do not use "level". Type?

--
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 2/6] mm, gup: Ensure real head page is ref-counted when using hugepages
From: Punit Agrawal @ 2017-05-23 15:43 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: akpm, linux-mm, linux-kernel, linux-arm-kernel, catalin.marinas,
	will.deacon, n-horiguchi, kirill.shutemov, mike.kravetz,
	steve.capper, mark.rutland, hillf.zj, linux-arch, aneesh.kumar,
	Michal Hocko
In-Reply-To: <20170523131312.aim6obne2t5sxtdr@node.shutemov.name>

"Kirill A. Shutemov" <kirill@shutemov.name> writes:

> On Mon, May 22, 2017 at 02:36:00PM +0100, Punit Agrawal wrote:
>> When speculatively taking references to a hugepage using
>> page_cache_add_speculative() in gup_huge_pmd(), it is assumed that the
>> page returned by pmd_page() is the head page. Although normally true,
>> this assumption doesn't hold when the hugepage comprises of successive
>> page table entries such as when using contiguous bit on arm64 at PTE or
>> PMD levels.
>> 
>> This can be addressed by ensuring that the page passed to
>> page_cache_add_speculative() is the real head or by de-referencing the
>> head page within the function.
>> 
>> We take the first approach to keep the usage pattern aligned with
>> page_cache_get_speculative() where users already pass the appropriate
>> page, i.e., the de-referenced head.
>> 
>> Apply the same logic to fix gup_huge_[pud|pgd]() as well.
>
> Hm. Okay. But I'm kinda surprise that this is the only place that need to
> be adjusted.
>
> Have you validated all other pmd_page() use-cases?

I came across the gup issues were found while investigating a failing
test from mce-tests.

I think the problem here is not due to the use of pmd_page() but because
page_cache_[add|get]_speculative() don't ensure they ref-count the head
page as is done in get_page().

Having said that, I had a quick look at the other uses of pmd_page() -

Quite a few of them are followed by an explicit BUG_ON() to check that
the page returned is a head page. All other instances seem to be dealing
with transparent hugepages where contiguous hugepages are not supported.

I don't see any call sites that ring alarm bells.

Did you have any particular part of the code in mind where pmd_page()
usage might be a problem?

Thanks,
Punit

--
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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox