linux-nvdimm.lists.01.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone
       [not found] <20190830091428.18399-1-david@redhat.com>
@ 2019-09-26 12:25 ` Aneesh Kumar K.V
  2019-09-26 12:25   ` [PATCH 2/2] mm/memmap_init: Update variable name in memmap_init_zone Aneesh Kumar K.V
                     ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Aneesh Kumar K.V @ 2019-09-26 12:25 UTC (permalink / raw)
  To: dan.j.williams, akpm, David Hildenbrand
  Cc: linux-mm, Aneesh Kumar K.V, linux-nvdimm

With altmap, all the resource pfns are not initialized. While initializing
pfn, altmap reserve space is skipped. Hence when removing pfn from zone skip
pfns that were never initialized.

Update memunmap_pages to calculate start and end pfn based on altmap
values. This fixes a kernel crash that is observed when destroying namespace.

[   74.745056] BUG: Unable to handle kernel data access at 0xc00c000001400000
[   74.745256] Faulting instruction address: 0xc0000000000b58b0
cpu 0x2: Vector: 300 (Data Access) at [c00000026ea93580]
    pc: c0000000000b58b0: memset+0x68/0x104
    lr: c0000000003eb008: page_init_poison+0x38/0x50
    ...
  current = 0xc000000271c67d80
  paca    = 0xc00000003fffd680   irqmask: 0x03   irq_happened: 0x01
    pid   = 3665, comm = ndctl
[link register   ] c0000000003eb008 page_init_poison+0x38/0x50
[c00000026ea93830] c0000000004754d4 remove_pfn_range_from_zone+0x64/0x3e0
[c00000026ea938a0] c0000000004b8a60 memunmap_pages+0x300/0x400
[c00000026ea93930] c0000000009e32a0 devm_action_release+0x30/0x50
...

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 mm/memremap.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/mm/memremap.c b/mm/memremap.c
index 390bb3544589..76b98110031e 100644
--- a/mm/memremap.c
+++ b/mm/memremap.c
@@ -113,7 +113,8 @@ static void dev_pagemap_cleanup(struct dev_pagemap *pgmap)
 void memunmap_pages(struct dev_pagemap *pgmap)
 {
 	struct resource *res = &pgmap->res;
-	unsigned long pfn = PHYS_PFN(res->start);
+	unsigned long start_pfn, end_pfn;
+	unsigned long pfn, nr_pages;
 	int nid;
 
 	dev_pagemap_kill(pgmap);
@@ -121,14 +122,18 @@ void memunmap_pages(struct dev_pagemap *pgmap)
 		put_page(pfn_to_page(pfn));
 	dev_pagemap_cleanup(pgmap);
 
+	start_pfn = pfn_first(pgmap);
+	end_pfn = pfn_end(pgmap);
+	nr_pages = end_pfn - start_pfn;
+
 	/* pages are dead and unused, undo the arch mapping */
-	nid = page_to_nid(pfn_to_page(pfn));
+	nid = page_to_nid(pfn_to_page(start_pfn));
 
 	mem_hotplug_begin();
-	remove_pfn_range_from_zone(page_zone(pfn_to_page(pfn)), pfn,
-				   PHYS_PFN(resource_size(res)));
+	remove_pfn_range_from_zone(page_zone(pfn_to_page(start_pfn)),
+				   start_pfn, nr_pages);
 	if (pgmap->type == MEMORY_DEVICE_PRIVATE) {
-		__remove_pages(pfn, PHYS_PFN(resource_size(res)), NULL);
+		__remove_pages(start_pfn, nr_pages, NULL);
 	} else {
 		arch_remove_memory(nid, res->start, resource_size(res),
 				pgmap_altmap(pgmap));
-- 
2.21.0

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/2] mm/memmap_init: Update variable name in memmap_init_zone
  2019-09-26 12:25 ` [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone Aneesh Kumar K.V
@ 2019-09-26 12:25   ` Aneesh Kumar K.V
  2019-09-26 12:56     ` David Hildenbrand
  2019-09-26 13:38     ` Pankaj Gupta
  2019-09-26 12:43   ` [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone David Hildenbrand
                     ` (2 subsequent siblings)
  3 siblings, 2 replies; 16+ messages in thread
From: Aneesh Kumar K.V @ 2019-09-26 12:25 UTC (permalink / raw)
  To: dan.j.williams, akpm, David Hildenbrand
  Cc: linux-mm, Aneesh Kumar K.V, linux-nvdimm

The third argument is actually number of pages. Changes the variable name
from size to nr_pages to indicate this better.

No functional change in this patch.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 mm/page_alloc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 3334a769eb91..df9e09a5359f 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5914,10 +5914,10 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
 #ifdef CONFIG_ZONE_DEVICE
 void __ref memmap_init_zone_device(struct zone *zone,
 				   unsigned long start_pfn,
-				   unsigned long size,
+				   unsigned long nr_pages,
 				   struct dev_pagemap *pgmap)
 {
-	unsigned long pfn, end_pfn = start_pfn + size;
+	unsigned long pfn, end_pfn = start_pfn + nr_pages;
 	struct pglist_data *pgdat = zone->zone_pgdat;
 	struct vmem_altmap *altmap = pgmap_altmap(pgmap);
 	unsigned long zone_idx = zone_idx(zone);
@@ -5934,7 +5934,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
 	 */
 	if (altmap) {
 		start_pfn = altmap->base_pfn + vmem_altmap_offset(altmap);
-		size = end_pfn - start_pfn;
+		nr_pages = end_pfn - start_pfn;
 	}
 
 	for (pfn = start_pfn; pfn < end_pfn; pfn++) {
@@ -5981,7 +5981,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
 	}
 
 	pr_info("%s initialised %lu pages in %ums\n", __func__,
-		size, jiffies_to_msecs(jiffies - start));
+		nr_pages, jiffies_to_msecs(jiffies - start));
 }
 
 #endif
-- 
2.21.0

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone
  2019-09-26 12:25 ` [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone Aneesh Kumar K.V
  2019-09-26 12:25   ` [PATCH 2/2] mm/memmap_init: Update variable name in memmap_init_zone Aneesh Kumar K.V
@ 2019-09-26 12:43   ` David Hildenbrand
  2019-09-26 13:15     ` Aneesh Kumar K.V
  2019-09-26 13:34   ` Pankaj Gupta
  2019-09-26 22:45   ` Andrew Morton
  3 siblings, 1 reply; 16+ messages in thread
From: David Hildenbrand @ 2019-09-26 12:43 UTC (permalink / raw)
  To: Aneesh Kumar K.V, dan.j.williams, akpm; +Cc: linux-mm, linux-nvdimm

