Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [RFC v3 14/17] x86/mm: Update the handle_speculative_fault's path
From: Laurent Dufour @ 2017-04-27 15:52 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora
In-Reply-To: <1493308376-23851-1-git-send-email-ldufour@linux.vnet.ibm.com>

If handle_speculative_fault failed due to a VM ERROR, we try again the
slow path to allow the signal to be delivered.

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 arch/x86/mm/fault.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index ee6d8799d958..8f3bd8a53d66 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -1320,10 +1320,14 @@ __do_page_fault(struct pt_regs *regs, unsigned long error_code,
 		fault = handle_speculative_fault(mm, address,
 					flags & ~FAULT_FLAG_ALLOW_RETRY);
 
-		if (fault & VM_FAULT_RETRY)
-			goto retry;
-
-		goto done;
+		/*
+		 * We also check against VM_FAULT_ERROR because we have to
+		 * raise a signal by calling later mm_fault_error() which
+		 * requires the vma pointer to be set. So in that case,
+		 * we fall through the normal path.
+		 */
+		if (!(fault & VM_FAULT_RETRY || fault & VM_FAULT_ERROR))
+			goto done;
 	}
 
 	/*
@@ -1429,20 +1433,13 @@ __do_page_fault(struct pt_regs *regs, unsigned long error_code,
 		return;
 	}
 
-	if (unlikely(fault & VM_FAULT_RETRY)) {
-		if (fatal_signal_pending(current))
-			return;
-
-		goto done;
-	}
-
 	up_read(&mm->mmap_sem);
-done:
 	if (unlikely(fault & VM_FAULT_ERROR)) {
 		mm_fault_error(regs, error_code, address, vma, fault);
 		return;
 	}
 
+done:
 	/*
 	 * Major/minor page fault accounting. If any of the events
 	 * returned VM_FAULT_MAJOR, we account it as a major fault.
-- 
2.7.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

* [RFC v3 15/17] mm/spf: Add check on the VMA's flags
From: Laurent Dufour @ 2017-04-27 15:52 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora
In-Reply-To: <1493308376-23851-1-git-send-email-ldufour@linux.vnet.ibm.com>

When handling speculative page fault we should check for the VMA's
access permission as it is done in handle_mm_fault() or access_error
in x86's fault handler.

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 mm/memory.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/mm/memory.c b/mm/memory.c
index 3b28de5838c7..4d9c6331ada1 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3756,6 +3756,30 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
 	if (address < vma->vm_start || vma->vm_end <= address)
 		goto unlock;
 
+	/* XXX Could we handle huge page here ? */
+	if (unlikely(is_vm_hugetlb_page(vma)))
+		goto unlock;
+
+	/*
+	 * The three following checks are copied from access_error from
+	 * arch/x86/mm/fault.c
+	 * XXX they may not be applicable to all architectures
+	 */
+	if (!arch_vma_access_permitted(vma, flags & FAULT_FLAG_WRITE,
+				       flags & FAULT_FLAG_INSTRUCTION,
+				       flags & FAULT_FLAG_REMOTE))
+		goto unlock;
+
+	/* This is one is required to check that the VMA has write access set */
+	if (flags & FAULT_FLAG_WRITE) {
+		if (unlikely(!(vma->vm_flags & VM_WRITE)))
+			goto unlock;
+	} else {
+		/* XXX This may not be required */
+		if (unlikely(!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE))))
+			goto unlock;
+	}
+
 	/*
 	 * We need to re-validate the VMA after checking the bounds, otherwise
 	 * we might have a false positive on the bounds.
-- 
2.7.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

* [RFC v3 17/17] mm/spf: protect mremap() against speculative pf
From: Laurent Dufour @ 2017-04-27 15:52 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora
In-Reply-To: <1493308376-23851-1-git-send-email-ldufour@linux.vnet.ibm.com>

mremap() is modifying the VMA layout and thus must be protected against
the speculative page fault handler.

XXX: Is the change to vma->vm_flags to set VM_ACCOUNT require the
protection ?

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 mm/mremap.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/mm/mremap.c b/mm/mremap.c
index 30d7d2482eea..40c3c869dffc 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -288,6 +288,10 @@ static unsigned long move_vma(struct vm_area_struct *vma,
 	if (!new_vma)
 		return -ENOMEM;
 
+	write_seqcount_begin(&vma->vm_sequence);
+	write_seqcount_begin_nested(&new_vma->vm_sequence,
+				    SINGLE_DEPTH_NESTING);
+
 	moved_len = move_page_tables(vma, old_addr, new_vma, new_addr, old_len,
 				     need_rmap_locks);
 	if (moved_len < old_len) {
@@ -304,6 +308,7 @@ static unsigned long move_vma(struct vm_area_struct *vma,
 		 */
 		move_page_tables(new_vma, new_addr, vma, old_addr, moved_len,
 				 true);
+		write_seqcount_end(&vma->vm_sequence);
 		vma = new_vma;
 		old_len = new_len;
 		old_addr = new_addr;
@@ -311,7 +316,9 @@ static unsigned long move_vma(struct vm_area_struct *vma,
 	} else {
 		arch_remap(mm, old_addr, old_addr + old_len,
 			   new_addr, new_addr + new_len);
+		write_seqcount_end(&vma->vm_sequence);
 	}
