[PATCH 15/17] memremap: drop private struct page_map

2017-12-15 Thread Christoph Hellwig
From: Logan Gunthorpe 

'struct page_map' is a private structure of 'struct dev_pagemap' but the
latter replicates all the same fields as the former so there isn't much
value in it. Thus drop it in favour of a completely public struct.

This is a clean up in preperation for a more generally useful
'devm_memeremap_pages' interface.

Signed-off-by: Logan Gunthorpe 
Signed-off-by: Christoph Hellwig 
---
 include/linux/memremap.h |  5 ++--
 kernel/memremap.c| 66 ++--
 mm/hmm.c |  2 +-
 3 files changed, 29 insertions(+), 44 deletions(-)

diff --git a/include/linux/memremap.h b/include/linux/memremap.h
index 3fddcfe57bb0..1cb5f39d25c1 100644
--- a/include/linux/memremap.h
+++ b/include/linux/memremap.h
@@ -113,8 +113,9 @@ typedef void (*dev_page_free_t)(struct page *page, void 
*data);
 struct dev_pagemap {
dev_page_fault_t page_fault;
dev_page_free_t page_free;
-   struct vmem_altmap *altmap;
-   const struct resource *res;
+   struct vmem_altmap altmap;
+   bool altmap_valid;
+   struct resource res;
struct percpu_ref *ref;
struct device *dev;
void *data;
diff --git a/kernel/memremap.c b/kernel/memremap.c
index 901404094df1..97782215bbd4 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -188,13 +188,6 @@ static RADIX_TREE(pgmap_radix, GFP_KERNEL);
 #define SECTION_MASK ~((1UL << PA_SECTION_SHIFT) - 1)
 #define SECTION_SIZE (1UL << PA_SECTION_SHIFT)
 
-struct page_map {
-   struct resource res;
-   struct percpu_ref *ref;
-   struct dev_pagemap pgmap;
-   struct vmem_altmap altmap;
-};
-
 static unsigned long order_at(struct resource *res, unsigned long pgoff)
 {
unsigned long phys_pgoff = PHYS_PFN(res->start) + pgoff;
@@ -260,22 +253,21 @@ static void pgmap_radix_release(struct resource *res)
synchronize_rcu();
 }
 
-static unsigned long pfn_first(struct page_map *page_map)
+static unsigned long pfn_first(struct dev_pagemap *pgmap)
 {
-   struct dev_pagemap *pgmap = &page_map->pgmap;
-   const struct resource *res = &page_map->res;
-   struct vmem_altmap *altmap = pgmap->altmap;
+   const struct resource *res = &pgmap->res;
+   struct vmem_altmap *altmap = &pgmap->altmap;
unsigned long pfn;
 
pfn = res->start >> PAGE_SHIFT;
-   if (altmap)
+   if (pgmap->altmap_valid)
pfn += vmem_altmap_offset(altmap);
return pfn;
 }
 
-static unsigned long pfn_end(struct page_map *page_map)
+static unsigned long pfn_end(struct dev_pagemap *pgmap)
 {
-   const struct resource *res = &page_map->res;
+   const struct resource *res = &pgmap->res;
 
return (res->start + resource_size(res)) >> PAGE_SHIFT;
 }
@@ -285,13 +277,12 @@ static unsigned long pfn_end(struct page_map *page_map)
 
 static void devm_memremap_pages_release(struct device *dev, void *data)
 {
-   struct page_map *page_map = data;
-   struct resource *res = &page_map->res;
+   struct dev_pagemap *pgmap = data;
+   struct resource *res = &pgmap->res;
resource_size_t align_start, align_size;
-   struct dev_pagemap *pgmap = &page_map->pgmap;
unsigned long pfn;
 
-   for_each_device_pfn(pfn, page_map)
+   for_each_device_pfn(pfn, pgmap)
put_page(pfn_to_page(pfn));
 
if (percpu_ref_tryget_live(pgmap->ref)) {
@@ -304,24 +295,22 @@ static void devm_memremap_pages_release(struct device 
*dev, void *data)
align_size = ALIGN(resource_size(res), SECTION_SIZE);
 
mem_hotplug_begin();
-   arch_remove_memory(align_start, align_size, pgmap->altmap);
+   arch_remove_memory(align_start, align_size, pgmap->altmap_valid ?
+   &pgmap->altmap : NULL);
mem_hotplug_done();
 
untrack_pfn(NULL, PHYS_PFN(align_start), align_size);
pgmap_radix_release(res);
-   dev_WARN_ONCE(dev, pgmap->altmap && pgmap->altmap->alloc,
-   "%s: failed to free all reserved pages\n", __func__);
+   dev_WARN_ONCE(dev, pgmap->altmap.alloc,
+ "%s: failed to free all reserved pages\n", __func__);
 }
 
 /* assumes rcu_read_lock() held at entry */
 static struct dev_pagemap *find_dev_pagemap(resource_size_t phys)
 {
-   struct page_map *page_map;
-
WARN_ON_ONCE(!rcu_read_lock_held());
 
-   page_map = radix_tree_lookup(&pgmap_radix, PHYS_PFN(phys));
-   return page_map ? &page_map->pgmap : NULL;
+   return radix_tree_lookup(&pgmap_radix, PHYS_PFN(phys));
 }
 
 /**
@@ -349,7 +338,6 @@ void *devm_memremap_pages(struct device *dev, struct 
resource *res,
unsigned long pfn, pgoff, order;
pgprot_t pgprot = PAGE_KERNEL;
struct dev_pagemap *pgmap;
-   struct page_map *page_map;
int error, nid, is_ram, i = 0;
 
align_start = res->start & ~(SECTION_SIZE - 1);
@@ -370,21 +358,19 @@ void *devm_memremap_pages(struct device *dev, stru

[PATCH 15/17] memremap: drop private struct page_map

2017-12-29 Thread Christoph Hellwig
From: Logan Gunthorpe 

'struct page_map' is a private structure of 'struct dev_pagemap' but the
latter replicates all the same fields as the former so there isn't much
value in it. Thus drop it in favour of a completely public struct.

This is a clean up in preperation for a more generally useful
'devm_memeremap_pages' interface.

Signed-off-by: Logan Gunthorpe 
Signed-off-by: Christoph Hellwig 
Reviewed-by: Dan Williams 
---
 include/linux/memremap.h |  5 ++--
 kernel/memremap.c| 68 ++--
 mm/hmm.c |  2 +-
 3 files changed, 30 insertions(+), 45 deletions(-)

diff --git a/include/linux/memremap.h b/include/linux/memremap.h
index 3fddcfe57bb0..1cb5f39d25c1 100644
--- a/include/linux/memremap.h
+++ b/include/linux/memremap.h
@@ -113,8 +113,9 @@ typedef void (*dev_page_free_t)(struct page *page, void 
*data);
 struct dev_pagemap {
dev_page_fault_t page_fault;
dev_page_free_t page_free;
-   struct vmem_altmap *altmap;
-   const struct resource *res;
+   struct vmem_altmap altmap;
+   bool altmap_valid;
+   struct resource res;
struct percpu_ref *ref;
struct device *dev;
void *data;
diff --git a/kernel/memremap.c b/kernel/memremap.c
index 12e78528fea4..9207c44cce20 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -188,13 +188,6 @@ static RADIX_TREE(pgmap_radix, GFP_KERNEL);
 #define SECTION_MASK ~((1UL << PA_SECTION_SHIFT) - 1)
 #define SECTION_SIZE (1UL << PA_SECTION_SHIFT)
 
-struct page_map {
-   struct resource res;
-   struct percpu_ref *ref;
-   struct dev_pagemap pgmap;
-   struct vmem_altmap altmap;
-};
-
 static unsigned long order_at(struct resource *res, unsigned long pgoff)
 {
unsigned long phys_pgoff = PHYS_PFN(res->start) + pgoff;
@@ -260,22 +253,21 @@ static void pgmap_radix_release(struct resource *res)
synchronize_rcu();
 }
 
-static unsigned long pfn_first(struct page_map *page_map)
+static unsigned long pfn_first(struct dev_pagemap *pgmap)
 {
-   struct dev_pagemap *pgmap = &page_map->pgmap;
-   const struct resource *res = &page_map->res;
-   struct vmem_altmap *altmap = pgmap->altmap;
+   const struct resource *res = &pgmap->res;
+   struct vmem_altmap *altmap = &pgmap->altmap;
unsigned long pfn;
 
pfn = res->start >> PAGE_SHIFT;
-   if (altmap)
+   if (pgmap->altmap_valid)
pfn += vmem_altmap_offset(altmap);
return pfn;
 }
 
-static unsigned long pfn_end(struct page_map *page_map)
+static unsigned long pfn_end(struct dev_pagemap *pgmap)
 {
-   const struct resource *res = &page_map->res;
+   const struct resource *res = &pgmap->res;
 
return (res->start + resource_size(res)) >> PAGE_SHIFT;
 }
@@ -285,13 +277,12 @@ static unsigned long pfn_end(struct page_map *page_map)
 
 static void devm_memremap_pages_release(struct device *dev, void *data)
 {
-   struct page_map *page_map = data;
-   struct resource *res = &page_map->res;
+   struct dev_pagemap *pgmap = data;
+   struct resource *res = &pgmap->res;
resource_size_t align_start, align_size;
-   struct dev_pagemap *pgmap = &page_map->pgmap;
unsigned long pfn;
 
-   for_each_device_pfn(pfn, page_map)
+   for_each_device_pfn(pfn, pgmap)
put_page(pfn_to_page(pfn));
 
if (percpu_ref_tryget_live(pgmap->ref)) {
@@ -304,24 +295,22 @@ static void devm_memremap_pages_release(struct device 
*dev, void *data)
align_size = ALIGN(resource_size(res), SECTION_SIZE);
 
mem_hotplug_begin();
-   arch_remove_memory(align_start, align_size, pgmap->altmap);
+   arch_remove_memory(align_start, align_size, pgmap->altmap_valid ?
+   &pgmap->altmap : NULL);
mem_hotplug_done();
 
untrack_pfn(NULL, PHYS_PFN(align_start), align_size);
pgmap_radix_release(res);
-   dev_WARN_ONCE(dev, pgmap->altmap && pgmap->altmap->alloc,
-   "%s: failed to free all reserved pages\n", __func__);
+   dev_WARN_ONCE(dev, pgmap->altmap.alloc,
+ "%s: failed to free all reserved pages\n", __func__);
 }
 
 /* assumes rcu_read_lock() held at entry */
 static struct dev_pagemap *find_dev_pagemap(resource_size_t phys)
 {
-   struct page_map *page_map;
-
WARN_ON_ONCE(!rcu_read_lock_held());
 
-   page_map = radix_tree_lookup(&pgmap_radix, PHYS_PFN(phys));
-   return page_map ? &page_map->pgmap : NULL;
+   return radix_tree_lookup(&pgmap_radix, PHYS_PFN(phys));
 }
 
 /**
@@ -349,7 +338,6 @@ void *devm_memremap_pages(struct device *dev, struct 
resource *res,
unsigned long pfn, pgoff, order;
pgprot_t pgprot = PAGE_KERNEL;
struct dev_pagemap *pgmap;
-   struct page_map *page_map;
int error, nid, is_ram, i = 0;
 
align_start = res->start & ~(SECTION_SIZE - 1);
@@ -370,22 +358,20 @@ void *devm_memremap_pag

Re: [PATCH 15/17] memremap: drop private struct page_map

2017-12-17 Thread Dan Williams
On Fri, Dec 15, 2017 at 6:09 AM, Christoph Hellwig  wrote:
> From: Logan Gunthorpe 
>
> 'struct page_map' is a private structure of 'struct dev_pagemap' but the
> latter replicates all the same fields as the former so there isn't much
> value in it. Thus drop it in favour of a completely public struct.
>
> This is a clean up in preperation for a more generally useful
> 'devm_memeremap_pages' interface.
>
> Signed-off-by: Logan Gunthorpe 
> Signed-off-by: Christoph Hellwig 

Looks good,

Reviewed-by: Dan Williams