On 26.09.19 14:25, Aneesh Kumar K.V wrote:
> With altmap, all the resource pfns are not initialized. While initializing
> pfn, altmap reserve space is skipped. Hence when removing pfn from zone skip
> pfns that were never initialized.
> 
> Update memunmap_pages to calculate start and end pfn based on altmap
> values. This fixes a kernel crash that is observed when destroying namespace.
> 
> [   74.745056] BUG: Unable to handle kernel data access at 0xc00c000001400000
> [   74.745256] Faulting instruction address: 0xc0000000000b58b0
> cpu 0x2: Vector: 300 (Data Access) at [c00000026ea93580]
>     pc: c0000000000b58b0: memset+0x68/0x104
>     lr: c0000000003eb008: page_init_poison+0x38/0x50
>     ...
>   current = 0xc000000271c67d80
>   paca    = 0xc00000003fffd680   irqmask: 0x03   irq_happened: 0x01
>     pid   = 3665, comm = ndctl
> [link register   ] c0000000003eb008 page_init_poison+0x38/0x50
> [c00000026ea93830] c0000000004754d4 remove_pfn_range_from_zone+0x64/0x3e0
> [c00000026ea938a0] c0000000004b8a60 memunmap_pages+0x300/0x400
> [c00000026ea93930] c0000000009e32a0 devm_action_release+0x30/0x50
> ...
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
>  mm/memremap.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/memremap.c b/mm/memremap.c
> index 390bb3544589..76b98110031e 100644
> --- a/mm/memremap.c
> +++ b/mm/memremap.c
> @@ -113,7 +113,8 @@ static void dev_pagemap_cleanup(struct dev_pagemap *pgmap)
>  void memunmap_pages(struct dev_pagemap *pgmap)
>  {
>  	struct resource *res = &pgmap->res;
> -	unsigned long pfn = PHYS_PFN(res->start);
> +	unsigned long start_pfn, end_pfn;
> +	unsigned long pfn, nr_pages;
>  	int nid;
>  
>  	dev_pagemap_kill(pgmap);
> @@ -121,14 +122,18 @@ void memunmap_pages(struct dev_pagemap *pgmap)
>  		put_page(pfn_to_page(pfn));
>  	dev_pagemap_cleanup(pgmap);
>  
> +	start_pfn = pfn_first(pgmap);
> +	end_pfn = pfn_end(pgmap);
> +	nr_pages = end_pfn - start_pfn;
> +
>  	/* pages are dead and unused, undo the arch mapping */
> -	nid = page_to_nid(pfn_to_page(pfn));
> +	nid = page_to_nid(pfn_to_page(start_pfn));
>  
>  	mem_hotplug_begin();
> -	remove_pfn_range_from_zone(page_zone(pfn_to_page(pfn)), pfn,
> -				   PHYS_PFN(resource_size(res)));
> +	remove_pfn_range_from_zone(page_zone(pfn_to_page(start_pfn)),
> +				   start_pfn, nr_pages);
>  	if (pgmap->type == MEMORY_DEVICE_PRIVATE) {
> -		__remove_pages(pfn, PHYS_PFN(resource_size(res)), NULL);
> +		__remove_pages(start_pfn, nr_pages, NULL);
>  	} else {
>  		arch_remove_memory(nid, res->start, resource_size(res),
>  				pgmap_altmap(pgmap));
> 

Just to make sure, my patches did not break that, right (IOW, broken
upstream)?

-- 

Thanks,

David / dhildenb
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] mm/memmap_init: Update variable name in memmap_init_zone
  2019-09-26 12:25   ` [PATCH 2/2] mm/memmap_init: Update variable name in memmap_init_zone Aneesh Kumar K.V
@ 2019-09-26 12:56     ` David Hildenbrand
  2019-09-26 13:38     ` Pankaj Gupta
  1 sibling, 0 replies; 16+ messages in thread
From: David Hildenbrand @ 2019-09-26 12:56 UTC (permalink / raw)
  To: Aneesh Kumar K.V, dan.j.williams, akpm; +Cc: linux-mm, linux-nvdimm

On 26.09.19 14:25, Aneesh Kumar K.V wrote:
> The third argument is actually number of pages. Changes the variable name
> from size to nr_pages to indicate this better.
> 
> No functional change in this patch.
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
>  mm/page_alloc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 3334a769eb91..df9e09a5359f 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5914,10 +5914,10 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
>  #ifdef CONFIG_ZONE_DEVICE
>  void __ref memmap_init_zone_device(struct zone *zone,
>  				   unsigned long start_pfn,
> -				   unsigned long size,
> +				   unsigned long nr_pages,
>  				   struct dev_pagemap *pgmap)
>  {
> -	unsigned long pfn, end_pfn = start_pfn + size;
> +	unsigned long pfn, end_pfn = start_pfn + nr_pages;
>  	struct pglist_data *pgdat = zone->zone_pgdat;
>  	struct vmem_altmap *altmap = pgmap_altmap(pgmap);
>  	unsigned long zone_idx = zone_idx(zone);
> @@ -5934,7 +5934,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
>  	 */
>  	if (altmap) {
>  		start_pfn = altmap->base_pfn + vmem_altmap_offset(altmap);
> -		size = end_pfn - start_pfn;
> +		nr_pages = end_pfn - start_pfn;
>  	}
>  
>  	for (pfn = start_pfn; pfn < end_pfn; pfn++) {
> @@ -5981,7 +5981,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
>  	}
>  
>  	pr_info("%s initialised %lu pages in %ums\n", __func__,
> -		size, jiffies_to_msecs(jiffies - start));
> +		nr_pages, jiffies_to_msecs(jiffies - start));
>  }
>  
>  #endif
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone
  2019-09-26 12:43   ` [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone David Hildenbrand
@ 2019-09-26 13:15     ` Aneesh Kumar K.V
  0 siblings, 0 replies; 16+ messages in thread
From: Aneesh Kumar K.V @ 2019-09-26 13:15 UTC (permalink / raw)
  To: David Hildenbrand, dan.j.williams, akpm; +Cc: linux-mm, linux-nvdimm

On 9/26/19 6:13 PM, David Hildenbrand wrote:
> On 26.09.19 14:25, Aneesh Kumar K.V wrote:
>> With altmap, all the resource pfns are not initialized. While initializing
>> pfn, altmap reserve space is skipped. Hence when removing pfn from zone skip
>> pfns that were never initialized.
>>
>> Update memunmap_pages to calculate start and end pfn based on altmap
>> values. This fixes a kernel crash that is observed when destroying namespace.
>>
>> [   74.745056] BUG: Unable to handle kernel data access at 0xc00c000001400000
>> [   74.745256] Faulting instruction address: 0xc0000000000b58b0
>> cpu 0x2: Vector: 300 (Data Access) at [c00000026ea93580]
>>      pc: c0000000000b58b0: memset+0x68/0x104
>>      lr: c0000000003eb008: page_init_poison+0x38/0x50
>>      ...
>>    current = 0xc000000271c67d80
>>    paca    = 0xc00000003fffd680   irqmask: 0x03   irq_happened: 0x01
>>      pid   = 3665, comm = ndctl
>> [link register   ] c0000000003eb008 page_init_poison+0x38/0x50
>> [c00000026ea93830] c0000000004754d4 remove_pfn_range_from_zone+0x64/0x3e0
>> [c00000026ea938a0] c0000000004b8a60 memunmap_pages+0x300/0x400
>> [c00000026ea93930] c0000000009e32a0 devm_action_release+0x30/0x50
>> ...
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>>   mm/memremap.c | 15 ++++++++++-----
>>   1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/memremap.c b/mm/memremap.c
>> index 390bb3544589..76b98110031e 100644
>> --- a/mm/memremap.c
>> +++ b/mm/memremap.c
>> @@ -113,7 +113,8 @@ static void dev_pagemap_cleanup(struct dev_pagemap *pgmap)
>>   void memunmap_pages(struct dev_pagemap *pgmap)
>>   {
>>   	struct resource *res = &pgmap->res;
>> -	unsigned long pfn = PHYS_PFN(res->start);
>> +	unsigned long start_pfn, end_pfn;
>> +	unsigned long pfn, nr_pages;
>>   	int nid;
>>   
>>   	dev_pagemap_kill(pgmap);
>> @@ -121,14 +122,18 @@ void memunmap_pages(struct dev_pagemap *pgmap)
>>   		put_page(pfn_to_page(pfn));
>>   	dev_pagemap_cleanup(pgmap);
>>   
>> +	start_pfn = pfn_first(pgmap);
>> +	end_pfn = pfn_end(pgmap);
>> +	nr_pages = end_pfn - start_pfn;
>> +
>>   	/* pages are dead and unused, undo the arch mapping */
>> -	nid = page_to_nid(pfn_to_page(pfn));
>> +	nid = page_to_nid(pfn_to_page(start_pfn));
>>   
>>   	mem_hotplug_begin();
>> -	remove_pfn_range_from_zone(page_zone(pfn_to_page(pfn)), pfn,
>> -				   PHYS_PFN(resource_size(res)));
>> +	remove_pfn_range_from_zone(page_zone(pfn_to_page(start_pfn)),
>> +				   start_pfn, nr_pages);
>>   	if (pgmap->type == MEMORY_DEVICE_PRIVATE) {
>> -		__remove_pages(pfn, PHYS_PFN(resource_size(res)), NULL);
>> +		__remove_pages(start_pfn, nr_pages, NULL);
>>   	} else {
>>   		arch_remove_memory(nid, res->start, resource_size(res),
>>   				pgmap_altmap(pgmap));
>>
> 
> Just to make sure, my patches did not break that, right (IOW, broken
> upstream)?
> 

That is correct. Your patches helped to remove other usages of wrong 
pfns. The last few left got fixed in this patch.

-aneesh
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone
  2019-09-26 12:25 ` [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone Aneesh Kumar K.V
  2019-09-26 12:25   ` [PATCH 2/2] mm/memmap_init: Update variable name in memmap_init_zone Aneesh Kumar K.V
  2019-09-26 12:43   ` [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone David Hildenbrand
@ 2019-09-26 13:34   ` Pankaj Gupta
  2019-09-26 22:45   ` Andrew Morton
  3 siblings, 0 replies; 16+ messages in thread
From: Pankaj Gupta @ 2019-09-26 13:34 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: linux-mm, linux-nvdimm, akpm, David Hildenbrand


> 
> With altmap, all the resource pfns are not initialized. While initializing
> pfn, altmap reserve space is skipped. Hence when removing pfn from zone skip
> pfns that were never initialized.
> 
> Update memunmap_pages to calculate start and end pfn based on altmap
> values. This fixes a kernel crash that is observed when destroying namespace.
> 
> [   74.745056] BUG: Unable to handle kernel data access at 0xc00c000001400000
> [   74.745256] Faulting instruction address: 0xc0000000000b58b0
> cpu 0x2: Vector: 300 (Data Access) at [c00000026ea93580]
>     pc: c0000000000b58b0: memset+0x68/0x104
>     lr: c0000000003eb008: page_init_poison+0x38/0x50
>     ...
>   current = 0xc000000271c67d80
>   paca    = 0xc00000003fffd680   irqmask: 0x03   irq_happened: 0x01
>     pid   = 3665, comm = ndctl
> [link register   ] c0000000003eb008 page_init_poison+0x38/0x50
> [c00000026ea93830] c0000000004754d4 remove_pfn_range_from_zone+0x64/0x3e0
> [c00000026ea938a0] c0000000004b8a60 memunmap_pages+0x300/0x400
> [c00000026ea93930] c0000000009e32a0 devm_action_release+0x30/0x50
> ...
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
>  mm/memremap.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/memremap.c b/mm/memremap.c
> index 390bb3544589..76b98110031e 100644
> --- a/mm/memremap.c
> +++ b/mm/memremap.c
> @@ -113,7 +113,8 @@ static void dev_pagemap_cleanup(struct dev_pagemap
> *pgmap)
>  void memunmap_pages(struct dev_pagemap *pgmap)
>  {
>  	struct resource *res = &pgmap->res;
> -	unsigned long pfn = PHYS_PFN(res->start);
> +	unsigned long start_pfn, end_pfn;
> +	unsigned long pfn, nr_pages;
>  	int nid;
>  
>  	dev_pagemap_kill(pgmap);
> @@ -121,14 +122,18 @@ void memunmap_pages(struct dev_pagemap *pgmap)
>  		put_page(pfn_to_page(pfn));
>  	dev_pagemap_cleanup(pgmap);
>  
> +	start_pfn = pfn_first(pgmap);
> +	end_pfn = pfn_end(pgmap);
> +	nr_pages = end_pfn - start_pfn;
> +
>  	/* pages are dead and unused, undo the arch mapping */
> -	nid = page_to_nid(pfn_to_page(pfn));
> +	nid = page_to_nid(pfn_to_page(start_pfn));
>  
>  	mem_hotplug_begin();
> -	remove_pfn_range_from_zone(page_zone(pfn_to_page(pfn)), pfn,
> -				   PHYS_PFN(resource_size(res)));
> +	remove_pfn_range_from_zone(page_zone(pfn_to_page(start_pfn)),
> +				   start_pfn, nr_pages);
>  	if (pgmap->type == MEMORY_DEVICE_PRIVATE) {
> -		__remove_pages(pfn, PHYS_PFN(resource_size(res)), NULL);
> +		__remove_pages(start_pfn, nr_pages, NULL);
>  	} else {
>  		arch_remove_memory(nid, res->start, resource_size(res),
>  				pgmap_altmap(pgmap));
> --
> 2.21.0

Reviewed-by: Pankaj Gupta <pagupta@redhat.com>

> 
> 
> 
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] mm/memmap_init: Update variable name in memmap_init_zone
  2019-09-26 12:25   ` [PATCH 2/2] mm/memmap_init: Update variable name in memmap_init_zone Aneesh Kumar K.V
  2019-09-26 12:56     ` David Hildenbrand
@ 2019-09-26 13:38     ` Pankaj Gupta
  1 sibling, 0 replies; 16+ messages in thread
From: Pankaj Gupta @ 2019-09-26 13:38 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: linux-mm, linux-nvdimm, akpm, David Hildenbrand



> 
> The third argument is actually number of pages. Changes the variable name
> from size to nr_pages to indicate this better.
> 
> No functional change in this patch.
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
>  mm/page_alloc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 3334a769eb91..df9e09a5359f 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5914,10 +5914,10 @@ void __meminit memmap_init_zone(unsigned long size,
> int nid, unsigned long zone,
>  #ifdef CONFIG_ZONE_DEVICE
>  void __ref memmap_init_zone_device(struct zone *zone,
>  				   unsigned long start_pfn,
> -				   unsigned long size,
> +				   unsigned long nr_pages,
>  				   struct dev_pagemap *pgmap)
>  {
> -	unsigned long pfn, end_pfn = start_pfn + size;
> +	unsigned long pfn, end_pfn = start_pfn + nr_pages;
>  	struct pglist_data *pgdat = zone->zone_pgdat;
>  	struct vmem_altmap *altmap = pgmap_altmap(pgmap);
>  	unsigned long zone_idx = zone_idx(zone);
> @@ -5934,7 +5934,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
>  	 */
>  	if (altmap) {
>  		start_pfn = altmap->base_pfn + vmem_altmap_offset(altmap);
> -		size = end_pfn - start_pfn;
> +		nr_pages = end_pfn - start_pfn;
>  	}
>  
>  	for (pfn = start_pfn; pfn < end_pfn; pfn++) {
> @@ -5981,7 +5981,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
>  	}
>  
>  	pr_info("%s initialised %lu pages in %ums\n", __func__,
> -		size, jiffies_to_msecs(jiffies - start));
> +		nr_pages, jiffies_to_msecs(jiffies - start));
>  }
>  
>  #endif
> --
> 2.21.0

Reviewed-by: Pankaj Gupta <pagupta@redhat.com>

> 
> 
> 
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone
  2019-09-26 12:25 ` [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone Aneesh Kumar K.V
                     ` (2 preceding siblings ...)
  2019-09-26 13:34   ` Pankaj Gupta
@ 2019-09-26 22:45   ` Andrew Morton
  2019-09-27  1:51     ` Aneesh Kumar K.V
  3 siblings, 1 reply; 16+ messages in thread
From: Andrew Morton @ 2019-09-26 22:45 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: linux-mm, linux-nvdimm, David Hildenbrand

On Thu, 26 Sep 2019 17:55:51 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> wrote:

> With altmap, all the resource pfns are not initialized. While initializing
> pfn, altmap reserve space is skipped. Hence when removing pfn from zone skip
> pfns that were never initialized.
> 
> Update memunmap_pages to calculate start and end pfn based on altmap
> values. This fixes a kernel crash that is observed when destroying namespace.
> 
> [   74.745056] BUG: Unable to handle kernel data access at 0xc00c000001400000
> [   74.745256] Faulting instruction address: 0xc0000000000b58b0
> cpu 0x2: Vector: 300 (Data Access) at [c00000026ea93580]
>     pc: c0000000000b58b0: memset+0x68/0x104
>     lr: c0000000003eb008: page_init_poison+0x38/0x50
>     ...
>   current = 0xc000000271c67d80
>   paca    = 0xc00000003fffd680   irqmask: 0x03   irq_happened: 0x01
>     pid   = 3665, comm = ndctl
> [link register   ] c0000000003eb008 page_init_poison+0x38/0x50
> [c00000026ea93830] c0000000004754d4 remove_pfn_range_from_zone+0x64/0x3e0
> [c00000026ea938a0] c0000000004b8a60 memunmap_pages+0x300/0x400
> [c00000026ea93930] c0000000009e32a0 devm_action_release+0x30/0x50

Doesn't apply to mainline or -next.  Which tree is this against?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone
  2019-09-26 22:45   ` Andrew Morton
@ 2019-09-27  1:51     ` Aneesh Kumar K.V
  2019-09-27  7:46       ` David Hildenbrand
  0 siblings, 1 reply; 16+ messages in thread
From: Aneesh Kumar K.V @ 2019-09-27  1:51 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-mm, linux-nvdimm, David Hildenbrand

On 9/27/19 4:15 AM, Andrew Morton wrote:
> On Thu, 26 Sep 2019 17:55:51 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> wrote:
> 
>> With altmap, all the resource pfns are not initialized. While initializing
>> pfn, altmap reserve space is skipped. Hence when removing pfn from zone skip
>> pfns that were never initialized.
>>
>> Update memunmap_pages to calculate start and end pfn based on altmap
>> values. This fixes a kernel crash that is observed when destroying namespace.
>>
>> [   74.745056] BUG: Unable to handle kernel data access at 0xc00c000001400000
>> [   74.745256] Faulting instruction address: 0xc0000000000b58b0
>> cpu 0x2: Vector: 300 (Data Access) at [c00000026ea93580]
>>      pc: c0000000000b58b0: memset+0x68/0x104
>>      lr: c0000000003eb008: page_init_poison+0x38/0x50
>>      ...
>>    current = 0xc000000271c67d80
>>    paca    = 0xc00000003fffd680   irqmask: 0x03   irq_happened: 0x01
>>      pid   = 3665, comm = ndctl
>> [link register   ] c0000000003eb008 page_init_poison+0x38/0x50
>> [c00000026ea93830] c0000000004754d4 remove_pfn_range_from_zone+0x64/0x3e0
>> [c00000026ea938a0] c0000000004b8a60 memunmap_pages+0x300/0x400
>> [c00000026ea93930] c0000000009e32a0 devm_action_release+0x30/0x50
> 
> Doesn't apply to mainline or -next.  Which tree is this against?
> 

After applying the patches from David on mainline. That is the reason I 
replied to this thread. I should have mentioned in the email that it is 
based on patch series "[PATCH v4 0/8] mm/memory_hotplug: Shrink zones 
before removing memory"

-aneesh
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone
  2019-09-27  1:51     ` Aneesh Kumar K.V
@ 2019-09-27  7:46       ` David Hildenbrand
  2019-09-27 10:32         ` [PATCH] " Aneesh Kumar K.V
  2019-09-27 10:36         ` [PATCH 1/2] " Aneesh Kumar K.V
  0 siblings, 2 replies; 16+ messages in thread
From: David Hildenbrand @ 2019-09-27  7:46 UTC (permalink / raw)
  To: Aneesh Kumar K.V, Andrew Morton; +Cc: linux-mm, linux-nvdimm

On 27.09.19 03:51, Aneesh Kumar K.V wrote:
> On 9/27/19 4:15 AM, Andrew Morton wrote:
>> On Thu, 26 Sep 2019 17:55:51 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> wrote:
>>
>>> With altmap, all the resource pfns are not initialized. While initializing
>>> pfn, altmap reserve space is skipped. Hence when removing pfn from zone skip
>>> pfns that were never initialized.
>>>
>>> Update memunmap_pages to calculate start and end pfn based on altmap
>>> values. This fixes a kernel crash that is observed when destroying namespace.
>>>
>>> [   74.745056] BUG: Unable to handle kernel data access at 0xc00c000001400000
>>> [   74.745256] Faulting instruction address: 0xc0000000000b58b0
>>> cpu 0x2: Vector: 300 (Data Access) at [c00000026ea93580]
>>>      pc: c0000000000b58b0: memset+0x68/0x104
>>>      lr: c0000000003eb008: page_init_poison+0x38/0x50
>>>      ...
>>>    current = 0xc000000271c67d80
>>>    paca    = 0xc00000003fffd680   irqmask: 0x03   irq_happened: 0x01
>>>      pid   = 3665, comm = ndctl
>>> [link register   ] c0000000003eb008 page_init_poison+0x38/0x50
>>> [c00000026ea93830] c0000000004754d4 remove_pfn_range_from_zone+0x64/0x3e0
>>> [c00000026ea938a0] c0000000004b8a60 memunmap_pages+0x300/0x400
>>> [c00000026ea93930] c0000000009e32a0 devm_action_release+0x30/0x50
>>
>> Doesn't apply to mainline or -next.  Which tree is this against?
>>
> 
> After applying the patches from David on mainline. That is the reason I 
> replied to this thread. I should have mentioned in the email that it is 
> based on patch series "[PATCH v4 0/8] mm/memory_hotplug: Shrink zones 
> before removing memory"

So if I am not wrong, my patch "[PATCH v4 4/8] mm/memory_hotplug: Poison
memmap in remove_pfn_range_from_zone()" makes it show up that we
actually call _remove_pages() with wrong parameters, right?

If so, I guess it would be better for you to fix it before my series and
I will rebase my series on top of that.

-- 

Thanks,

David / dhildenb
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] mm/memunmap: Use the correct start and end pfn when removing pages from zone
  2019-09-27  7:46       ` David Hildenbrand
@ 2019-09-27 10:32         ` Aneesh Kumar K.V
  2019-09-27 10:38           ` David Hildenbrand
  2019-09-27 10:36         ` [PATCH 1/2] " Aneesh Kumar K.V
  1 sibling, 1 reply; 16+ messages in thread
From: Aneesh Kumar K.V @ 2019-09-27 10:32 UTC (permalink / raw)
  To: dan.j.williams, akpm, David Hildenbrand
  Cc: linux-mm, Aneesh Kumar K.V, linux-nvdimm

With altmap, all the resource pfns are not initialized. While initializing
pfn, altmap reserve space is skipped. Hence when removing pfn from zone skip
pfns that were never initialized.

Update memunmap_pages to calculate start and end pfn based on altmap
values. This fixes a kernel crash that is observed when destroying namespace.

[   81.356173] kernel BUG at include/linux/mm.h:1107!
cpu 0x1: Vector: 700 (Program Check) at [c000000274087890]
    pc: c0000000004b9728: memunmap_pages+0x238/0x340
    lr: c0000000004b9724: memunmap_pages+0x234/0x340
...
    pid   = 3669, comm = ndctl
kernel BUG at include/linux/mm.h:1107!
[c000000274087ba0] c0000000009e3500 devm_action_release+0x30/0x50
[c000000274087bc0] c0000000009e4758 release_nodes+0x268/0x2d0
[c000000274087c30] c0000000009dd144 device_release_driver_internal+0x174/0x240
[c000000274087c70] c0000000009d9dfc unbind_store+0x13c/0x190
[c000000274087cb0] c0000000009d8a24 drv_attr_store+0x44/0x60
[c000000274087cd0] c0000000005a7470 sysfs_kf_write+0x70/0xa0
[c000000274087d10] c0000000005a5cac kernfs_fop_write+0x1ac/0x290
[c000000274087d60] c0000000004be45c __vfs_write+0x3c/0x70
[c000000274087d80] c0000000004c26e4 vfs_write+0xe4/0x200
[c000000274087dd0] c0000000004c2a6c ksys_write+0x7c/0x140
[c000000274087e20] c00000000000bbd0 system_call+0x5c/0x68

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
Note:
This patch alone won't fix all the kernel crashes related wrong usage of pfn.
For ndctl destroy-namespace to work correctly we need rest of patches from
the series posted at

https://lore.kernel.org/linux-mm/20190830091428.18399-1-david@redhat.com

 mm/memremap.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/mm/memremap.c b/mm/memremap.c
index 32c79b51af86..4b31f0b7c42d 100644
--- a/mm/memremap.c
+++ b/mm/memremap.c
@@ -105,7 +105,8 @@ static void dev_pagemap_cleanup(struct dev_pagemap *pgmap)
 void memunmap_pages(struct dev_pagemap *pgmap)
 {
 	struct resource *res = &pgmap->res;
-	unsigned long pfn;
+	unsigned long start_pfn, end_pfn;
+	unsigned long pfn, nr_pages;
 	int nid;
 
 	dev_pagemap_kill(pgmap);
@@ -113,14 +114,17 @@ void memunmap_pages(struct dev_pagemap *pgmap)
 		put_page(pfn_to_page(pfn));
 	dev_pagemap_cleanup(pgmap);
 
+	start_pfn = pfn_first(pgmap);
+	end_pfn = pfn_end(pgmap);
+	nr_pages = end_pfn - start_pfn;
+
 	/* pages are dead and unused, undo the arch mapping */
-	nid = page_to_nid(pfn_to_page(PHYS_PFN(res->start)));
+	nid = page_to_nid(pfn_to_page(start_pfn));
 
 	mem_hotplug_begin();
 	if (pgmap->type == MEMORY_DEVICE_PRIVATE) {
-		pfn = PHYS_PFN(res->start);
-		__remove_pages(page_zone(pfn_to_page(pfn)), pfn,
-				 PHYS_PFN(resource_size(res)), NULL);
+		__remove_pages(page_zone(pfn_to_page(start_pfn)), start_pfn,
+			       nr_pages, NULL);
 	} else {
 		arch_remove_memory(nid, res->start, resource_size(res),
 				pgmap_altmap(pgmap));
-- 
2.21.0

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone
  2019-09-27  7:46       ` David Hildenbrand
  2019-09-27 10:32         ` [PATCH] " Aneesh Kumar K.V
@ 2019-09-27 10:36         ` Aneesh Kumar K.V
  2019-09-27 10:40           ` David Hildenbrand
  1 sibling, 1 reply; 16+ messages in thread
From: Aneesh Kumar K.V @ 2019-09-27 10:36 UTC (permalink / raw)
  To: David Hildenbrand, Andrew Morton; +Cc: linux-mm, linux-nvdimm

On 9/27/19 1:16 PM, David Hildenbrand wrote:
> On 27.09.19 03:51, Aneesh Kumar K.V wrote:
>> On 9/27/19 4:15 AM, Andrew Morton wrote:
>>> On Thu, 26 Sep 2019 17:55:51 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> wrote:
>>>
>>>> With altmap, all the resource pfns are not initialized. While initializing
>>>> pfn, altmap reserve space is skipped. Hence when removing pfn from zone skip
>>>> pfns that were never initialized.
>>>>
>>>> Update memunmap_pages to calculate start and end pfn based on altmap
>>>> values. This fixes a kernel crash that is observed when destroying namespace.
>>>>
>>>> [   74.745056] BUG: Unable to handle kernel data access at 0xc00c000001400000
>>>> [   74.745256] Faulting instruction address: 0xc0000000000b58b0
>>>> cpu 0x2: Vector: 300 (Data Access) at [c00000026ea93580]
>>>>       pc: c0000000000b58b0: memset+0x68/0x104
>>>>       lr: c0000000003eb008: page_init_poison+0x38/0x50
>>>>       ...
>>>>     current = 0xc000000271c67d80
>>>>     paca    = 0xc00000003fffd680   irqmask: 0x03   irq_happened: 0x01
>>>>       pid   = 3665, comm = ndctl
>>>> [link register   ] c0000000003eb008 page_init_poison+0x38/0x50
>>>> [c00000026ea93830] c0000000004754d4 remove_pfn_range_from_zone+0x64/0x3e0
>>>> [c00000026ea938a0] c0000000004b8a60 memunmap_pages+0x300/0x400
>>>> [c00000026ea93930] c0000000009e32a0 devm_action_release+0x30/0x50
>>>
>>> Doesn't apply to mainline or -next.  Which tree is this against?
>>>
>>
>> After applying the patches from David on mainline. That is the reason I
>> replied to this thread. I should have mentioned in the email that it is
>> based on patch series "[PATCH v4 0/8] mm/memory_hotplug: Shrink zones
>> before removing memory"
> 
> So if I am not wrong, my patch "[PATCH v4 4/8] mm/memory_hotplug: Poison
> memmap in remove_pfn_range_from_zone()" makes it show up that we
> actually call _remove_pages() with wrong parameters, right?
> 
> If so, I guess it would be better for you to fix it before my series and
> I will rebase my series on top of that.
> 

I posted a patch that can be applied to mainline. I sent that as a reply 
to this email. Can you include that and PATCH 2 as first two patches in 
your series?  That should help to locate the full patch series needed 
for fixing the kernel crash.

-aneesh
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mm/memunmap: Use the correct start and end pfn when removing pages from zone
  2019-09-27 10:32         ` [PATCH] " Aneesh Kumar K.V
@ 2019-09-27 10:38           ` David Hildenbrand
  0 siblings, 0 replies; 16+ messages in thread
From: David Hildenbrand @ 2019-09-27 10:38 UTC (permalink / raw)
  To: Aneesh Kumar K.V, dan.j.williams, akpm; +Cc: linux-mm, linux-nvdimm

On 27.09.19 12:32, Aneesh Kumar K.V wrote:
> With altmap, all the resource pfns are not initialized. While initializing
> pfn, altmap reserve space is skipped. Hence when removing pfn from zone skip
> pfns that were never initialized.
> 
> Update memunmap_pages to calculate start and end pfn based on altmap
> values. This fixes a kernel crash that is observed when destroying namespace.
> 
> [   81.356173] kernel BUG at include/linux/mm.h:1107!
> cpu 0x1: Vector: 700 (Program Check) at [c000000274087890]
>     pc: c0000000004b9728: memunmap_pages+0x238/0x340
>     lr: c0000000004b9724: memunmap_pages+0x234/0x340
> ...
>     pid   = 3669, comm = ndctl
> kernel BUG at include/linux/mm.h:1107!
> [c000000274087ba0] c0000000009e3500 devm_action_release+0x30/0x50
> [c000000274087bc0] c0000000009e4758 release_nodes+0x268/0x2d0
> [c000000274087c30] c0000000009dd144 device_release_driver_internal+0x174/0x240
> [c000000274087c70] c0000000009d9dfc unbind_store+0x13c/0x190
> [c000000274087cb0] c0000000009d8a24 drv_attr_store+0x44/0x60
> [c000000274087cd0] c0000000005a7470 sysfs_kf_write+0x70/0xa0
> [c000000274087d10] c0000000005a5cac kernfs_fop_write+0x1ac/0x290
> [c000000274087d60] c0000000004be45c __vfs_write+0x3c/0x70
> [c000000274087d80] c0000000004c26e4 vfs_write+0xe4/0x200
> [c000000274087dd0] c0000000004c2a6c ksys_write+0x7c/0x140
> [c000000274087e20] c00000000000bbd0 system_call+0x5c/0x68
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> Note:
> This patch alone won't fix all the kernel crashes related wrong usage of pfn.
> For ndctl destroy-namespace to work correctly we need rest of patches from
> the series posted at
> 
> https://lore.kernel.org/linux-mm/20190830091428.18399-1-david@redhat.com
> 
>  mm/memremap.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/memremap.c b/mm/memremap.c
> index 32c79b51af86..4b31f0b7c42d 100644
> --- a/mm/memremap.c
> +++ b/mm/memremap.c
> @@ -105,7 +105,8 @@ static void dev_pagemap_cleanup(struct dev_pagemap *pgmap)
>  void memunmap_pages(struct dev_pagemap *pgmap)
>  {
>  	struct resource *res = &pgmap->res;
> -	unsigned long pfn;
> +	unsigned long start_pfn, end_pfn;
> +	unsigned long pfn, nr_pages;

pack all into a single line?

>  	int nid;
>  
>  	dev_pagemap_kill(pgmap);
> @@ -113,14 +114,17 @@ void memunmap_pages(struct dev_pagemap *pgmap)
>  		put_page(pfn_to_page(pfn));
>  	dev_pagemap_cleanup(pgmap);
>  
> +	start_pfn = pfn_first(pgmap);
> +	end_pfn = pfn_end(pgmap);
> +	nr_pages = end_pfn - start_pfn;
> +
>  	/* pages are dead and unused, undo the arch mapping */
> -	nid = page_to_nid(pfn_to_page(PHYS_PFN(res->start)));
> +	nid = page_to_nid(pfn_to_page(start_pfn));
>  
>  	mem_hotplug_begin();
>  	if (pgmap->type == MEMORY_DEVICE_PRIVATE) {
> -		pfn = PHYS_PFN(res->start);
> -		__remove_pages(page_zone(pfn_to_page(pfn)), pfn,
> -				 PHYS_PFN(resource_size(res)), NULL);
> +		__remove_pages(page_zone(pfn_to_page(start_pfn)), start_pfn,
> +			       nr_pages, NULL);
>  	} else {
>  		arch_remove_memory(nid, res->start, resource_size(res),
>  				pgmap_altmap(pgmap));
> 

I am no expert on the details of the whole pgmap,
what-to-add-what-to-initialize-whatsoever devmem specific stuff, but
this change looks sane to me as far as I can tell.

-- 

Thanks,

David / dhildenb
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone
  2019-09-27 10:36         ` [PATCH 1/2] " Aneesh Kumar K.V
@ 2019-09-27 10:40           ` David Hildenbrand
  2019-09-27 11:35             ` Aneesh Kumar K.V
  0 siblings, 1 reply; 16+ messages in thread
From: David Hildenbrand @ 2019-09-27 10:40 UTC (permalink / raw)
  To: Aneesh Kumar K.V, Andrew Morton; +Cc: linux-mm, linux-nvdimm

On 27.09.19 12:36, Aneesh Kumar K.V wrote:
> On 9/27/19 1:16 PM, David Hildenbrand wrote:
>> On 27.09.19 03:51, Aneesh Kumar K.V wrote:
>>> On 9/27/19 4:15 AM, Andrew Morton wrote:
>>>> On Thu, 26 Sep 2019 17:55:51 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> wrote:
>>>>
>>>>> With altmap, all the resource pfns are not initialized. While initializing
>>>>> pfn, altmap reserve space is skipped. Hence when removing pfn from zone skip
>>>>> pfns that were never initialized.
>>>>>
>>>>> Update memunmap_pages to calculate start and end pfn based on altmap
>>>>> values. This fixes a kernel crash that is observed when destroying namespace.
>>>>>
>>>>> [   74.745056] BUG: Unable to handle kernel data access at 0xc00c000001400000
>>>>> [   74.745256] Faulting instruction address: 0xc0000000000b58b0
>>>>> cpu 0x2: Vector: 300 (Data Access) at [c00000026ea93580]
>>>>>       pc: c0000000000b58b0: memset+0x68/0x104
>>>>>       lr: c0000000003eb008: page_init_poison+0x38/0x50
>>>>>       ...
>>>>>     current = 0xc000000271c67d80
>>>>>     paca    = 0xc00000003fffd680   irqmask: 0x03   irq_happened: 0x01
>>>>>       pid   = 3665, comm = ndctl
>>>>> [link register   ] c0000000003eb008 page_init_poison+0x38/0x50
>>>>> [c00000026ea93830] c0000000004754d4 remove_pfn_range_from_zone+0x64/0x3e0
>>>>> [c00000026ea938a0] c0000000004b8a60 memunmap_pages+0x300/0x400
>>>>> [c00000026ea93930] c0000000009e32a0 devm_action_release+0x30/0x50
>>>>
>>>> Doesn't apply to mainline or -next.  Which tree is this against?
>>>>
>>>
>>> After applying the patches from David on mainline. That is the reason I
>>> replied to this thread. I should have mentioned in the email that it is
>>> based on patch series "[PATCH v4 0/8] mm/memory_hotplug: Shrink zones
>>> before removing memory"
>>
>> So if I am not wrong, my patch "[PATCH v4 4/8] mm/memory_hotplug: Poison
>> memmap in remove_pfn_range_from_zone()" makes it show up that we
>> actually call _remove_pages() with wrong parameters, right?
>>
>> If so, I guess it would be better for you to fix it before my series and
>> I will rebase my series on top of that.
>>
> 
> I posted a patch that can be applied to mainline. I sent that as a reply 
> to this email. Can you include that and PATCH 2 as first two patches in 
> your series?  That should help to locate the full patch series needed 
> for fixing the kernel crash.

I can drag these along, unless Andrew wants to pick them up right away
(or we're waiting for more feedback).

Is there a Fixes: Tag we can add to the first patch?

-- 

Thanks,

David / dhildenb
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone
  2019-09-27 10:40           ` David Hildenbrand
@ 2019-09-27 11:35             ` Aneesh Kumar K.V
  2019-09-27 11:38               ` David Hildenbrand
  0 siblings, 1 reply; 16+ messages in thread
From: Aneesh Kumar K.V @ 2019-09-27 11:35 UTC (permalink / raw)
  To: David Hildenbrand, Andrew Morton; +Cc: linux-mm, linux-nvdimm

On 9/27/19 4:10 PM, David Hildenbrand wrote:
> On 27.09.19 12:36, Aneesh Kumar K.V wrote:
>> On 9/27/19 1:16 PM, David Hildenbrand wrote:
>>> On 27.09.19 03:51, Aneesh Kumar K.V wrote:
>>>> On 9/27/19 4:15 AM, Andrew Morton wrote:
>>>>> On Thu, 26 Sep 2019 17:55:51 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> wrote:
>>>>>
>>>>>> With altmap, all the resource pfns are not initialized. While initializing
>>>>>> pfn, altmap reserve space is skipped. Hence when removing pfn from zone skip
>>>>>> pfns that were never initialized.
>>>>>>
>>>>>> Update memunmap_pages to calculate start and end pfn based on altmap
>>>>>> values. This fixes a kernel crash that is observed when destroying namespace.
>>>>>>
>>>>>> [   74.745056] BUG: Unable to handle kernel data access at 0xc00c000001400000
>>>>>> [   74.745256] Faulting instruction address: 0xc0000000000b58b0
>>>>>> cpu 0x2: Vector: 300 (Data Access) at [c00000026ea93580]
>>>>>>        pc: c0000000000b58b0: memset+0x68/0x104
>>>>>>        lr: c0000000003eb008: page_init_poison+0x38/0x50
>>>>>>        ...
>>>>>>      current = 0xc000000271c67d80
>>>>>>      paca    = 0xc00000003fffd680   irqmask: 0x03   irq_happened: 0x01
>>>>>>        pid   = 3665, comm = ndctl
>>>>>> [link register   ] c0000000003eb008 page_init_poison+0x38/0x50
>>>>>> [c00000026ea93830] c0000000004754d4 remove_pfn_range_from_zone+0x64/0x3e0
>>>>>> [c00000026ea938a0] c0000000004b8a60 memunmap_pages+0x300/0x400
>>>>>> [c00000026ea93930] c0000000009e32a0 devm_action_release+0x30/0x50
>>>>>
>>>>> Doesn't apply to mainline or -next.  Which tree is this against?
>>>>>
>>>>
>>>> After applying the patches from David on mainline. That is the reason I
>>>> replied to this thread. I should have mentioned in the email that it is
>>>> based on patch series "[PATCH v4 0/8] mm/memory_hotplug: Shrink zones
>>>> before removing memory"
>>>
>>> So if I am not wrong, my patch "[PATCH v4 4/8] mm/memory_hotplug: Poison
>>> memmap in remove_pfn_range_from_zone()" makes it show up that we
>>> actually call _remove_pages() with wrong parameters, right?
>>>
>>> If so, I guess it would be better for you to fix it before my series and
>>> I will rebase my series on top of that.
>>>
>>
>> I posted a patch that can be applied to mainline. I sent that as a reply
>> to this email. Can you include that and PATCH 2 as first two patches in
>> your series?  That should help to locate the full patch series needed
>> for fixing the kernel crash.
> 
> I can drag these along, unless Andrew wants to pick them up right away
> (or we're waiting for more feedback).

Considering this patch alone won't fix the issue, It would be nice if we 
could club them with rest of the changes.

> 
> Is there a Fixes: Tag we can add to the first patch?
> 

IIUC this was always broken.

-aneesh
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone
  2019-09-27 11:35             ` Aneesh Kumar K.V
@ 2019-09-27 11:38               ` David Hildenbrand
  0 siblings, 0 replies; 16+ messages in thread
From: David Hildenbrand @ 2019-09-27 11:38 UTC (permalink / raw)
  To: Aneesh Kumar K.V, Andrew Morton; +Cc: linux-mm, linux-nvdimm

On 27.09.19 13:35, Aneesh Kumar K.V wrote:
> On 9/27/19 4:10 PM, David Hildenbrand wrote:
>> On 27.09.19 12:36, Aneesh Kumar K.V wrote:
>>> On 9/27/19 1:16 PM, David Hildenbrand wrote:
>>>> On 27.09.19 03:51, Aneesh Kumar K.V wrote:
>>>>> On 9/27/19 4:15 AM, Andrew Morton wrote:
>>>>>> On Thu, 26 Sep 2019 17:55:51 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> wrote:
>>>>>>
>>>>>>> With altmap, all the resource pfns are not initialized. While initializing
>>>>>>> pfn, altmap reserve space is skipped. Hence when removing pfn from zone skip
>>>>>>> pfns that were never initialized.
>>>>>>>
>>>>>>> Update memunmap_pages to calculate start and end pfn based on altmap
>>>>>>> values. This fixes a kernel crash that is observed when destroying namespace.
>>>>>>>
>>>>>>> [   74.745056] BUG: Unable to handle kernel data access at 0xc00c000001400000
>>>>>>> [   74.745256] Faulting instruction address: 0xc0000000000b58b0
>>>>>>> cpu 0x2: Vector: 300 (Data Access) at [c00000026ea93580]
>>>>>>>        pc: c0000000000b58b0: memset+0x68/0x104
>>>>>>>        lr: c0000000003eb008: page_init_poison+0x38/0x50
>>>>>>>        ...
>>>>>>>      current = 0xc000000271c67d80
>>>>>>>      paca    = 0xc00000003fffd680   irqmask: 0x03   irq_happened: 0x01
>>>>>>>        pid   = 3665, comm = ndctl
>>>>>>> [link register   ] c0000000003eb008 page_init_poison+0x38/0x50
>>>>>>> [c00000026ea93830] c0000000004754d4 remove_pfn_range_from_zone+0x64/0x3e0
>>>>>>> [c00000026ea938a0] c0000000004b8a60 memunmap_pages+0x300/0x400
>>>>>>> [c00000026ea93930] c0000000009e32a0 devm_action_release+0x30/0x50
>>>>>>
>>>>>> Doesn't apply to mainline or -next.  Which tree is this against?
>>>>>>
>>>>>
>>>>> After applying the patches from David on mainline. That is the reason I
>>>>> replied to this thread. I should have mentioned in the email that it is
>>>>> based on patch series "[PATCH v4 0/8] mm/memory_hotplug: Shrink zones
>>>>> before removing memory"
>>>>
>>>> So if I am not wrong, my patch "[PATCH v4 4/8] mm/memory_hotplug: Poison
>>>> memmap in remove_pfn_range_from_zone()" makes it show up that we
>>>> actually call _remove_pages() with wrong parameters, right?
>>>>
>>>> If so, I guess it would be better for you to fix it before my series and
>>>> I will rebase my series on top of that.
>>>>
>>>
>>> I posted a patch that can be applied to mainline. I sent that as a reply
>>> to this email. Can you include that and PATCH 2 as first two patches in
>>> your series?  That should help to locate the full patch series needed
>>> for fixing the kernel crash.
>>
>> I can drag these along, unless Andrew wants to pick them up right away
>> (or we're waiting for more feedback).
> 
> Considering this patch alone won't fix the issue, It would be nice if we 
> could club them with rest of the changes.
> 

I'll drag them along, adding Pankaj's RB's. If they get picked up
independently, fine :)

-- 

Thanks,

David / dhildenb
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-09-27 11:38 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20190830091428.18399-1-david@redhat.com>
2019-09-26 12:25 ` [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone Aneesh Kumar K.V
2019-09-26 12:25   ` [PATCH 2/2] mm/memmap_init: Update variable name in memmap_init_zone Aneesh Kumar K.V
2019-09-26 12:56     ` David Hildenbrand
2019-09-26 13:38     ` Pankaj Gupta
2019-09-26 12:43   ` [PATCH 1/2] mm/memunmap: Use the correct start and end pfn when removing pages from zone David Hildenbrand
2019-09-26 13:15     ` Aneesh Kumar K.V
2019-09-26 13:34   ` Pankaj Gupta
2019-09-26 22:45   ` Andrew Morton
2019-09-27  1:51     ` Aneesh Kumar K.V
2019-09-27  7:46       ` David Hildenbrand
2019-09-27 10:32         ` [PATCH] " Aneesh Kumar K.V
2019-09-27 10:38           ` David Hildenbrand
2019-09-27 10:36         ` [PATCH 1/2] " Aneesh Kumar K.V
2019-09-27 10:40           ` David Hildenbrand
2019-09-27 11:35             ` Aneesh Kumar K.V
2019-09-27 11:38               ` David Hildenbrand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).