+	write_seqcount_end(&new_vma->vm_sequence);
 
 	/* Conceal VM_ACCOUNT so old reservation is not undone */
 	if (vm_flags & VM_ACCOUNT) {
-- 
2.7.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

* [RFC v3 16/17] mm: protect madvise vs speculative pf
From: Laurent Dufour @ 2017-04-27 15:52 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora
In-Reply-To: <1493308376-23851-1-git-send-email-ldufour@linux.vnet.ibm.com>

This is an attempt to protect madvise's effect against the speculative
page fault handler.

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 mm/madvise.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/mm/madvise.c b/mm/madvise.c
index 0e3828eae9f8..f91b64564571 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -132,8 +132,9 @@ static long madvise_behavior(struct vm_area_struct *vma,
 	/*
 	 * vm_flags is protected by the mmap_sem held in write mode.
 	 */
+	write_seqcount_begin(&vma->vm_sequence);
 	vma->vm_flags = new_flags;
-
+	write_seqcount_end(&vma->vm_sequence);
 out:
 	if (error == -ENOMEM)
 		error = -EAGAIN;
@@ -403,9 +404,11 @@ static void madvise_free_page_range(struct mmu_gather *tlb,
 		.private = tlb,
 	};
 
+	write_seqcount_begin(&vma->vm_sequence);
 	tlb_start_vma(tlb, vma);
 	walk_page_range(addr, end, &free_walk);
 	tlb_end_vma(tlb, vma);
+	write_seqcount_end(&vma->vm_sequence);
 }
 
 static int madvise_free_single_vma(struct vm_area_struct *vma,
-- 
2.7.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, zone_device: replace {get, put}_zone_device_page() with a single reference
From: Logan Gunthorpe @ 2017-04-27 16:11 UTC (permalink / raw)
  To: Dan Williams, akpm
  Cc: linux-mm, Jérôme Glisse, linux-kernel, Kirill Shutemov
In-Reply-To: <149325431313.40660.7404075559824162131.stgit@dwillia2-desk3.amr.corp.intel.com>



On 26/04/17 06:55 PM, Dan Williams wrote:
> @@ -277,7 +269,10 @@ struct dev_pagemap *find_dev_pagemap(resource_size_t phys)
>   *
>   * Notes:
>   * 1/ @ref must be 'live' on entry and 'dead' before devm_memunmap_pages() time
> - *    (or devm release event).
> + *    (or devm release event). The expected order of events is that @ref has
> + *    been through percpu_ref_kill() before devm_memremap_pages_release(). The
> + *    wait for the completion of kill and percpu_ref_exit() must occur after
> + *    devm_memremap_pages_release().
>   *
>   * 2/ @res is expected to be a host memory range that could feasibly be
>   *    treated as a "System RAM" range, i.e. not a device mmio range, but
> @@ -379,6 +374,7 @@ void *devm_memremap_pages(struct device *dev, struct resource *res,
>  		 */
>  		list_del(&page->lru);
>  		page->pgmap = pgmap;
> +		percpu_ref_get(ref);
>  	}
>  	devres_add(dev, page_map);
>  	return __va(res->start);
> diff --git a/mm/swap.c b/mm/swap.c
> index 5dabf444d724..01267dda6668 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -97,6 +97,16 @@ static void __put_compound_page(struct page *page)
>  
>  void __put_page(struct page *page)
>  {
> +	if (is_zone_device_page(page)) {
> +		put_dev_pagemap(page->pgmap);
> +
> +		/*
> +		 * The page belong to device, do not return it to
> +		 * page allocator.
> +		 */
> +		return;
> +	}
> +
>  	if (unlikely(PageCompound(page)))
>  		__put_compound_page(page);
>  	else
> 

Forgive me if I'm missing something but this doesn't make sense to me.
We are taking a reference once when the region is initialized and
releasing it every time a page within the region's reference count drops
to zero. That does not seem to be symmetric and I don't see how it
tracks that pages are in use. Shouldn't get_dev_pagemap be called when
any page is allocated or something like that (ie. the inverse of
__put_page)?

Thanks,

Logan

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v5 09/32] x86/mm: Provide general kernel support for memory encryption
From: Borislav Petkov @ 2017-04-27 16:12 UTC (permalink / raw)
  To: Tom Lendacky
  Cc: linux-arch, linux-efi, kvm, linux-doc, x86, kexec, linux-kernel,
	kasan-dev, linux-mm, iommu, Rik van Riel,
	Radim Krčmář, Toshimitsu Kani, Arnd Bergmann,
	Jonathan Corbet, Matt Fleming, Michael S. Tsirkin, Joerg Roedel,
	Konrad Rzeszutek Wilk, Paolo Bonzini, Larry Woodman,
	Brijesh Singh, Ingo Molnar, Andy Lutomirski, H. Peter Anvin,
	Andrey Ryabinin, Alexander Potapenko, Dave Young, Thomas Gleixner,
	Dmitry Vyukov
In-Reply-To: <20170418211754.10190.25082.stgit@tlendack-t1.amdoffice.net>

On Tue, Apr 18, 2017 at 04:17:54PM -0500, Tom Lendacky wrote:
> Changes to the existing page table macros will allow the SME support to
> be enabled in a simple fashion with minimal changes to files that use these
> macros.  Since the memory encryption mask will now be part of the regular
> pagetable macros, we introduce two new macros (_PAGE_TABLE_NOENC and
> _KERNPG_TABLE_NOENC) to allow for early pagetable creation/initialization
> without the encryption mask before SME becomes active.  Two new pgprot()
> macros are defined to allow setting or clearing the page encryption mask.

...

> @@ -55,7 +57,7 @@ static inline void copy_user_page(void *to, void *from, unsigned long vaddr,
>  	__phys_addr_symbol(__phys_reloc_hide((unsigned long)(x)))
>  
>  #ifndef __va
> -#define __va(x)			((void *)((unsigned long)(x)+PAGE_OFFSET))
> +#define __va(x)			((void *)(__sme_clr(x) + PAGE_OFFSET))
>  #endif
>  
>  #define __boot_va(x)		__va(x)
> diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h
> index 7bd0099..fead0a5 100644
> --- a/arch/x86/include/asm/page_types.h
> +++ b/arch/x86/include/asm/page_types.h
> @@ -15,7 +15,7 @@
>  #define PUD_PAGE_SIZE		(_AC(1, UL) << PUD_SHIFT)
>  #define PUD_PAGE_MASK		(~(PUD_PAGE_SIZE-1))
>  
> -#define __PHYSICAL_MASK		((phys_addr_t)((1ULL << __PHYSICAL_MASK_SHIFT) - 1))
> +#define __PHYSICAL_MASK		((phys_addr_t)(__sme_clr((1ULL << __PHYSICAL_MASK_SHIFT) - 1)))

That looks strange: poking SME mask hole into a mask...?

>  #define __VIRTUAL_MASK		((1UL << __VIRTUAL_MASK_SHIFT) - 1)
>  
>  /* Cast *PAGE_MASK to a signed type so that it is sign-extended if

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH] mm, zone_device: replace {get, put}_zone_device_page() with a single reference
From: Dan Williams @ 2017-04-27 16:14 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: Andrew Morton, Linux MM, Jérôme Glisse,
	linux-kernel@vger.kernel.org, Kirill Shutemov
In-Reply-To: <3e595ba6-2ea1-e25d-e254-6c7edcf23f88@deltatee.com>

On Thu, Apr 27, 2017 at 9:11 AM, Logan Gunthorpe <logang@deltatee.com> wrote:
>
>
> On 26/04/17 06:55 PM, Dan Williams wrote:
>> @@ -277,7 +269,10 @@ struct dev_pagemap *find_dev_pagemap(resource_size_t phys)
>>   *
>>   * Notes:
>>   * 1/ @ref must be 'live' on entry and 'dead' before devm_memunmap_pages() time
>> - *    (or devm release event).
>> + *    (or devm release event). The expected order of events is that @ref has
>> + *    been through percpu_ref_kill() before devm_memremap_pages_release(). The
>> + *    wait for the completion of kill and percpu_ref_exit() must occur after
>> + *    devm_memremap_pages_release().
>>   *
>>   * 2/ @res is expected to be a host memory range that could feasibly be
>>   *    treated as a "System RAM" range, i.e. not a device mmio range, but
>> @@ -379,6 +374,7 @@ void *devm_memremap_pages(struct device *dev, struct resource *res,
>>                */
>>               list_del(&page->lru);
>>               page->pgmap = pgmap;
>> +             percpu_ref_get(ref);
>>       }
>>       devres_add(dev, page_map);
>>       return __va(res->start);
>> diff --git a/mm/swap.c b/mm/swap.c
>> index 5dabf444d724..01267dda6668 100644
>> --- a/mm/swap.c
>> +++ b/mm/swap.c
>> @@ -97,6 +97,16 @@ static void __put_compound_page(struct page *page)
>>
>>  void __put_page(struct page *page)
>>  {
>> +     if (is_zone_device_page(page)) {
>> +             put_dev_pagemap(page->pgmap);
>> +
>> +             /*
>> +              * The page belong to device, do not return it to
>> +              * page allocator.
>> +              */
>> +             return;
>> +     }
>> +
>>       if (unlikely(PageCompound(page)))
>>               __put_compound_page(page);
>>       else
>>
>
> Forgive me if I'm missing something but this doesn't make sense to me.
> We are taking a reference once when the region is initialized and
> releasing it every time a page within the region's reference count drops
> to zero. That does not seem to be symmetric and I don't see how it
> tracks that pages are in use. Shouldn't get_dev_pagemap be called when
> any page is allocated or something like that (ie. the inverse of
> __put_page)?

You're overlooking that the page reference count 1 after
arch_add_memory(). So at the end of time we're just dropping the
arch_add_memory() reference to release the page and related
dev_pagemap.

--
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, zone_device: replace {get, put}_zone_device_page() with a single reference
From: Logan Gunthorpe @ 2017-04-27 16:33 UTC (permalink / raw)
  To: Dan Williams
  Cc: Andrew Morton, Linux MM, Jérôme Glisse,
	linux-kernel@vger.kernel.org, Kirill Shutemov
In-Reply-To: <CAPcyv4it4eGhLjws_j8+M1BeAzr_gHRZ4zE-nC+4QMpFp72Hyg@mail.gmail.com>



On 27/04/17 10:14 AM, Dan Williams wrote:
> You're overlooking that the page reference count 1 after
> arch_add_memory(). So at the end of time we're just dropping the
> arch_add_memory() reference to release the page and related
> dev_pagemap.

Thanks, that does actually make a lot more sense to me now. However,
there still appears to be an asymmetry in that the pgmap->ref is
incremented once and decremented once per page...

Logan

--
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, zone_device: replace {get, put}_zone_device_page() with a single reference
From: Dan Williams @ 2017-04-27 16:38 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: Andrew Morton, Linux MM, Jérôme Glisse,
	linux-kernel@vger.kernel.org, Kirill Shutemov
In-Reply-To: <d2b0159c-9227-3283-2a57-74e03d47a0cd@deltatee.com>

On Thu, Apr 27, 2017 at 9:33 AM, Logan Gunthorpe <logang@deltatee.com> wrote:
>
>
> On 27/04/17 10:14 AM, Dan Williams wrote:
>> You're overlooking that the page reference count 1 after
>> arch_add_memory(). So at the end of time we're just dropping the
>> arch_add_memory() reference to release the page and related
>> dev_pagemap.
>
> Thanks, that does actually make a lot more sense to me now. However,
> there still appears to be an asymmetry in that the pgmap->ref is
> incremented once and decremented once per page...
>

No, this hunk...

@@ -379,6 +374,7 @@ void *devm_memremap_pages(struct device *dev,
struct resource *res,
                 */
                list_del(&page->lru);
                page->pgmap = pgmap;
+               percpu_ref_get(ref);
        }


...is inside a for_each_device_pfn() loop.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v5 08/11] mm: hwpoison: soft offline supports thp migration
From: Zi Yan @ 2017-04-27 16:39 UTC (permalink / raw)
  To: Naoya Horiguchi
  Cc: Anshuman Khandual, Zi Yan, kirill.shutemov@linux.intel.com,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	akpm@linux-foundation.org, minchan@kernel.org, vbabka@suse.cz,
	mgorman@techsingularity.net, mhocko@kernel.org,
	dnellans@nvidia.com
In-Reply-To: <20170427044112.GA18781@hori1.linux.bs1.fc.nec.co.jp>

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



Naoya Horiguchi wrote:
> On Fri, Apr 21, 2017 at 10:55:49AM -0500, Zi Yan wrote:
>>
>> Anshuman Khandual wrote:
>>> On 04/21/2017 02:17 AM, Zi Yan wrote:
>>>> From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
>>>>
>>>> This patch enables thp migration for soft offline.
>>>>
>>>> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
>>>>
>>>> ChangeLog: v1 -> v5:
>>>> - fix page isolation counting error
>>>>
>>>> Signed-off-by: Zi Yan <zi.yan@cs.rutgers.edu>
>>>> ---
>>>>  mm/memory-failure.c | 35 ++++++++++++++---------------------
>>>>  1 file changed, 14 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>>>> index 9b77476ef31f..23ff02eb3ed4 100644
>>>> --- a/mm/memory-failure.c
>>>> +++ b/mm/memory-failure.c
>>>> @@ -1481,7 +1481,17 @@ static struct page *new_page(struct page *p, unsigned long private, int **x)
>>>>  	if (PageHuge(p))
>>>>  		return alloc_huge_page_node(page_hstate(compound_head(p)),
>>>>  						   nid);
>>>> -	else
>>>> +	else if (thp_migration_supported() && PageTransHuge(p)) {
>>>> +		struct page *thp;
>>>> +
>>>> +		thp = alloc_pages_node(nid,
>>>> +			(GFP_TRANSHUGE | __GFP_THISNODE) & ~__GFP_RECLAIM,
>>> Why not __GFP_RECLAIM ? Its soft offline path we wait a bit before
>>> declaring that THP page cannot be allocated and hence should invoke
>>> reclaim methods as well.
>> I am not sure how much effort the kernel wants to put here to soft
>> offline a THP. Naoya knows more here.
> 
> What I thought at first was that soft offline is not an urgent user
> and no need to reclaim (i.e. give a little impact on other thread.)
> But that's not a strong opinion, so if you like __GFP_RECLAIM here,
> I'm fine about that.

OK, I will add __GFP_RECLAIM.

> 
>>
>>>> +			HPAGE_PMD_ORDER);
>>>> +		if (!thp)
>>>> +			return NULL;
>>>> +		prep_transhuge_page(thp);
>>>> +		return thp;
>>>> +	} else
>>>>  		return __alloc_pages_node(nid, GFP_HIGHUSER_MOVABLE, 0);
>>>>  }
>>>>  
>>>> @@ -1665,8 +1675,8 @@ static int __soft_offline_page(struct page *page, int flags)
>>>>  		 * cannot have PAGE_MAPPING_MOVABLE.
>>>>  		 */
>>>>  		if (!__PageMovable(page))
>>>> -			inc_node_page_state(page, NR_ISOLATED_ANON +
>>>> -						page_is_file_cache(page));
>>>> +			mod_node_page_state(page_pgdat(page), NR_ISOLATED_ANON +
>>>> +						page_is_file_cache(page), hpage_nr_pages(page));
>>>>  		list_add(&page->lru, &pagelist);
>>>>  		ret = migrate_pages(&pagelist, new_page, NULL, MPOL_MF_MOVE_ALL,
>>>>  					MIGRATE_SYNC, MR_MEMORY_FAILURE);
>>>> @@ -1689,28 +1699,11 @@ static int __soft_offline_page(struct page *page, int flags)
>>>>  static int soft_offline_in_use_page(struct page *page, int flags)
>>>>  {
>>>>  	int ret;
>>>> -	struct page *hpage = compound_head(page);
>>>> -
>>>> -	if (!PageHuge(page) && PageTransHuge(hpage)) {
>>>> -		lock_page(hpage);
>>>> -		if (!PageAnon(hpage) || unlikely(split_huge_page(hpage))) {
>>>> -			unlock_page(hpage);
>>>> -			if (!PageAnon(hpage))
>>>> -				pr_info("soft offline: %#lx: non anonymous thp\n", page_to_pfn(page));
>>>> -			else
>>>> -				pr_info("soft offline: %#lx: thp split failed\n", page_to_pfn(page));
>>>> -			put_hwpoison_page(hpage);
>>>> -			return -EBUSY;
>>>> -		}
>>>> -		unlock_page(hpage);
>>>> -		get_hwpoison_page(page);
>>>> -		put_hwpoison_page(hpage);
>>>> -	}
>>>>  
>>>>  	if (PageHuge(page))
>>>>  		ret = soft_offline_huge_page(page, flags);
>>>>  	else
>>>> -		ret = __soft_offline_page(page, flags);
>>>> +		ret = __soft_offline_page(compound_head(page), flags);
>>> Hmm, what if the THP allocation fails in the new_page() path and
>>> we fallback for general page allocation. In that case we will
>>> always be still calling with the head page ? Because we dont
>>> split the huge page any more.
>> This could be a problem if the user wants to offline a TailPage but due
>> to THP allocation failure, the HeadPage is offlined.
> 
> Right, "retry with split" part is unfinished, so we need some improvement.
> 
>> It may be better to only soft offline THPs if page ==
>> compound_head(page). If page != compound_head(page), we still split THPs
>> like before.
>>
>> Because in migrate_pages(), we cannot guarantee any TailPages in that
>> THP are migrated (1. THP allocation failure causes THP splitting, then
>> only HeadPage is going to be migrated; 2. even if we change existing
>> migrate_pages() implementation to add all TailPages to migration list
>> instead of LRU list, we still cannot guarantee the TailPage we want to
>> migrate is migrated.).
>>
>> Naoya, what do you think?
> 
> Maybe soft offline is a special caller of page migration because it
> basically wants to migrate only one page, but thp migration still has
> a benefit because we can avoid thp split.
> So I like that we try thp migration at first, and if it fails we fall
> back to split and migrate (only) a raw error page. This should be done
> in caller side for soft offline, because it knows where the error page is.

Make sense. So when migrate_pages() sees the migrate reason is
MR_MEMORY_FAILURE, it will not split THP when newpage allocation fails.
Then, the soft offline caller will split failed THP and retry migrating
the error subpage. I can do that.

> 
> As for generic case (for other migration callers which mainly want to
> migrate multiple pages for their purpose,) thp split and retry can be
> done in common migration code. After thp split, all subpages are linked
> to migration list, then we retry without returning to the caller.
> So I think that split_huge_page() can be moved to (for example) for-loop
> in migrate_pages().
> 
> I tried to write a patch for it last year, but considering vm event
> accounting, the patch might be large (~100 lines).

Yes. I saw your code on your github. I can pick it up and send it for
review after this patchset is merged, if you are OK with it.


-- 
Best Regards,
Yan Zi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 537 bytes --]

^ permalink raw reply

* Re: [PATCH] mm, zone_device: replace {get, put}_zone_device_page() with a single reference
From: Logan Gunthorpe @ 2017-04-27 16:45 UTC (permalink / raw)
  To: Dan Williams
  Cc: Andrew Morton, Linux MM, Jérôme Glisse,
	linux-kernel@vger.kernel.org, Kirill Shutemov
In-Reply-To: <CAPcyv4iFKb9VbwdjRwF4KLQ=2R6-=vYb6BbHQG6Kk-8QemC6WA@mail.gmail.com>



On 27/04/17 10:38 AM, Dan Williams wrote:
> ...is inside a for_each_device_pfn() loop.
> 

Ah, oops. Then that makes perfect sense. Thanks.

You may have my review tag if you'd like:

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

Logan

--
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, zone_device: replace {get, put}_zone_device_page() with a single reference
From: Dan Williams @ 2017-04-27 16:46 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: Andrew Morton, Linux MM, Jérôme Glisse,
	linux-kernel@vger.kernel.org, Kirill Shutemov
In-Reply-To: <c9fdf838-049c-3e91-08df-c4c7a3806cfb@deltatee.com>

On Thu, Apr 27, 2017 at 9:45 AM, Logan Gunthorpe <logang@deltatee.com> wrote:
>
>
> On 27/04/17 10:38 AM, Dan Williams wrote:
>> ...is inside a for_each_device_pfn() loop.
>>
>
> Ah, oops. Then that makes perfect sense. Thanks.
>
> You may have my review tag if you'd like:
>
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

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

* Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free
From: Tim Chen @ 2017-04-27 16:48 UTC (permalink / raw)
  To: Huang, Ying
  Cc: Minchan Kim, Andrew Morton, linux-mm, linux-kernel, Hugh Dickins,
	Shaohua Li, Rik van Riel
In-Reply-To: <87efwe3as0.fsf@yhuang-dev.intel.com>

On Thu, 2017-04-27 at 09:21 +0800, Huang, Ying wrote:
> Tim Chen <tim.c.chen@linux.intel.com> writes:
> 
> > 
> > > 
> > > 
> > > From 7bd903c42749c448ef6acbbdee8dcbc1c5b498b9 Mon Sep 17 00:00:00 2001
> > > From: Huang Ying <ying.huang@intel.com>
> > > Date: Thu, 23 Feb 2017 13:05:20 +0800
> > > Subject: [PATCH -v5] mm, swap: Sort swap entries before free
> > > 
> > > A 
> > > ---
> > > A mm/swapfile.c | 43 ++++++++++++++++++++++++++++++++++++++-----
> > > A 1 file changed, 38 insertions(+), 5 deletions(-)
> > > diff --git a/mm/swapfile.c b/mm/swapfile.c
> > > index 71890061f653..10e75f9e8ac1 100644
> > > --- a/mm/swapfile.c
> > > +++ b/mm/swapfile.c
> > > @@ -37,6 +37,7 @@
> > > A #include <linux/swapfile.h>
> > > A #include <linux/export.h>
> > > A #include <linux/swap_slots.h>
> > > +#include <linux/sort.h>
> > > A 
> > > A #include <asm/pgtable.h>
> > > A #include <asm/tlbflush.h>
> > > @@ -1065,20 +1066,52 @@ void swapcache_free(swp_entry_t entry)
> > > A 	}
> > > A }
> > > A 
> > > +static int swp_entry_cmp(const void *ent1, const void *ent2)
> > > +{
> > > +	const swp_entry_t *e1 = ent1, *e2 = ent2;
> > > +
> > > +	return (int)(swp_type(*e1) - swp_type(*e2));
> > > +}
> > > +
> > > A void swapcache_free_entries(swp_entry_t *entries, int n)
> > > A {
> > > A 	struct swap_info_struct *p, *prev;
> > > -	int i;
> > > +	int i, m;
> > > +	swp_entry_t entry;
> > > +	unsigned int prev_swp_type;
> > I think it will be clearer to name prev_swp_type as first_swp_type
> > as this is the swp type of the first entry.
> Yes.A A That is better!A A Will do that.
> 
> > 
> > > 
> > > A 
> > > A 	if (n <= 0)
> > > A 		return;
> > > A 
> > > A 	prev = NULL;
> > > A 	p = NULL;
> > > -	for (i = 0; i < n; ++i) {
> > > -		p = swap_info_get_cont(entries[i], prev);
> > > -		if (p)
> > > -			swap_entry_free(p, entries[i]);
> > > +	m = 0;
> > > +	prev_swp_type = swp_type(entries[0]);
> > > +	for (i = 0; i < n; i++) {
> > > +		entry = entries[i];
> > > +		if (likely(swp_type(entry) == prev_swp_type)) {
> > > +			p = swap_info_get_cont(entry, prev);
> > > +			if (likely(p))
> > > +				swap_entry_free(p, entry);
> > > +			prev = p;
> > > +		} else if (!m)
> > > +			m = i;
> > > +	}
> > > +	if (p)
> > > +		spin_unlock(&p->lock);
> > > +	if (likely(!m))
> > > +		return;
> > > +
> > We could still have prev_swp_type at the first entry after sorting.
> > and we can avoid an unlock/relock for this case if we do this:
> > 
> > 	if (likely(!m)) {
> > 		if (p)
> > 			spin_unlock(&p->lock);
> > 		return;
> > 	}
> > 		
> > > 
> > > +	/* Sort swap entries by swap device, so each lock is only taken once. */
> > > +	sort(entries + m, n - m, sizeof(entries[0]), swp_entry_cmp, NULL);
> > > +	prev = NULL;
> > Can eliminate prev=NULL if we adopt the above change.
> > 
> > > 
> > > +	for (i = m; i < n; i++) {
> > > +		entry = entries[i];
> > > +		if (swp_type(entry) == prev_swp_type)
> > > +			continue;
> > The if/continue statement seems incorrect. When swp_type(entry) == prev_swp_type
> > we also need to free entry. A The if/continue statement should be deleted.
> > 
> > Say we have 3 entries with swp_type
> > 1,2,1
> > 
> > We will get prev_swp_type as 1 and free the first entry
> > and sort the remaining two. A The last entry with
> > swp_type 1 will not be freed.
> The first loop in the function will scan all elements of the array, so
> the first and third entry will be freed in the first loop.A A Then the the
> second and the third entry will be sorted.A A But all entries with the
> same swap type (device) of the first entry needn't to be freed again.
> The key point is that we will scan all elements of the array in the
> first loop, record the first entry that has different swap type
> (device).

I was under the wrong impression that the code break from the first
loop when it finds a different swp type. A Yes, we should skip the
free in the second loop if the first loop scan the whole list.

Thanks.

Tim

> 
> Best Regards,
> Huang, Ying
> 
> > 
> > > 
> > > +		p = swap_info_get_cont(entry, prev);
> > > +		if (likely(p))
> > > +			swap_entry_free(p, entry);
> > > A 		prev = p;
> > > A 	}
> > > A 	if (p)
> > Thanks.
> > 
> > Tim

--
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 man-pages 1/2] userfaultfd.2: start documenting non-cooperative events
From: Michael Kerrisk (man-pages) @ 2017-04-27 17:26 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: mtk.manpages, Andrea Arcangeli, linux-kernel, linux-mm, linux-man
In-Reply-To: <1493302474-4701-2-git-send-email-rppt@linux.vnet.ibm.com>

Hi Mike,

I've applied this, but have some questions/points I think 
further clarification.

On 04/27/2017 04:14 PM, Mike Rapoport wrote:
> Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> ---
>  man2/userfaultfd.2 | 135 ++++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 128 insertions(+), 7 deletions(-)
> 
> diff --git a/man2/userfaultfd.2 b/man2/userfaultfd.2
> index cfea5cb..44af3e4 100644
> --- a/man2/userfaultfd.2
> +++ b/man2/userfaultfd.2
> @@ -75,7 +75,7 @@ flag in
>  .PP
>  When the last file descriptor referring to a userfaultfd object is closed,
>  all memory ranges that were registered with the object are unregistered
> -and unread page-fault events are flushed.
> +and unread events are flushed.
>  .\"
>  .SS Usage
>  The userfaultfd mechanism is designed to allow a thread in a multithreaded
> @@ -99,6 +99,20 @@ In such non-cooperative mode,
>  the process that monitors userfaultfd and handles page faults
>  needs to be aware of the changes in the virtual memory layout
>  of the faulting process to avoid memory corruption.
> +
> +Starting from Linux 4.11,
> +userfaultfd may notify the fault-handling threads about changes
> +in the virtual memory layout of the faulting process.
> +In addition, if the faulting process invokes
> +.BR fork (2)
> +system call,
> +the userfaultfd objects associated with the parent may be duplicated
> +into the child process and the userfaultfd monitor will be notified
> +about the file descriptor associated with the userfault objects

What does "notified about the file descriptor" mean?

> +created for the child process,
> +which allows userfaultfd monitor to perform user-space paging
> +for the child process.
> +
>  .\" FIXME elaborate about non-cooperating mode, describe its limitations
>  .\" for kernels before 4.11, features added in 4.11
>  .\" and limitations remaining in 4.11
> @@ -144,6 +158,10 @@ Details of the various
>  operations can be found in
>  .BR ioctl_userfaultfd (2).
>  
> +Since Linux 4.11, events other than page-fault may enabled during
> +.B UFFDIO_API
> +operation.
> +
>  Up to Linux 4.11,
>  userfaultfd can be used only with anonymous private memory mappings.
>  
> @@ -156,7 +174,8 @@ Each
>  .BR read (2)
>  from the userfaultfd file descriptor returns one or more
>  .I uffd_msg
> -structures, each of which describes a page-fault event:
> +structures, each of which describes a page-fault event
> +or an event required for the non-cooperative userfaultfd usage:
>  
>  .nf
>  .in +4n
> @@ -168,6 +187,23 @@ struct uffd_msg {
>              __u64 flags;        /* Flags describing fault */
>              __u64 address;      /* Faulting address */
>          } pagefault;
> +        struct {
> +            __u32 ufd;          /* userfault file descriptor
> +                                   of the child process */
> +        } fork;                 /* since Linux 4.11 */
> +        struct {
> +            __u64 from;         /* old address of the
> +                                   remapped area */
> +            __u64 to;           /* new address of the
> +                                   remapped area */
> +            __u64 len;          /* original mapping length */
> +        } remap;                /* since Linux 4.11 */
> +        struct {
> +            __u64 start;        /* start address of the
> +                                   removed area */
> +            __u64 end;          /* end address of the
> +                                   removed area */
> +        } remove;               /* since Linux 4.11 */
>          ...
>      } arg;
>  
> @@ -194,14 +230,73 @@ structure are as follows:
>  .TP
>  .I event
>  The type of event.
> -Currently, only one value can appear in this field:
> -.BR UFFD_EVENT_PAGEFAULT ,
> -which indicates a page-fault event.
> +Depending of the event type,
> +different fields of the
> +.I arg
> +union represent details required for the event processing.
> +The non-page-fault events are generated only when appropriate feature
> +is enabled during API handshake with
> +.B UFFDIO_API
> +.BR ioctl (2).
> +
> +The following values can appear in the
> +.I event
> +field:
> +.RS
> +.TP
> +.B UFFD_EVENT_PAGEFAULT
> +A page-fault event.
> +The page-fault details are available in the
> +.I pagefault
> +field.
>  .TP
> -.I address
> +.B UFFD_EVENT_FORK
> +Generated when the faulting process invokes
> +.BR fork (2)
> +system call.
> +The event details are available in the
> +.I fork
> +field.
> +.\" FIXME descirbe duplication of userfault file descriptor during fork
> +.TP
> +.B UFFD_EVENT_REMAP
> +Generated when the faulting process invokes
> +.BR mremap (2)
> +system call.
> +The event details are available in the
> +.I remap
> +field.
> +.TP
> +.B UFFD_EVENT_REMOVE
> +Generated when the faulting process invokes
> +.BR madvise (2)
> +system call with
> +.BR MADV_DONTNEED
> +or
> +.BR MADV_REMOVE
> +advice.
> +The event details are available in the
> +.I remove
> +field.
> +.TP
> +.B UFFD_EVENT_UNMAP
> +Generated when the faulting process unmaps a memory range,
> +either explicitly using
> +.BR munmap (2)
> +system call or implicitly during
> +.BR mmap (2)
> +or
> +.BR mremap (2)
> +system calls.
> +The event details are available in the
> +.I remove
> +field.
> +.RE
> +.TP
> +.I pagefault.address
>  The address that triggered the page fault.
>  .TP
> -.I flags
> +.I pagefault.flags
>  A bit mask of flags that describe the event.
>  For
>  .BR UFFD_EVENT_PAGEFAULT ,
> @@ -218,6 +313,32 @@ otherwise it is a read fault.
>  .\"
>  .\" UFFD_PAGEFAULT_FLAG_WP is not yet supported.
>  .RE
> +.TP
> +.I fork.ufd
> +The file descriptor associated with the userfault object
> +created for the child process
> +.TP
> +.I remap.from
> +The original address of the memory range that was remapped using
> +.BR mremap (2).
> +.TP
> +.I remap.to
> +The new address of the memory range that was remapped using
> +.BR mremap (2).
> +.TP
> +.I remap.len
> +The original length of the the memory range that was remapped using
> +.BR mremap (2).
> +.TP
> +.I remove.start
> +The start address of the memory range that was freed using
> +.BR madvise (2)
> +or unmapped
> +.TP
> +.I remove.end
> +The end address of the memory range that was freed using
> +.BR madvise (2)
> +or unmapped
>  .PP
>  A
>  .BR read (2)

Cheers,

Michael



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

--
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 man-pages 2/2] ioctl_userfaultfd.2: start adding details about userfaultfd features
From: Michael Kerrisk (man-pages) @ 2017-04-27 17:26 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: mtk.manpages, Andrea Arcangeli, linux-kernel, linux-mm, linux-man
In-Reply-To: <1493302474-4701-3-git-send-email-rppt@linux.vnet.ibm.com>

On 04/27/2017 04:14 PM, Mike Rapoport wrote:
> Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>

Thanks, Mike. Applied, and lightly edited.

All changes now pushed to Git.

Cheers,

Michael

> ---
>  man2/ioctl_userfaultfd.2 | 53 ++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 51 insertions(+), 2 deletions(-)
> 
> diff --git a/man2/ioctl_userfaultfd.2 b/man2/ioctl_userfaultfd.2
> index 42bf7a7..cdc07e0 100644
> --- a/man2/ioctl_userfaultfd.2
> +++ b/man2/ioctl_userfaultfd.2
> @@ -121,22 +121,70 @@ and explicitly enable userfaultfd features that are disabled by default.
>  The kernel always reports all the available features in the
>  .I features
>  field.
> +
> +To enable userfaultfd features the application should set
> +a bit corresponding to each feature it wants to enable in the
> +.I features
> +field.
> +If the kernel supports all the requested features it will enable them.
> +Otherwise it will zero out the returned
> +.I uffdio_api
> +structure and return
> +.BR EINVAL .
>  .\" FIXME add more details about feature negotiation and enablement
>  
>  Since Linux 4.11, the following feature bits may be set:
>  .TP
>  .B UFFD_FEATURE_EVENT_FORK
> +When this feature is enabled,
> +the userfaultfd objects associated with a parent process are duplicated
> +into the child process during
> +.BR fork (2)
> +system call and the
> +.I UFFD_EVENT_FORK
> +is delivered to the userfaultfd monitor
>  .TP
>  .B UFFD_FEATURE_EVENT_REMAP
> +If this feature is enabled,
> +when the faulting process invokes
> +.BR mremap (2)
> +system call
> +the userfaultfd monitor will receive an event of type
> +.I UFFD_EVENT_REMAP.
>  .TP
>  .B UFFD_FEATURE_EVENT_REMOVE
> +If this feature is enabled,
> +when the faulting process calls
> +.BR madvise(2)
> +system call with
> +.I MADV_DONTNEED
> +or
> +.I MADV_REMOVE
> +advice to free a virtual memory area
> +the userfaultfd monitor will receive an event of type
> +.I UFFD_EVENT_REMOVE.
>  .TP
>  .B UFFD_FEATURE_EVENT_UNMAP
> +If this feature is enabled,
> +when the faulting process unmaps virtual memory either explicitly with
> +.BR munmap (2)
> +system call, or implicitly either during
> +.BR mmap (2)
> +or
> +.BR mremap (2)
> +system call,
> +the userfaultfd monitor will receive an event of type
> +.I UFFD_EVENT_UNMAP
>  .TP
>  .B UFFD_FEATURE_MISSING_HUGETLBFS
> +If this feature bit is set,
> +the kernel supports registering userfaultfd ranges on hugetlbfs
> +virtual memory areas
>  .TP
>  .B UFFD_FEATURE_MISSING_SHMEM
> -.\" FIXME add feature description
> +If this feature bit is set,
> +the kernel supports registering userfaultfd ranges on tmpfs
> +virtual memory areas
>  
>  The returned
>  .I ioctls
> @@ -182,7 +230,8 @@ The API version requested in the
>  .I api
>  field is not supported by this kernel, or the
>  .I features
> -field was not zero.
> +field passed to the kernel includes feature bits that are not supported
> +by the current kernel version.
>  .\" FIXME In the above error case, the returned 'uffdio_api' structure is
>  .\" zeroed out. Why is this done? This should be explained in the manual page.
>  .\"
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

--
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 0/3 v2] ARM/ARM64: silence large module first time allocation
From: Florian Fainelli @ 2017-04-27 17:38 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Florian Fainelli, Russell King, Catalin Marinas, Will Deacon,
	Ard Biesheuvel, Andrew Morton, Michal Hocko, zijun_hu,
	Kirill A. Shutemov, Andrey Ryabinin, Chris Wilson, open list,
	open list:MEMORY MANAGEMENT, angus

With kernels built with CONFIG_ARM{,64}_MODULES_PLTS=y, the first allocation
done from module space will fail, produce a general OOM allocation and also a
vmap warning. The second allocation from vmalloc space may or may not be
successful, but is actually the one we are interested about in these cases.

This patch series passed __GFP_NOWARN to silence such allocations from the
ARM/ARM64 module loader's first time allocation when the MODULES_PLT option is
enabled, and also makes alloc_vmap_area() react to the caller setting
__GFP_NOWARN to silence "vmap allocation for size..." messages.

Changes in v2:

- check __GFP_NOWARN out of the printk_ratelimited() check (Michal)

Here is an example of what we would get without these two patches, pretty
scary huh?

# insmod /mnt/nfs/huge.ko 
[   22.114143] random: nonblocking pool is initialized
[   22.183575] vmap allocation for size 15736832 failed: use vmalloc=<size> to increase size.
[   22.191873] vmalloc: allocation failure: 15729534 bytes
[   22.197112] insmod: page allocation failure: order:0, mode:0xd0
[   22.203048] CPU: 2 PID: 1506 Comm: insmod Tainted: G           O    4.1.20-1.9pre-01082-gbbbff07bc3ce #9
[   22.212536] Hardware name: Broadcom STB (Flattened Device Tree)
[   22.218480] [<c0017eec>] (unwind_backtrace) from [<c00135c8>] (show_stack+0x10/0x14)
[   22.226238] [<c00135c8>] (show_stack) from [<c0638684>] (dump_stack+0x90/0xa4)
[   22.233473] [<c0638684>] (dump_stack) from [<c00aae1c>] (warn_alloc_failed+0x104/0x144)
[   22.241490] [<c00aae1c>] (warn_alloc_failed) from [<c00d72e0>] (__vmalloc_node_range+0x170/0x218)
[   22.250375] [<c00d72e0>] (__vmalloc_node_range) from [<c00147d0>] (module_alloc+0x50/0xac)
[   22.258651] [<c00147d0>] (module_alloc) from [<c008ae2c>] (module_alloc_update_bounds+0xc/0x6c)
[   22.267360] [<c008ae2c>] (module_alloc_update_bounds) from [<c008b778>] (load_module+0x8ec/0x2058)
[   22.276329] [<c008b778>] (load_module) from [<c008cfd4>] (SyS_init_module+0xf0/0x174)
[   22.284170] [<c008cfd4>] (SyS_init_module) from [<c0010140>] (ret_fast_syscall+0x0/0x3c)
[   22.292277] Mem-Info:
[   22.294567] active_anon:5236 inactive_anon:1773 isolated_anon:0
[   22.294567]  active_file:1 inactive_file:3822 isolated_file:0
[   22.294567]  unevictable:0 dirty:0 writeback:0 unstable:0
[   22.294567]  slab_reclaimable:238 slab_unreclaimable:1594
[   22.294567]  mapped:855 shmem:2950 pagetables:36 bounce:0
[   22.294567]  free:39031 free_pcp:198 free_cma:3928
[   22.327196] DMA free:156124kB min:1880kB low:2348kB high:2820kB active_anon:20944kB inactive_anon:7092kB active_file:4kB inactive_file:15288kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:262144kB managed:227676kB mlocked:0kB dirty:0kB writeback:0kB mapped:3420kB shmem:11800kB slab_reclaimable:952kB slab_unreclaimable:6376kB kernel_stack:560kB pagetables:144kB unstable:0kB bounce:0kB free_pcp:792kB local_pcp:68kB free_cma:15712kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no
[   22.371631] lowmem_reserve[]: 0 0 0 0
[   22.375372] HighMem free:0kB min:128kB low:128kB high:128kB active_anon:0kB inactive_anon:0kB active_file:0kB inactive_file:0kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:2883584kB managed:0kB mlocked:0kB dirty:0kB writeback:0kB mapped:0kB shmem:0kB slab_reclaimable:0kB slab_unreclaimable:0kB kernel_stack:0kB pagetables:0kB unstable:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? yes
[   22.416249] lowmem_reserve[]: 0 0 0 0
[   22.419986] DMA: 3*4kB (UEM) 4*8kB (UE) 1*16kB (M) 4*32kB (UEMC) 3*64kB (EMC) 1*128kB (E) 4*256kB (UEMC) 2*512kB (UE) 2*1024kB (MC) 4*2048kB (UEMC) 35*4096kB (MRC) = 156156kB
[   22.435922] HighMem: 0*4kB 0*8kB 0*16kB 0*32kB 0*64kB 0*128kB 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 0kB
[   22.446130] 6789 total pagecache pages
[   22.449889] 0 pages in swap cache
[   22.453212] Swap cache stats: add 0, delete 0, find 0/0
[   22.458447] Free swap  = 0kB
[   22.461334] Total swap = 0kB
[   22.464222] 786432 pages RAM
[   22.467110] 720896 pages HighMem/MovableOnly
[   22.471388] 725417 pages reserved
[   22.474711] 4096 pages cma reserved
[   22.511310] big_init: I am a big module using 3932160 bytes of data!

Florian Fainelli (3):
  mm: Silence vmap() allocation failures based on caller gfp_flags
  ARM: Silence first allocation with CONFIG_ARM_MODULE_PLTS=y
  arm64: Silence first allocation with CONFIG_ARM64_MODULE_PLTS=y

 arch/arm/kernel/module.c   | 11 +++++++++--
 arch/arm64/kernel/module.c |  7 ++++++-
 mm/vmalloc.c               |  2 +-
 3 files changed, 16 insertions(+), 4 deletions(-)

-- 
2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* [PATCH v2 1/3] mm: Silence vmap() allocation failures based on caller gfp_flags
From: Florian Fainelli @ 2017-04-27 17:38 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Florian Fainelli, Russell King, Catalin Marinas, Will Deacon,
	Ard Biesheuvel, Andrew Morton, Michal Hocko, zijun_hu,
	Kirill A. Shutemov, Andrey Ryabinin, Chris Wilson, open list,
	open list:MEMORY MANAGEMENT, angus
In-Reply-To: <20170427173900.2538-1-f.fainelli@gmail.com>

If the caller has set __GFP_NOWARN don't print the following message:
vmap allocation for size 15736832 failed: use vmalloc=<size> to increase
size.

This can happen with the ARM/Linux or ARM64/Linux module loader built
with CONFIG_ARM{,64}_MODULE_PLTS=y which does a first attempt at loading
a large module from module space, then falls back to vmalloc space.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 mm/vmalloc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 0b057628a7ba..d8a851634674 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -521,9 +521,13 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
 		}
 	}
 
+	if (gfp_mask & __GFP_NOWARN)
+		goto out;
+
 	if (printk_ratelimit())
 		pr_warn("vmap allocation for size %lu failed: use vmalloc=<size> to increase size\n",
 			size);
+out:
 	kfree(va);
 	return ERR_PTR(-EBUSY);
 }
-- 
2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related

* [PATCH v2 2/3] ARM: Silence first allocation with CONFIG_ARM_MODULE_PLTS=y
From: Florian Fainelli @ 2017-04-27 17:38 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Florian Fainelli, Russell King, Catalin Marinas, Will Deacon,
	Ard Biesheuvel, Andrew Morton, Michal Hocko, zijun_hu,
	Kirill A. Shutemov, Andrey Ryabinin, Chris Wilson, open list,
	open list:MEMORY MANAGEMENT, angus
In-Reply-To: <20170427173900.2538-1-f.fainelli@gmail.com>

When CONFIG_ARM_MODULE_PLTS is enabled, the first allocation using the
module space fails, because the module is too big, and then the module
allocation is attempted from vmalloc space. Silence the first allocation
failure in that case by setting __GFP_NOWARN.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/arm/kernel/module.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index 80254b47dc34..503d1a39464a 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -40,8 +40,15 @@
 #ifdef CONFIG_MMU
 void *module_alloc(unsigned long size)
 {
-	void *p = __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END,
-				GFP_KERNEL, PAGE_KERNEL_EXEC, 0, NUMA_NO_NODE,
+	gfp_t gfp_mask = GFP_KERNEL;
+	void *p;
+
+#if IS_ENABLED(CONFIG_ARM_MODULE_PLTS)
+	/* Silence the initial allocation */
+	gfp_mask |= __GFP_NOWARN;
+#endif
+	p = __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END,
+				gfp_mask, PAGE_KERNEL_EXEC, 0, NUMA_NO_NODE,
 				__builtin_return_address(0));
 	if (!IS_ENABLED(CONFIG_ARM_MODULE_PLTS) || p)
 		return p;
-- 
2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related

* [PATCH v2 3/3] arm64: Silence first allocation with CONFIG_ARM64_MODULE_PLTS=y
From: Florian Fainelli @ 2017-04-27 17:39 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Florian Fainelli, Russell King, Catalin Marinas, Will Deacon,
	Ard Biesheuvel, Andrew Morton, Michal Hocko, zijun_hu,
	Kirill A. Shutemov, Andrey Ryabinin, Chris Wilson, open list,
	open list:MEMORY MANAGEMENT, angus
In-Reply-To: <20170427173900.2538-1-f.fainelli@gmail.com>

When CONFIG_ARM64_MODULE_PLTS is enabled, the first allocation using the
module space fails, because the module is too big, and then the module
allocation is attempted from vmalloc space. Silence the first allocation
failure in that case by setting __GFP_NOWARN.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/arm64/kernel/module.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
index 7f316982ce00..58bd5cfdd544 100644
--- a/arch/arm64/kernel/module.c
+++ b/arch/arm64/kernel/module.c
@@ -32,11 +32,16 @@
 
 void *module_alloc(unsigned long size)
 {
+	gfp_t gfp_mask = GFP_KERNEL;
 	void *p;
 
+#if IS_ENABLED(CONFIG_ARM64_MODULE_PLTS)
+	/* Silence the initial allocation */
+	gfp_mask |= __GFP_NOWARN;
+#endif
 	p = __vmalloc_node_range(size, MODULE_ALIGN, module_alloc_base,
 				module_alloc_base + MODULES_VSIZE,
-				GFP_KERNEL, PAGE_KERNEL_EXEC, 0,
+				gfp_mask, PAGE_KERNEL_EXEC, 0,
 				NUMA_NO_NODE, __builtin_return_address(0));
 
 	if (!p && IS_ENABLED(CONFIG_ARM64_MODULE_PLTS) &&
-- 
2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related

* Re: [PATCH v2 1/3] mm: Silence vmap() allocation failures based on caller gfp_flags
From: Michal Hocko @ 2017-04-27 17:56 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-arm-kernel, Russell King, Catalin Marinas, Will Deacon,
	Ard Biesheuvel, Andrew Morton, zijun_hu, Kirill A. Shutemov,
	Andrey Ryabinin, Chris Wilson, open list,
	open list:MEMORY MANAGEMENT, angus
In-Reply-To: <20170427173900.2538-2-f.fainelli@gmail.com>

On Thu 27-04-17 10:38:58, Florian Fainelli wrote:
> If the caller has set __GFP_NOWARN don't print the following message:
> vmap allocation for size 15736832 failed: use vmalloc=<size> to increase
> size.
> 
> This can happen with the ARM/Linux or ARM64/Linux module loader built
> with CONFIG_ARM{,64}_MODULE_PLTS=y which does a first attempt at loading
> a large module from module space, then falls back to vmalloc space.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Acked-by: Michal Hocko <mhocko@suse.com>

just a nit

> ---
>  mm/vmalloc.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 0b057628a7ba..d8a851634674 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -521,9 +521,13 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
>  		}
>  	}
>  
> +	if (gfp_mask & __GFP_NOWARN)
> +		goto out;
> +
>  	if (printk_ratelimit())

	if (!(gfp_mask & __GFP_NOWARN) && printk_ratelimit())
>  		pr_warn("vmap allocation for size %lu failed: use vmalloc=<size> to increase size\n",
>  			size);

would be shorter and you wouldn't need the goto and a label.

> +out:
>  	kfree(va);
>  	return ERR_PTR(-EBUSY);
>  }
> -- 
> 2.9.3
> 

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v2 1/3] mm: Silence vmap() allocation failures based on caller gfp_flags
From: Florian Fainelli @ 2017-04-27 18:03 UTC (permalink / raw)
  To: Michal Hocko
  Cc: linux-arm-kernel, Russell King, Catalin Marinas, Will Deacon,
	Ard Biesheuvel, Andrew Morton, zijun_hu, Kirill A. Shutemov,
	Andrey Ryabinin, Chris Wilson, open list,
	open list:MEMORY MANAGEMENT, angus
In-Reply-To: <20170427175653.GB30672@dhcp22.suse.cz>

On 04/27/2017 10:56 AM, Michal Hocko wrote:
> On Thu 27-04-17 10:38:58, Florian Fainelli wrote:
>> If the caller has set __GFP_NOWARN don't print the following message:
>> vmap allocation for size 15736832 failed: use vmalloc=<size> to increase
>> size.
>>
>> This can happen with the ARM/Linux or ARM64/Linux module loader built
>> with CONFIG_ARM{,64}_MODULE_PLTS=y which does a first attempt at loading
>> a large module from module space, then falls back to vmalloc space.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> Acked-by: Michal Hocko <mhocko@suse.com>
> 
> just a nit
> 
>> ---
>>  mm/vmalloc.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
>> index 0b057628a7ba..d8a851634674 100644
>> --- a/mm/vmalloc.c
>> +++ b/mm/vmalloc.c
>> @@ -521,9 +521,13 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
>>  		}
>>  	}
>>  
>> +	if (gfp_mask & __GFP_NOWARN)
>> +		goto out;
>> +
>>  	if (printk_ratelimit())
> 
> 	if (!(gfp_mask & __GFP_NOWARN) && printk_ratelimit())
>>  		pr_warn("vmap allocation for size %lu failed: use vmalloc=<size> to increase size\n",
>>  			size);
> 
> would be shorter and you wouldn't need the goto and a label.

Do you want me to resubmit with that change included?
-- 
Florian

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v2 3/3] arm64: Silence first allocation with CONFIG_ARM64_MODULE_PLTS=y
From: Ard Biesheuvel @ 2017-04-27 18:07 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-arm-kernel, Russell King, Catalin Marinas, Will Deacon,
	Andrew Morton, Michal Hocko, zijun_hu, Kirill A. Shutemov,
	Andrey Ryabinin, Chris Wilson, open list,
	open list:MEMORY MANAGEMENT, angus
In-Reply-To: <20170427173900.2538-4-f.fainelli@gmail.com>


> On 27 Apr 2017, at 18:39, Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
> When CONFIG_ARM64_MODULE_PLTS is enabled, the first allocation using the
> module space fails, because the module is too big, and then the module
> allocation is attempted from vmalloc space. Silence the first allocation
> failure in that case by setting __GFP_NOWARN.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> arch/arm64/kernel/module.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
> index 7f316982ce00..58bd5cfdd544 100644
> --- a/arch/arm64/kernel/module.c
> +++ b/arch/arm64/kernel/module.c
> @@ -32,11 +32,16 @@
> 
> void *module_alloc(unsigned long size)
> {
> +    gfp_t gfp_mask = GFP_KERNEL;
>    void *p;
> 
> +#if IS_ENABLED(CONFIG_ARM64_MODULE_PLTS)
> +    /* Silence the initial allocation */
> +    gfp_mask |= __GFP_NOWARN;
> +#endif

Please use IS_ENABLED() instead here

>    p = __vmalloc_node_range(size, MODULE_ALIGN, module_alloc_base,
>                module_alloc_base + MODULES_VSIZE,
> -                GFP_KERNEL, PAGE_KERNEL_EXEC, 0,
> +                gfp_mask, PAGE_KERNEL_EXEC, 0,
>                NUMA_NO_NODE, __builtin_return_address(0));
> 
>    if (!p && IS_ENABLED(CONFIG_ARM64_MODULE_PLTS) &&
> -- 
> 2.9.3
> 

Other than that, and with Michal's nit addressed:

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v2 3/3] arm64: Silence first allocation with CONFIG_ARM64_MODULE_PLTS=y
From: Florian Fainelli @ 2017-04-27 18:09 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel, Russell King, Catalin Marinas, Will Deacon,
	Andrew Morton, Michal Hocko, zijun_hu, Kirill A. Shutemov,
	Andrey Ryabinin, Chris Wilson, open list,
	open list:MEMORY MANAGEMENT, angus
In-Reply-To: <C103C078-3462-43D9-AEF5-5DEC3A74CA7E@linaro.org>

On 04/27/2017 11:07 AM, Ard Biesheuvel wrote:
> 
>> On 27 Apr 2017, at 18:39, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>> When CONFIG_ARM64_MODULE_PLTS is enabled, the first allocation using the
>> module space fails, because the module is too big, and then the module
>> allocation is attempted from vmalloc space. Silence the first allocation
>> failure in that case by setting __GFP_NOWARN.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> arch/arm64/kernel/module.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
>> index 7f316982ce00..58bd5cfdd544 100644
>> --- a/arch/arm64/kernel/module.c
>> +++ b/arch/arm64/kernel/module.c
>> @@ -32,11 +32,16 @@
>>
>> void *module_alloc(unsigned long size)
>> {
>> +    gfp_t gfp_mask = GFP_KERNEL;
>>    void *p;
>>
>> +#if IS_ENABLED(CONFIG_ARM64_MODULE_PLTS)
>> +    /* Silence the initial allocation */
>> +    gfp_mask |= __GFP_NOWARN;
>> +#endif
> 
> Please use IS_ENABLED() instead here

How do you mean?

if (IS_ENABLED()) vs. #if IS_ENABLED()?

> 
>>    p = __vmalloc_node_range(size, MODULE_ALIGN, module_alloc_base,
>>                module_alloc_base + MODULES_VSIZE,
>> -                GFP_KERNEL, PAGE_KERNEL_EXEC, 0,
>> +                gfp_mask, PAGE_KERNEL_EXEC, 0,
>>                NUMA_NO_NODE, __builtin_return_address(0));
>>
>>    if (!p && IS_ENABLED(CONFIG_ARM64_MODULE_PLTS) &&
>> -- 
>> 2.9.3
>>
> 
> Other than that, and with Michal's nit addressed:
> 
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 


-- 
Florian

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v2 3/3] arm64: Silence first allocation with CONFIG_ARM64_MODULE_PLTS=y
From: Ard Biesheuvel @ 2017-04-27 18:12 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-arm-kernel, Russell King, Catalin Marinas, Will Deacon,
	Andrew Morton, Michal Hocko, zijun_hu, Kirill A. Shutemov,
	Andrey Ryabinin, Chris Wilson, open list,
	open list:MEMORY MANAGEMENT, angus
In-Reply-To: <53d960d0-e44c-3a8d-17fd-a3895ecee858@gmail.com>



> On 27 Apr 2017, at 19:09, Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
>> On 04/27/2017 11:07 AM, Ard Biesheuvel wrote:
>> 
>>> On 27 Apr 2017, at 18:39, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>> 
>>> When CONFIG_ARM64_MODULE_PLTS is enabled, the first allocation using the
>>> module space fails, because the module is too big, and then the module
>>> allocation is attempted from vmalloc space. Silence the first allocation
>>> failure in that case by setting __GFP_NOWARN.
>>> 
>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>> ---
>>> arch/arm64/kernel/module.c | 7 ++++++-
>>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
>>> index 7f316982ce00..58bd5cfdd544 100644
>>> --- a/arch/arm64/kernel/module.c
>>> +++ b/arch/arm64/kernel/module.c
>>> @@ -32,11 +32,16 @@
>>> 
>>> void *module_alloc(unsigned long size)
>>> {
>>> +    gfp_t gfp_mask = GFP_KERNEL;
>>>   void *p;
>>> 
>>> +#if IS_ENABLED(CONFIG_ARM64_MODULE_PLTS)
>>> +    /* Silence the initial allocation */
>>> +    gfp_mask |= __GFP_NOWARN;
>>> +#endif
>> 
>> Please use IS_ENABLED() instead here
> 
> How do you mean?
> 
> if (IS_ENABLED()) vs. #if IS_ENABLED()?
> 

Apologies, I didn't read carefully.

Use the C if not the preprocessor if

>> 
>>>   p = __vmalloc_node_range(size, MODULE_ALIGN, module_alloc_base,
>>>               module_alloc_base + MODULES_VSIZE,
>>> -                GFP_KERNEL, PAGE_KERNEL_EXEC, 0,
>>> +                gfp_mask, PAGE_KERNEL_EXEC, 0,
>>>               NUMA_NO_NODE, __builtin_return_address(0));
>>> 
>>>   if (!p && IS_ENABLED(CONFIG_ARM64_MODULE_PLTS) &&
>>> -- 
>>> 2.9.3
>>> 
>> 
>> Other than that, and with Michal's nit addressed:
>> 
>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> 
> 
> 
> -- 
> Florian

--
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 0/3 v3] ARM/ARM64: silence large module first time allocation
From: Florian Fainelli @ 2017-04-27 18:18 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Florian Fainelli, Russell King, Catalin Marinas, Will Deacon,
	Ard Biesheuvel, Andrew Morton, Michal Hocko, zijun_hu,
	Kirill A. Shutemov, Andrey Ryabinin, Chris Wilson, open list,
	open list:MEMORY MANAGEMENT, angus

With kernels built with CONFIG_ARM{,64}_MODULES_PLTS=y, the first allocation
done from module space will fail, produce a general OOM allocation and also a
vmap warning. The second allocation from vmalloc space may or may not be
successful, but is actually the one we are interested about in these cases.

This patch series passed __GFP_NOWARN to silence such allocations from the
ARM/ARM64 module loader's first time allocation when the MODULES_PLT option is
enabled, and also makes alloc_vmap_area() react to the caller setting
__GFP_NOWARN to silence "vmap allocation for size..." messages.


Changes in v3:
- check for __GFP_NOWARN not set where the check for printk_ratelimited()
  is done, add Michal's Acked-by

- use C conditionals and not CPP conditionals for IS_ENABLED(), add Ard's
  Reviewed-by tag

Changes in v2:

- check __GFP_NOWARN out of the printk_ratelimited() check (Michal)

Here is an example of what we would get without these two patches, pretty
scary huh?

# insmod /mnt/nfs/huge.ko 
[   22.114143] random: nonblocking pool is initialized
[   22.183575] vmap allocation for size 15736832 failed: use vmalloc=<size> to increase size.
[   22.191873] vmalloc: allocation failure: 15729534 bytes
[   22.197112] insmod: page allocation failure: order:0, mode:0xd0
[   22.203048] CPU: 2 PID: 1506 Comm: insmod Tainted: G           O    4.1.20-1.9pre-01082-gbbbff07bc3ce #9
[   22.212536] Hardware name: Broadcom STB (Flattened Device Tree)
[   22.218480] [<c0017eec>] (unwind_backtrace) from [<c00135c8>] (show_stack+0x10/0x14)
[   22.226238] [<c00135c8>] (show_stack) from [<c0638684>] (dump_stack+0x90/0xa4)
[   22.233473] [<c0638684>] (dump_stack) from [<c00aae1c>] (warn_alloc_failed+0x104/0x144)
[   22.241490] [<c00aae1c>] (warn_alloc_failed) from [<c00d72e0>] (__vmalloc_node_range+0x170/0x218)
[   22.250375] [<c00d72e0>] (__vmalloc_node_range) from [<c00147d0>] (module_alloc+0x50/0xac)
[   22.258651] [<c00147d0>] (module_alloc) from [<c008ae2c>] (module_alloc_update_bounds+0xc/0x6c)
[   22.267360] [<c008ae2c>] (module_alloc_update_bounds) from [<c008b778>] (load_module+0x8ec/0x2058)
[   22.276329] [<c008b778>] (load_module) from [<c008cfd4>] (SyS_init_module+0xf0/0x174)
[   22.284170] [<c008cfd4>] (SyS_init_module) from [<c0010140>] (ret_fast_syscall+0x0/0x3c)
[   22.292277] Mem-Info:
[   22.294567] active_anon:5236 inactive_anon:1773 isolated_anon:0
[   22.294567]  active_file:1 inactive_file:3822 isolated_file:0
[   22.294567]  unevictable:0 dirty:0 writeback:0 unstable:0
[   22.294567]  slab_reclaimable:238 slab_unreclaimable:1594
[   22.294567]  mapped:855 shmem:2950 pagetables:36 bounce:0
[   22.294567]  free:39031 free_pcp:198 free_cma:3928
[   22.327196] DMA free:156124kB min:1880kB low:2348kB high:2820kB active_anon:20944kB inactive_anon:7092kB active_file:4kB inactive_file:15288kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:262144kB managed:227676kB mlocked:0kB dirty:0kB writeback:0kB mapped:3420kB shmem:11800kB slab_reclaimable:952kB slab_unreclaimable:6376kB kernel_stack:560kB pagetables:144kB unstable:0kB bounce:0kB free_pcp:792kB local_pcp:68kB free_cma:15712kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no
[   22.371631] lowmem_reserve[]: 0 0 0 0
[   22.375372] HighMem free:0kB min:128kB low:128kB high:128kB active_anon:0kB inactive_anon:0kB active_file:0kB inactive_file:0kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:2883584kB managed:0kB mlocked:0kB dirty:0kB writeback:0kB mapped:0kB shmem:0kB slab_reclaimable:0kB slab_unreclaimable:0kB kernel_stack:0kB pagetables:0kB unstable:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? yes
[   22.416249] lowmem_reserve[]: 0 0 0 0
[   22.419986] DMA: 3*4kB (UEM) 4*8kB (UE) 1*16kB (M) 4*32kB (UEMC) 3*64kB (EMC) 1*128kB (E) 4*256kB (UEMC) 2*512kB (UE) 2*1024kB (MC) 4*2048kB (UEMC) 35*4096kB (MRC) = 156156kB
[   22.435922] HighMem: 0*4kB 0*8kB 0*16kB 0*32kB 0*64kB 0*128kB 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 0kB
[   22.446130] 6789 total pagecache pages
[   22.449889] 0 pages in swap cache
[   22.453212] Swap cache stats: add 0, delete 0, find 0/0
[   22.458447] Free swap  = 0kB
[   22.461334] Total swap = 0kB
[   22.464222] 786432 pages RAM
[   22.467110] 720896 pages HighMem/MovableOnly
[   22.471388] 725417 pages reserved
[   22.474711] 4096 pages cma reserved
[   22.511310] big_init: I am a big module using 3932160 bytes of data!

Florian Fainelli (3):
  mm: Silence vmap() allocation failures based on caller gfp_flags
  ARM: Silence first allocation with CONFIG_ARM_MODULE_PLTS=y
  arm64: Silence first allocation with CONFIG_ARM64_MODULE_PLTS=y

 arch/arm/kernel/module.c   | 11 +++++++++--
 arch/arm64/kernel/module.c |  7 ++++++-
 mm/vmalloc.c               |  2 +-
 3 files changed, 16 insertions(+), 4 deletions(-)

-- 
2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply


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