xen: dma-buf support for grant device

From: Oleksandr Andrushchenko <[email protected]>

Make set/clear page private code shared and accessible to other kernel modules which can re-use these instead of open-coding.

Signed-off-by Oleksandr Andrushchenko <[email protected]>

From: Oleksandr Andrushchenko <[email protected]>

Make set/clear page private code shared and accessible to other kernel modules which can re-use these instead of open-coding.

Signed-off-by Oleksandr Andrushchenko <[email protected]>

From: Oleksandr Andrushchenko <[email protected]>

Make set/clear page private code shared and accessible to other kernel modules which can re-use these instead of open-coding.

Signed-off-by Oleksandr Andrushchenko <[email protected]>

---
 drivers/xen/grant-table.c | 54 +++++++++++++++++++++++++--------------
 include/xen/grant_table.h |  3 +++
 2 files changed, 38 insertions(+), 19 deletions(-)

drivers/xen/grant-table.c +3 -14

index 27be107d6480..d7488226e1f2 100644
@@ -769,29 +769,18 @@ void gnttab_free_auto_xlat_frames(void)
 }
 EXPORT_SYMBOL_GPL(gnttab_free_auto_xlat_frames);
 
-/**
- * gnttab_alloc_pages - alloc pages suitable for grant mapping into
- * @nr_pages: number of pages to alloc
- * @pages: returns the pages
- */
-int gnttab_alloc_pages(int nr_pages, struct page **pages)
+int gnttab_pages_set_private(int nr_pages, struct page **pages)
 {
 	int i;
-	int ret;
-
-	ret = alloc_xenballooned_pages(nr_pages, pages);
-	if (ret < 0)
-		return ret;
 
 	for (i = 0; i < nr_pages; i++) {
 #if BITS_PER_LONG < 64
 		struct xen_page_foreign *foreign;
 
 		foreign = kzalloc(sizeof(*foreign), GFP_KERNEL);
-		if (!foreign) {
-			gnttab_free_pages(nr_pages, pages);
+		if (!foreign)
 			return -ENOMEM;
-		}
+
 		set_page_private(pages[i], (unsigned long)foreign);
 #endif
 		SetPagePrivate(pages[i]);

drivers/xen/grant-table.c +20 -4

 /**
- * gnttab_free_pages - free pages allocated by gnttab_alloc_pages()
- * @nr_pages; number of pages to free
- * @pages: the pages
+ * gnttab_alloc_pages - alloc pages suitable for grant mapping into
+ * @nr_pages: number of pages to alloc
+ * @pages: returns the pages
  */
-void gnttab_free_pages(int nr_pages, struct page **pages)
+int gnttab_alloc_pages(int nr_pages, struct page **pages)
+{
+	int ret;
+
+	ret = alloc_xenballooned_pages(nr_pages, pages);
+	if (ret < 0)
+		return ret;
+
+	ret = gnttab_pages_set_private(nr_pages, pages);
+	if (ret < 0)
+		gnttab_free_pages(nr_pages, pages);
+
+	return ret;
+}
+EXPORT_SYMBOL(gnttab_alloc_pages);
+
+void gnttab_pages_clear_private(int nr_pages, struct page **pages)
 {
 	int i;
 

Quoted 50 lines · Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <[email protected]>

Make set/clear page private code shared and accessible to other kernel modules which can re-use these instead of open-coding.

Signed-off-by Oleksandr Andrushchenko <[email protected]>

---
  drivers/xen/grant-table.c | 54 +++++++++++++++++++++++++--------------
  include/xen/grant_table.h |  3 +++
  2 files changed, 38 insertions(+), 19 deletions(-)

drivers/xen/grant-table.c +3 -14

index 27be107d6480..d7488226e1f2 100644
@@ -769,29 +769,18 @@ void gnttab_free_auto_xlat_frames(void)
  }
  EXPORT_SYMBOL_GPL(gnttab_free_auto_xlat_frames);
 
-/**
- * gnttab_alloc_pages - alloc pages suitable for grant mapping into
- * @nr_pages: number of pages to alloc
- * @pages: returns the pages
- */
-int gnttab_alloc_pages(int nr_pages, struct page **pages)
+int gnttab_pages_set_private(int nr_pages, struct page **pages)
  {
  	int i;
-	int ret;
-
-	ret = alloc_xenballooned_pages(nr_pages, pages);
-	if (ret < 0)
-		return ret;
 
  	for (i = 0; i < nr_pages; i++) {
  #if BITS_PER_LONG < 64
  		struct xen_page_foreign *foreign;
 
  		foreign = kzalloc(sizeof(*foreign), GFP_KERNEL);
-		if (!foreign) {
-			gnttab_free_pages(nr_pages, pages);
+		if (!foreign)
  			return -ENOMEM;
-		}
+
  		set_page_private(pages[i], (unsigned long)foreign);
  #endif
  		SetPagePrivate(pages[i]);

@@ -799,14 +788,30 @@ int gnttab_alloc_pages(int nr_pages, struct page **pages)
 

drivers/xen/grant-table.c int gnttab_alloc_pages(int nr_pages, struct page **pages) · line 791

Quoted 4 lines

drivers/xen/grant-table.c +1 -1

  	return 0;
  }
-EXPORT_SYMBOL(gnttab_alloc_pages);
+EXPORT_SYMBOL(gnttab_pages_set_private);

EXPORT_SYMBOL_GPL()

Quoted 30 lines

drivers/xen/grant-table.c +20 -4

  /**
- * gnttab_free_pages - free pages allocated by gnttab_alloc_pages()
- * @nr_pages; number of pages to free
- * @pages: the pages
+ * gnttab_alloc_pages - alloc pages suitable for grant mapping into
+ * @nr_pages: number of pages to alloc
+ * @pages: returns the pages
   */
-void gnttab_free_pages(int nr_pages, struct page **pages)
+int gnttab_alloc_pages(int nr_pages, struct page **pages)
+{
+	int ret;
+
+	ret = alloc_xenballooned_pages(nr_pages, pages);
+	if (ret < 0)
+		return ret;
+
+	ret = gnttab_pages_set_private(nr_pages, pages);
+	if (ret < 0)
+		gnttab_free_pages(nr_pages, pages);
+
+	return ret;
+}
+EXPORT_SYMBOL(gnttab_alloc_pages);
+
+void gnttab_pages_clear_private(int nr_pages, struct page **pages)
  {
  	int i;
 

drivers/xen/grant-table.c void gnttab_free_pages(int nr_pages, struct page **pages) · line 827

Quoted 6 lines

drivers/xen/grant-table.c +2 -0

@@ -818,6 +823,17 @@ void gnttab_free_pages(int nr_pages, struct page **pages)
  			ClearPagePrivate(pages[i]);
  		}
  	}
+}
+EXPORT_SYMBOL(gnttab_pages_clear_private);

EXPORT_SYMBOL_GPL()

Juergen

Quoted 50 lines · Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <[email protected]>

Make set/clear page private code shared and accessible to other kernel modules which can re-use these instead of open-coding.

Signed-off-by Oleksandr Andrushchenko <[email protected]>

---
  drivers/xen/grant-table.c | 54 +++++++++++++++++++++++++--------------
  include/xen/grant_table.h |  3 +++
  2 files changed, 38 insertions(+), 19 deletions(-)

drivers/xen/grant-table.c +3 -14

index 27be107d6480..d7488226e1f2 100644
@@ -769,29 +769,18 @@ void gnttab_free_auto_xlat_frames(void)
  }
  EXPORT_SYMBOL_GPL(gnttab_free_auto_xlat_frames);
 
-/**
- * gnttab_alloc_pages - alloc pages suitable for grant mapping into
- * @nr_pages: number of pages to alloc
- * @pages: returns the pages
- */
-int gnttab_alloc_pages(int nr_pages, struct page **pages)
+int gnttab_pages_set_private(int nr_pages, struct page **pages)
  {
  	int i;
-	int ret;
-
-	ret = alloc_xenballooned_pages(nr_pages, pages);
-	if (ret < 0)
-		return ret;
 
  	for (i = 0; i < nr_pages; i++) {
  #if BITS_PER_LONG < 64
  		struct xen_page_foreign *foreign;
 
  		foreign = kzalloc(sizeof(*foreign), GFP_KERNEL);
-		if (!foreign) {
-			gnttab_free_pages(nr_pages, pages);
+		if (!foreign)
  			return -ENOMEM;
-		}
+
  		set_page_private(pages[i], (unsigned long)foreign);
  #endif
  		SetPagePrivate(pages[i]);

@@ -799,14 +788,30 @@ int gnttab_alloc_pages(int nr_pages, struct page **pages)
 

drivers/xen/grant-table.c int gnttab_alloc_pages(int nr_pages, struct page **pages) · line 791

Quoted 4 lines

drivers/xen/grant-table.c +1 -1

  	return 0;
  }
-EXPORT_SYMBOL(gnttab_alloc_pages);
+EXPORT_SYMBOL(gnttab_pages_set_private);

EXPORT_SYMBOL_GPL()

Quoted 30 lines

drivers/xen/grant-table.c +20 -4

  /**
- * gnttab_free_pages - free pages allocated by gnttab_alloc_pages()
- * @nr_pages; number of pages to free
- * @pages: the pages
+ * gnttab_alloc_pages - alloc pages suitable for grant mapping into
+ * @nr_pages: number of pages to alloc
+ * @pages: returns the pages
   */
-void gnttab_free_pages(int nr_pages, struct page **pages)
+int gnttab_alloc_pages(int nr_pages, struct page **pages)
+{
+	int ret;
+
+	ret = alloc_xenballooned_pages(nr_pages, pages);
+	if (ret < 0)
+		return ret;
+
+	ret = gnttab_pages_set_private(nr_pages, pages);
+	if (ret < 0)
+		gnttab_free_pages(nr_pages, pages);
+
+	return ret;
+}
+EXPORT_SYMBOL(gnttab_alloc_pages);
+
+void gnttab_pages_clear_private(int nr_pages, struct page **pages)
  {
  	int i;
 

drivers/xen/grant-table.c void gnttab_free_pages(int nr_pages, struct page **pages) · line 827

Quoted 6 lines

drivers/xen/grant-table.c +2 -0

@@ -818,6 +823,17 @@ void gnttab_free_pages(int nr_pages, struct page **pages)
  			ClearPagePrivate(pages[i]);
  		}
  	}
+}
+EXPORT_SYMBOL(gnttab_pages_clear_private);

EXPORT_SYMBOL_GPL()

Juergen

From: Oleksandr Andrushchenko <[email protected]>

Make set/clear page private code shared and accessible to other kernel modules which can re-use these instead of open-coding.

Signed-off-by Oleksandr Andrushchenko <[email protected]>

---
 drivers/xen/grant-table.c | 54 +++++++++++++++++++++++++--------------
 include/xen/grant_table.h |  3 +++
 2 files changed, 38 insertions(+), 19 deletions(-)

drivers/xen/grant-table.c +3 -14

index 27be107d6480..d7488226e1f2 100644
@@ -769,29 +769,18 @@ void gnttab_free_auto_xlat_frames(void)
 }
 EXPORT_SYMBOL_GPL(gnttab_free_auto_xlat_frames);
 
-/**
- * gnttab_alloc_pages - alloc pages suitable for grant mapping into
- * @nr_pages: number of pages to alloc
- * @pages: returns the pages
- */
-int gnttab_alloc_pages(int nr_pages, struct page **pages)
+int gnttab_pages_set_private(int nr_pages, struct page **pages)
 {
 	int i;
-	int ret;
-
-	ret = alloc_xenballooned_pages(nr_pages, pages);
-	if (ret < 0)
-		return ret;
 
 	for (i = 0; i < nr_pages; i++) {
 #if BITS_PER_LONG < 64
 		struct xen_page_foreign *foreign;
 
 		foreign = kzalloc(sizeof(*foreign), GFP_KERNEL);
-		if (!foreign) {
-			gnttab_free_pages(nr_pages, pages);
+		if (!foreign)
 			return -ENOMEM;
-		}
+
 		set_page_private(pages[i], (unsigned long)foreign);
 #endif
 		SetPagePrivate(pages[i]);

drivers/xen/grant-table.c +20 -4

 /**
- * gnttab_free_pages - free pages allocated by gnttab_alloc_pages()
- * @nr_pages; number of pages to free
- * @pages: the pages
+ * gnttab_alloc_pages - alloc pages suitable for grant mapping into
+ * @nr_pages: number of pages to alloc
+ * @pages: returns the pages
  */
-void gnttab_free_pages(int nr_pages, struct page **pages)
+int gnttab_alloc_pages(int nr_pages, struct page **pages)
+{
+	int ret;
+
+	ret = alloc_xenballooned_pages(nr_pages, pages);
+	if (ret < 0)
+		return ret;
+
+	ret = gnttab_pages_set_private(nr_pages, pages);
+	if (ret < 0)
+		gnttab_free_pages(nr_pages, pages);
+
+	return ret;
+}
+EXPORT_SYMBOL(gnttab_alloc_pages);
+
+void gnttab_pages_clear_private(int nr_pages, struct page **pages)
 {
 	int i;
 

From: Oleksandr Andrushchenko <[email protected]>

Make set/clear page private code shared and accessible to other kernel modules which can re-use these instead of open-coding.

Signed-off-by Oleksandr Andrushchenko <[email protected]>

---
 drivers/xen/grant-table.c | 54 +++++++++++++++++++++++++--------------
 include/xen/grant_table.h |  3 +++
 2 files changed, 38 insertions(+), 19 deletions(-)

drivers/xen/grant-table.c +1 -10

index 27be107d6480..d7488226e1f2 100644
@@ -769,29 +769,18 @@ void gnttab_free_auto_xlat_frames(void)
 }
 EXPORT_SYMBOL_GPL(gnttab_free_auto_xlat_frames);
 
-/**
- * gnttab_alloc_pages - alloc pages suitable for grant mapping into
- * @nr_pages: number of pages to alloc
- * @pages: returns the pages
- */
-int gnttab_alloc_pages(int nr_pages, struct page **pages)
+int gnttab_pages_set_private(int nr_pages, struct page **pages)
 {
 	int i;
-	int ret;
-
-	ret = alloc_xenballooned_pages(nr_pages, pages);
-	if (ret < 0)

drivers/xen/grant-table.c +33 -6

 			return -ENOMEM;
-		}
+
 		set_page_private(pages[i], (unsigned long)foreign);
 #endif
 		SetPagePrivate(pages[i]);

@@ -799,14 +788,30 @@ int gnttab_alloc_pages(int nr_pages, struct page **pages)
 
 	return 0;
 }
-EXPORT_SYMBOL(gnttab_alloc_pages);
+EXPORT_SYMBOL(gnttab_pages_set_private);
 
 /**
- * gnttab_free_pages - free pages allocated by gnttab_alloc_pages()
- * @nr_pages; number of pages to free
- * @pages: the pages
+ * gnttab_alloc_pages - alloc pages suitable for grant mapping into
+ * @nr_pages: number of pages to alloc
+ * @pages: returns the pages
  */
-void gnttab_free_pages(int nr_pages, struct page **pages)
+int gnttab_alloc_pages(int nr_pages, struct page **pages)
+{
+	int ret;
+
+	ret = alloc_xenballooned_pages(nr_pages, pages);
+	if (ret < 0)
+		return ret;
+
+	ret = gnttab_pages_set_private(nr_pages, pages);
+	if (ret < 0)
+		gnttab_free_pages(nr_pages, pages);
+
+	return ret;
+}
+EXPORT_SYMBOL(gnttab_alloc_pages);
+
+void gnttab_pages_clear_private(int nr_pages, struct page **pages)
 {
 	int i;
 

@@ -818,6 +823,17 @@ void gnttab_free_pages(int nr_pages, struct page **pages)
 			ClearPagePrivate(pages[i]);
 		}
 	}
+}
+EXPORT_SYMBOL(gnttab_pages_clear_private);
+
+/**
+ * gnttab_free_pages - free pages allocated by gnttab_alloc_pages()
+ * @nr_pages; number of pages to free
+ * @pages: the pages
+ */
+void gnttab_free_pages(int nr_pages, struct page **pages)
+{
+	gnttab_pages_clear_private(nr_pages, pages);
 	free_xenballooned_pages(nr_pages, pages);
 }
 EXPORT_SYMBOL(gnttab_free_pages);

include/xen/grant_table.h +3 -0

index 2e37741f6b8d..de03f2542bb7 100644
@@ -198,6 +198,9 @@ void gnttab_free_auto_xlat_frames(void);
 int gnttab_alloc_pages(int nr_pages, struct page **pages);
 void gnttab_free_pages(int nr_pages, struct page **pages);
 
+int gnttab_pages_set_private(int nr_pages, struct page **pages);
+void gnttab_pages_clear_private(int nr_pages, struct page **pages);
+
 int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
 		    struct gnttab_map_grant_ref *kmap_ops,
 		    struct page **pages, unsigned int count);

Signature · 1 line

2.17.0

Quoted 38 lines · Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <[email protected]>

Make set/clear page private code shared and accessible to other kernel modules which can re-use these instead of open-coding.

Signed-off-by Oleksandr Andrushchenko <[email protected]>

---
  drivers/xen/grant-table.c | 54 +++++++++++++++++++++++++--------------
  include/xen/grant_table.h |  3 +++
  2 files changed, 38 insertions(+), 19 deletions(-)

drivers/xen/grant-table.c +1 -11

index 27be107d6480..d7488226e1f2 100644
@@ -769,29 +769,18 @@ void gnttab_free_auto_xlat_frames(void)
  }
  EXPORT_SYMBOL_GPL(gnttab_free_auto_xlat_frames);
 
-/**
- * gnttab_alloc_pages - alloc pages suitable for grant mapping into
- * @nr_pages: number of pages to alloc
- * @pages: returns the pages
- */
-int gnttab_alloc_pages(int nr_pages, struct page **pages)
+int gnttab_pages_set_private(int nr_pages, struct page **pages)
  {
  	int i;
-	int ret;
-
-	ret = alloc_xenballooned_pages(nr_pages, pages);
-	if (ret < 0)
-		return ret;
 
  	for (i = 0; i < nr_pages; i++) {
  #if BITS_PER_LONG < 64
  		struct xen_page_foreign *foreign;
 

drivers/xen/grant-table.c void gnttab_free_auto_xlat_frames(void) · line 781

Quoted 4 lines

drivers/xen/grant-table.c +1 -2

  		foreign = kzalloc(sizeof(*foreign), GFP_KERNEL);
-		if (!foreign) {
-			gnttab_free_pages(nr_pages, pages);
+		if (!foreign)

Don't we have to free previously allocated "foreign"(s) if it fails in the middle (e.g. 0 < i && i < nr_pages - 1) before returning?

Quoted 68 lines

drivers/xen/grant-table.c +33 -6

  			return -ENOMEM;
-		}
+
  		set_page_private(pages[i], (unsigned long)foreign);
  #endif
  		SetPagePrivate(pages[i]);

@@ -799,14 +788,30 @@ int gnttab_alloc_pages(int nr_pages, struct page **pages)
 
  	return 0;
  }
-EXPORT_SYMBOL(gnttab_alloc_pages);
+EXPORT_SYMBOL(gnttab_pages_set_private);
 
  /**
- * gnttab_free_pages - free pages allocated by gnttab_alloc_pages()
- * @nr_pages; number of pages to free
- * @pages: the pages
+ * gnttab_alloc_pages - alloc pages suitable for grant mapping into
+ * @nr_pages: number of pages to alloc
+ * @pages: returns the pages
   */
-void gnttab_free_pages(int nr_pages, struct page **pages)
+int gnttab_alloc_pages(int nr_pages, struct page **pages)
+{
+	int ret;
+
+	ret = alloc_xenballooned_pages(nr_pages, pages);
+	if (ret < 0)
+		return ret;
+
+	ret = gnttab_pages_set_private(nr_pages, pages);
+	if (ret < 0)
+		gnttab_free_pages(nr_pages, pages);
+
+	return ret;
+}
+EXPORT_SYMBOL(gnttab_alloc_pages);
+
+void gnttab_pages_clear_private(int nr_pages, struct page **pages)
  {
  	int i;
 

@@ -818,6 +823,17 @@ void gnttab_free_pages(int nr_pages, struct page **pages)
  			ClearPagePrivate(pages[i]);
  		}
  	}
+}
+EXPORT_SYMBOL(gnttab_pages_clear_private);
+
+/**
+ * gnttab_free_pages - free pages allocated by gnttab_alloc_pages()
+ * @nr_pages; number of pages to free
+ * @pages: the pages
+ */
+void gnttab_free_pages(int nr_pages, struct page **pages)
+{
+	gnttab_pages_clear_private(nr_pages, pages);
  	free_xenballooned_pages(nr_pages, pages);
  }
  EXPORT_SYMBOL(gnttab_free_pages);

include/xen/grant_table.h +0 -0

index 2e37741f6b8d..de03f2542bb7 100644
@@ -198,6 +198,9 @@ void gnttab_free_auto_xlat_frames(void);
  int gnttab_alloc_pages(int nr_pages, struct page **pages);
  void gnttab_free_pages(int nr_pages, struct page **pages);
 

Quoted 9 lines

include/xen/grant_table.h +3 -0

+int gnttab_pages_set_private(int nr_pages, struct page **pages);
+void gnttab_pages_clear_private(int nr_pages, struct page **pages);
+
  int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
  		    struct gnttab_map_grant_ref *kmap_ops,
  		    struct page **pages, unsigned int count);

Signature · 1 line

2.17.0

Quoted 38 lines · Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <[email protected]>

Make set/clear page private code shared and accessible to other kernel modules which can re-use these instead of open-coding.

Signed-off-by Oleksandr Andrushchenko <[email protected]>

---
  drivers/xen/grant-table.c | 54 +++++++++++++++++++++++++--------------
  include/xen/grant_table.h |  3 +++
  2 files changed, 38 insertions(+), 19 deletions(-)

drivers/xen/grant-table.c +1 -11

index 27be107d6480..d7488226e1f2 100644
@@ -769,29 +769,18 @@ void gnttab_free_auto_xlat_frames(void)
  }
  EXPORT_SYMBOL_GPL(gnttab_free_auto_xlat_frames);
 
-/**
- * gnttab_alloc_pages - alloc pages suitable for grant mapping into
- * @nr_pages: number of pages to alloc
- * @pages: returns the pages
- */
-int gnttab_alloc_pages(int nr_pages, struct page **pages)
+int gnttab_pages_set_private(int nr_pages, struct page **pages)
  {
  	int i;
-	int ret;
-
-	ret = alloc_xenballooned_pages(nr_pages, pages);
-	if (ret < 0)
-		return ret;
 
  	for (i = 0; i < nr_pages; i++) {
  #if BITS_PER_LONG < 64
  		struct xen_page_foreign *foreign;
 

drivers/xen/grant-table.c void gnttab_free_auto_xlat_frames(void) · line 781

Quoted 4 lines

drivers/xen/grant-table.c +1 -2

  		foreign = kzalloc(sizeof(*foreign), GFP_KERNEL);
-		if (!foreign) {
-			gnttab_free_pages(nr_pages, pages);
+		if (!foreign)

Don't we have to free previously allocated "foreign"(s) if it fails in the middle (e.g. 0 < i && i < nr_pages - 1) before returning?

Quoted 68 lines

drivers/xen/grant-table.c +33 -6

  			return -ENOMEM;
-		}
+
  		set_page_private(pages[i], (unsigned long)foreign);
  #endif
  		SetPagePrivate(pages[i]);

@@ -799,14 +788,30 @@ int gnttab_alloc_pages(int nr_pages, struct page **pages)
 
  	return 0;
  }
-EXPORT_SYMBOL(gnttab_alloc_pages);
+EXPORT_SYMBOL(gnttab_pages_set_private);
 
  /**
- * gnttab_free_pages - free pages allocated by gnttab_alloc_pages()
- * @nr_pages; number of pages to free
- * @pages: the pages
+ * gnttab_alloc_pages - alloc pages suitable for grant mapping into
+ * @nr_pages: number of pages to alloc
+ * @pages: returns the pages
   */
-void gnttab_free_pages(int nr_pages, struct page **pages)
+int gnttab_alloc_pages(int nr_pages, struct page **pages)
+{
+	int ret;
+
+	ret = alloc_xenballooned_pages(nr_pages, pages);
+	if (ret < 0)
+		return ret;
+
+	ret = gnttab_pages_set_private(nr_pages, pages);
+	if (ret < 0)
+		gnttab_free_pages(nr_pages, pages);
+
+	return ret;
+}
+EXPORT_SYMBOL(gnttab_alloc_pages);
+
+void gnttab_pages_clear_private(int nr_pages, struct page **pages)
  {
  	int i;
 

@@ -818,6 +823,17 @@ void gnttab_free_pages(int nr_pages, struct page **pages)
  			ClearPagePrivate(pages[i]);
  		}
  	}
+}
+EXPORT_SYMBOL(gnttab_pages_clear_private);
+
+/**
+ * gnttab_free_pages - free pages allocated by gnttab_alloc_pages()
+ * @nr_pages; number of pages to free
+ * @pages: the pages
+ */
+void gnttab_free_pages(int nr_pages, struct page **pages)
+{
+	gnttab_pages_clear_private(nr_pages, pages);
  	free_xenballooned_pages(nr_pages, pages);
  }
  EXPORT_SYMBOL(gnttab_free_pages);

include/xen/grant_table.h +0 -0

index 2e37741f6b8d..de03f2542bb7 100644
@@ -198,6 +198,9 @@ void gnttab_free_auto_xlat_frames(void);
  int gnttab_alloc_pages(int nr_pages, struct page **pages);
  void gnttab_free_pages(int nr_pages, struct page **pages);
 

Quoted 9 lines

include/xen/grant_table.h +3 -0

+int gnttab_pages_set_private(int nr_pages, struct page **pages);
+void gnttab_pages_clear_private(int nr_pages, struct page **pages);
+
  int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
  		    struct gnttab_map_grant_ref *kmap_ops,
  		    struct page **pages, unsigned int count);

Signature · 1 line

2.17.0

From: Oleksandr Andrushchenko <[email protected]>

Make set/clear page private code shared and accessible to other kernel modules which can re-use these instead of open-coding.

Signed-off-by Oleksandr Andrushchenko <[email protected]>

---
 drivers/xen/grant-table.c | 54 +++++++++++++++++++++++++--------------
 include/xen/grant_table.h |  3 +++
 2 files changed, 38 insertions(+), 19 deletions(-)

drivers/xen/grant-table.c +1 -10

index 27be107d6480..d7488226e1f2 100644
@@ -769,29 +769,18 @@ void gnttab_free_auto_xlat_frames(void)
 }
 EXPORT_SYMBOL_GPL(gnttab_free_auto_xlat_frames);
 
-/**
- * gnttab_alloc_pages - alloc pages suitable for grant mapping into
- * @nr_pages: number of pages to alloc
- * @pages: returns the pages
- */
-int gnttab_alloc_pages(int nr_pages, struct page **pages)
+int gnttab_pages_set_private(int nr_pages, struct page **pages)
 {
 	int i;
-	int ret;
-
-	ret = alloc_xenballooned_pages(nr_pages, pages);
-	if (ret < 0)

drivers/xen/grant-table.c +33 -6

 			return -ENOMEM;
-		}
+
 		set_page_private(pages[i], (unsigned long)foreign);
 #endif
 		SetPagePrivate(pages[i]);

@@ -799,14 +788,30 @@ int gnttab_alloc_pages(int nr_pages, struct page **pages)
 
 	return 0;
 }
-EXPORT_SYMBOL(gnttab_alloc_pages);
+EXPORT_SYMBOL(gnttab_pages_set_private);
 
 /**
- * gnttab_free_pages - free pages allocated by gnttab_alloc_pages()
- * @nr_pages; number of pages to free
- * @pages: the pages
+ * gnttab_alloc_pages - alloc pages suitable for grant mapping into
+ * @nr_pages: number of pages to alloc
+ * @pages: returns the pages
  */
-void gnttab_free_pages(int nr_pages, struct page **pages)
+int gnttab_alloc_pages(int nr_pages, struct page **pages)
+{
+	int ret;
+
+	ret = alloc_xenballooned_pages(nr_pages, pages);
+	if (ret < 0)
+		return ret;
+
+	ret = gnttab_pages_set_private(nr_pages, pages);
+	if (ret < 0)
+		gnttab_free_pages(nr_pages, pages);
+
+	return ret;
+}
+EXPORT_SYMBOL(gnttab_alloc_pages);
+
+void gnttab_pages_clear_private(int nr_pages, struct page **pages)
 {
 	int i;
 

@@ -818,6 +823,17 @@ void gnttab_free_pages(int nr_pages, struct page **pages)
 			ClearPagePrivate(pages[i]);
 		}
 	}
+}
+EXPORT_SYMBOL(gnttab_pages_clear_private);
+
+/**
+ * gnttab_free_pages - free pages allocated by gnttab_alloc_pages()
+ * @nr_pages; number of pages to free
+ * @pages: the pages
+ */
+void gnttab_free_pages(int nr_pages, struct page **pages)
+{
+	gnttab_pages_clear_private(nr_pages, pages);
 	free_xenballooned_pages(nr_pages, pages);
 }
 EXPORT_SYMBOL(gnttab_free_pages);

include/xen/grant_table.h +3 -0

index 2e37741f6b8d..de03f2542bb7 100644
@@ -198,6 +198,9 @@ void gnttab_free_auto_xlat_frames(void);
 int gnttab_alloc_pages(int nr_pages, struct page **pages);
 void gnttab_free_pages(int nr_pages, struct page **pages);
 
+int gnttab_pages_set_private(int nr_pages, struct page **pages);
+void gnttab_pages_clear_private(int nr_pages, struct page **pages);
+
 int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
 		    struct gnttab_map_grant_ref *kmap_ops,
 		    struct page **pages, unsigned int count);

Signature · 1 line

2.17.0

From: Oleksandr Andrushchenko <[email protected]>

Memory {increase|decrease}_reservation and VA mappings update/reset code used in balloon driver can be made common, so other drivers can also re-use the same functionality without open-coding. Create a dedicated module

for the shared code and export corresponding
symbols for other kernel modules.

Signed-off-by Oleksandr Andrushchenko <[email protected]>

---
 drivers/xen/Makefile          |   1 +
 drivers/xen/balloon.c         |  71 ++----------------
 drivers/xen/mem-reservation.c | 134 ++++++++++++++++++++++++++++++++++
 include/xen/mem_reservation.h |  29 ++++++++
 4 files changed, 170 insertions(+), 65 deletions(-)
 create mode 100644 drivers/xen/mem-reservation.c
 create mode 100644 include/xen/mem_reservation.h

drivers/xen/Makefile +1 -0

index 451e833f5931..3c87b0c3aca6 100644
@@ -2,6 +2,7 @@
 obj-$(CONFIG_HOTPLUG_CPU)		+= cpu_hotplug.o
 obj-$(CONFIG_X86)			+= fallback.o
 obj-y	+= grant-table.o features.o balloon.o manage.o preempt.o time.o
+obj-y	+= mem-reservation.o
 obj-y	+= events/
 obj-y	+= xenbus/
 

drivers/xen/balloon.c +2 -29

index 065f0b607373..57b482d67a3a 100644
@@ -71,6 +71,7 @@
 #include <xen/balloon.h>
 #include <xen/features.h>
 #include <xen/page.h>
+#include <xen/mem_reservation.h>
 
 static int xen_hotplug_unpopulated;
 

@@ -157,13 +158,6 @@ static DECLARE_DELAYED_WORK(balloon_worker, balloon_process);
 #define GFP_BALLOON \
 	(GFP_HIGHUSER | __GFP_NOWARN | __GFP_NORETRY | __GFP_NOMEMALLOC)
 
-static void scrub_page(struct page *page)
-{
-#ifdef CONFIG_XEN_SCRUB_PAGES
-	clear_highpage(page);
-#endif
-}
-
 /* balloon_append: add the given page to the balloon. */
 static void __balloon_append(struct page *page)
 {

@@ -463,11 +457,6 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
 	int rc;
 	unsigned long i;
 	struct page   *page;
-	struct xen_memory_reservation reservation = {
-		.address_bits = 0,
-		.extent_order = EXTENT_ORDER,
-		.domid        = DOMID_SELF
-	};
 
 	if (nr_pages > ARRAY_SIZE(frame_list))
 		nr_pages = ARRAY_SIZE(frame_list);

@@ -486,9 +475,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
 		page = balloon_next_page(page);
 	}
 
-	set_xen_guest_handle(reservation.extent_start, frame_list);
-	reservation.nr_extents = nr_pages;
-	rc = HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation);
+	rc = xenmem_reservation_increase(nr_pages, frame_list);
 	if (rc <= 0)
 		return BP_EAGAIN;
 

@@ -496,29 +483,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
 		page = balloon_retrieve(false);
 		BUG_ON(page == NULL);
 
-#ifdef CONFIG_XEN_HAVE_PVMMU
-		/*
-		 * We don't support PV MMU when Linux and Xen is using
-		 * different page granularity.
-		 */
-		BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
-
-		if (!xen_feature(XENFEAT_auto_translated_physmap)) {
-			unsigned long pfn = page_to_pfn(page);
-
-			set_phys_to_machine(pfn, frame_list[i]);
-
-			/* Link back into the page tables if not highmem. */
-			if (!PageHighMem(page)) {

drivers/xen/balloon.c +1 -6

 		/* Relinquish the page back to the allocator. */
 		free_reserved_page(page);

@@ -535,11 +500,6 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
 	unsigned long i;
 	struct page *page, *tmp;
 	int ret;
-	struct xen_memory_reservation reservation = {
-		.address_bits = 0,
-		.extent_order = EXTENT_ORDER,
-		.domid        = DOMID_SELF
-	};
 	LIST_HEAD(pages);
 
 	if (nr_pages > ARRAY_SIZE(frame_list))

@@ -553,7 +513,7 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
 			break;
 		}
 		adjust_managed_page_count(page, -1);
-		scrub_page(page);
+		xenmem_reservation_scrub_page(page);
 		list_add(&page->lru, &pages);
 	}
 

@@ -575,25 +535,8 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
 		/* XENMEM_decrease_reservation requires a GFN */
 		frame_list[i++] = xen_page_to_gfn(page);
 

drivers/xen/balloon.c +1 -12

-			if (!PageHighMem(page)) {
-				ret = HYPERVISOR_update_va_mapping(
-						(unsigned long)__va(pfn << PAGE_SHIFT),
-						__pte_ma(0), 0);
-				BUG_ON(ret);
-			}
-			__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
-		}
-#endif
 		list_del(&page->lru);
 
 		balloon_append(page);

@@ -601,9 +544,7 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
 
 	flush_tlb_all();
 
-	set_xen_guest_handle(reservation.extent_start, frame_list);
-	reservation.nr_extents   = nr_pages;
-	ret = HYPERVISOR_memory_op(XENMEM_decrease_reservation, &reservation);
+	ret = xenmem_reservation_decrease(nr_pages, frame_list);
 	BUG_ON(ret != nr_pages);
 
 	balloon_stats.current_pages -= nr_pages;

drivers/xen/mem-reservation.c new file +0 -0

new file mode 100644
index 000000000000..29882e4324f5

drivers/xen/mem-reservation.c new file +133 -0

+
+/******************************************************************************
+ * Xen memory reservation utilities.
+ *
+ * Copyright (c) 2003, B Dragovic
+ * Copyright (c) 2003-2004, M Williamson, K Fraser
+ * Copyright (c) 2005 Dan M. Smith, IBM Corporation
+ * Copyright (c) 2010 Daniel Kiper
+ * Copyright (c) 2018, Oleksandr Andrushchenko, EPAM Systems Inc.
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+
+#include <asm/tlb.h>
+#include <asm/xen/hypercall.h>
+
+#include <xen/interface/memory.h>
+#include <xen/page.h>
+
+/*
+ * Use one extent per PAGE_SIZE to avoid to break down the page into
+ * multiple frame.
+ */
+#define EXTENT_ORDER (fls(XEN_PFN_PER_PAGE) - 1)
+
+void xenmem_reservation_scrub_page(struct page *page)
+{
+#ifdef CONFIG_XEN_SCRUB_PAGES
+	clear_highpage(page);
+#endif
+}
+EXPORT_SYMBOL(xenmem_reservation_scrub_page);
+
+void xenmem_reservation_va_mapping_update(unsigned long count,
+					  struct page **pages,
+					  xen_pfn_t *frames)
+{
+#ifdef CONFIG_XEN_HAVE_PVMMU
+	int i;
+
+	for (i = 0; i < count; i++) {
+		struct page *page;
+
+		page = pages[i];
+		BUG_ON(page == NULL);
+
+		/*
+		 * We don't support PV MMU when Linux and Xen is using
+		 * different page granularity.
+		 */
+		BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
+
+		if (!xen_feature(XENFEAT_auto_translated_physmap)) {
+			unsigned long pfn = page_to_pfn(page);
+
+			set_phys_to_machine(pfn, frames[i]);
+
+			/* Link back into the page tables if not highmem. */
+			if (!PageHighMem(page)) {
+				int ret;
+
+				ret = HYPERVISOR_update_va_mapping(
+						(unsigned long)__va(pfn << PAGE_SHIFT),
+						mfn_pte(frames[i], PAGE_KERNEL),
+						0);
+				BUG_ON(ret);
+			}
+		}
+	}
+#endif
+}
+EXPORT_SYMBOL(xenmem_reservation_va_mapping_update);
+
+void xenmem_reservation_va_mapping_reset(unsigned long count,
+					 struct page **pages)
+{
+#ifdef CONFIG_XEN_HAVE_PVMMU
+	int i;
+
+	for (i = 0; i < count; i++) {
+		/*
+		 * We don't support PV MMU when Linux and Xen is using
+		 * different page granularity.
+		 */
+		BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
+
+		if (!xen_feature(XENFEAT_auto_translated_physmap)) {
+			struct page *page = pages[i];
+			unsigned long pfn = page_to_pfn(page);
+
+			if (!PageHighMem(page)) {
+				int ret;
+
+				ret = HYPERVISOR_update_va_mapping(
+						(unsigned long)__va(pfn << PAGE_SHIFT),
+						__pte_ma(0), 0);
+				BUG_ON(ret);
+			}
+			__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
+		}
+	}
+#endif
+}
+EXPORT_SYMBOL(xenmem_reservation_va_mapping_reset);
+
+int xenmem_reservation_increase(int count, xen_pfn_t *frames)
+{
+	struct xen_memory_reservation reservation = {
+		.address_bits = 0,
+		.extent_order = EXTENT_ORDER,
+		.domid        = DOMID_SELF
+	};
+
+	set_xen_guest_handle(reservation.extent_start, frames);
+	reservation.nr_extents = count;
+	return HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation);
+}
+EXPORT_SYMBOL(xenmem_reservation_increase);
+
+int xenmem_reservation_decrease(int count, xen_pfn_t *frames)
+{
+	struct xen_memory_reservation reservation = {
+		.address_bits = 0,
+		.extent_order = EXTENT_ORDER,
+		.domid        = DOMID_SELF
+	};
+
+	set_xen_guest_handle(reservation.extent_start, frames);
+	reservation.nr_extents = count;
+	return HYPERVISOR_memory_op(XENMEM_decrease_reservation, &reservation);
+}
+EXPORT_SYMBOL(xenmem_reservation_decrease);

include/xen/mem_reservation.h new file +0 -0

new file mode 100644
index 000000000000..9306d9b8743c

include/xen/mem_reservation.h new file +28 -0

+
+/*
+ * Xen memory reservation utilities.
+ *
+ * Copyright (c) 2003, B Dragovic
+ * Copyright (c) 2003-2004, M Williamson, K Fraser
+ * Copyright (c) 2005 Dan M. Smith, IBM Corporation
+ * Copyright (c) 2010 Daniel Kiper
+ * Copyright (c) 2018, Oleksandr Andrushchenko, EPAM Systems Inc.
+ */
+
+#ifndef _XENMEM_RESERVATION_H
+#define _XENMEM_RESERVATION_H
+
+void xenmem_reservation_scrub_page(struct page *page);
+
+void xenmem_reservation_va_mapping_update(unsigned long count,
+					  struct page **pages,
+					  xen_pfn_t *frames);
+
+void xenmem_reservation_va_mapping_reset(unsigned long count,
+					 struct page **pages);
+
+int xenmem_reservation_increase(int count, xen_pfn_t *frames);
+
+int xenmem_reservation_decrease(int count, xen_pfn_t *frames);
+
+#endif

Quoted 6 lines · Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <[email protected]>

Memory {increase|decrease}_reservation and VA mappings update/reset code used in balloon driver can be made common, so other drivers can also re-use the same functionality without open-coding. Create a dedicated module

IIUIC this is not really a module, it's a common file.

Quoted 100 lines

for the shared code and export corresponding
symbols for other kernel modules.

Signed-off-by Oleksandr Andrushchenko <[email protected]>

---
  drivers/xen/Makefile          |   1 +
  drivers/xen/balloon.c         |  71 ++----------------
  drivers/xen/mem-reservation.c | 134 ++++++++++++++++++++++++++++++++++
  include/xen/mem_reservation.h |  29 ++++++++
  4 files changed, 170 insertions(+), 65 deletions(-)

create mode 100644 drivers/xen/mem-reservation.c create mode 100644 include/xen/mem_reservation.h

drivers/xen/Makefile +1 -0

index 451e833f5931..3c87b0c3aca6 100644
@@ -2,6 +2,7 @@
  obj-$(CONFIG_HOTPLUG_CPU)		+= cpu_hotplug.o
  obj-$(CONFIG_X86)			+= fallback.o
  obj-y	+= grant-table.o features.o balloon.o manage.o preempt.o time.o
+obj-y	+= mem-reservation.o
  obj-y	+= events/
  obj-y	+= xenbus/
 

drivers/xen/balloon.c +2 -36

index 065f0b607373..57b482d67a3a 100644
@@ -71,6 +71,7 @@
  #include <xen/balloon.h>
  #include <xen/features.h>
  #include <xen/page.h>
+#include <xen/mem_reservation.h>
 
  static int xen_hotplug_unpopulated;
 

@@ -157,13 +158,6 @@ static DECLARE_DELAYED_WORK(balloon_worker, balloon_process);
  #define GFP_BALLOON \
  	(GFP_HIGHUSER | __GFP_NOWARN | __GFP_NORETRY | __GFP_NOMEMALLOC)
 
-static void scrub_page(struct page *page)
-{
-#ifdef CONFIG_XEN_SCRUB_PAGES
-	clear_highpage(page);
-#endif
-}
-
  /* balloon_append: add the given page to the balloon. */
  static void __balloon_append(struct page *page)
  {

@@ -463,11 +457,6 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
  	int rc;
  	unsigned long i;
  	struct page   *page;
-	struct xen_memory_reservation reservation = {
-		.address_bits = 0,
-		.extent_order = EXTENT_ORDER,
-		.domid        = DOMID_SELF
-	};
 
  	if (nr_pages > ARRAY_SIZE(frame_list))
  		nr_pages = ARRAY_SIZE(frame_list);

@@ -486,9 +475,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
  		page = balloon_next_page(page);
  	}
 
-	set_xen_guest_handle(reservation.extent_start, frame_list);
-	reservation.nr_extents = nr_pages;
-	rc = HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation);
+	rc = xenmem_reservation_increase(nr_pages, frame_list);
  	if (rc <= 0)
  		return BP_EAGAIN;
 

@@ -496,29 +483,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
  		page = balloon_retrieve(false);
  		BUG_ON(page == NULL);
 
-#ifdef CONFIG_XEN_HAVE_PVMMU
-		/*
-		 * We don't support PV MMU when Linux and Xen is using
-		 * different page granularity.
-		 */
-		BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
-
-		if (!xen_feature(XENFEAT_auto_translated_physmap)) {
-			unsigned long pfn = page_to_pfn(page);
-
-			set_phys_to_machine(pfn, frame_list[i]);
-
-			/* Link back into the page tables if not highmem. */
-			if (!PageHighMem(page)) {
-				int ret;
-				ret = HYPERVISOR_update_va_mapping(
-						(unsigned long)__va(pfn << PAGE_SHIFT),
-						mfn_pte(frame_list[i], PAGE_KERNEL),
-						0);
-				BUG_ON(ret);
-			}

drivers/xen/balloon.c static enum bp_state increase_reservation(unsigned long nr_pages) · line 486

Quoted 3 lines

drivers/xen/balloon.c +1 -2

-		}
-#endif
+		xenmem_reservation_va_mapping_update(1, &page, &frame_list[i]);

Can you make a single call to xenmem_reservation_va_mapping_update(rc, ...)? You need to keep track of pages but presumable they can be put into an array (or a list). In fact, perhaps we can have balloon_retrieve() return a set of pages.

Quoted 31 lines

drivers/xen/balloon.c +1 -6

  		/* Relinquish the page back to the allocator. */
  		free_reserved_page(page);

@@ -535,11 +500,6 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
  	unsigned long i;
  	struct page *page, *tmp;
  	int ret;
-	struct xen_memory_reservation reservation = {
-		.address_bits = 0,
-		.extent_order = EXTENT_ORDER,
-		.domid        = DOMID_SELF
-	};
  	LIST_HEAD(pages);
 
  	if (nr_pages > ARRAY_SIZE(frame_list))

@@ -553,7 +513,7 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
  			break;
  		}
  		adjust_managed_page_count(page, -1);
-		scrub_page(page);
+		xenmem_reservation_scrub_page(page);
  		list_add(&page->lru, &pages);
  	}
 

@@ -575,25 +535,8 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
  		/* XENMEM_decrease_reservation requires a GFN */
  		frame_list[i++] = xen_page_to_gfn(page);
 

drivers/xen/balloon.c static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) · line 538

Quoted 10 lines

drivers/xen/balloon.c +1 -9

-#ifdef CONFIG_XEN_HAVE_PVMMU
-		/*
-		 * We don't support PV MMU when Linux and Xen is using
-		 * different page granularity.
-		 */
-		BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
-
-		if (!xen_feature(XENFEAT_auto_translated_physmap)) {
-			unsigned long pfn = page_to_pfn(page);
+		xenmem_reservation_va_mapping_reset(1, &page);

and here too.

Quoted 24 lines

drivers/xen/balloon.c +1 -12

-			if (!PageHighMem(page)) {
-				ret = HYPERVISOR_update_va_mapping(
-						(unsigned long)__va(pfn << PAGE_SHIFT),
-						__pte_ma(0), 0);
-				BUG_ON(ret);
-			}
-			__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
-		}
-#endif
  		list_del(&page->lru);
 
  		balloon_append(page);

@@ -601,9 +544,7 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
 
  	flush_tlb_all();
 
-	set_xen_guest_handle(reservation.extent_start, frame_list);
-	reservation.nr_extents   = nr_pages;
-	ret = HYPERVISOR_memory_op(XENMEM_decrease_reservation, &reservation);
+	ret = xenmem_reservation_decrease(nr_pages, frame_list);
  	BUG_ON(ret != nr_pages);
 

drivers/xen/mem-reservation.c line 1

Quoted 8 lines

drivers/xen/balloon.c +0 -0

  	balloon_stats.current_pages -= nr_pages;

drivers/xen/mem-reservation.c new file +1 -0

new file mode 100644
index 000000000000..29882e4324f5
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT

Why is this "OR MIT"? The original file was licensed GPLv2 only.

Quoted 134 lines

drivers/xen/mem-reservation.c new file +133 -0

+
+/******************************************************************************
+ * Xen memory reservation utilities.
+ *
+ * Copyright (c) 2003, B Dragovic
+ * Copyright (c) 2003-2004, M Williamson, K Fraser
+ * Copyright (c) 2005 Dan M. Smith, IBM Corporation
+ * Copyright (c) 2010 Daniel Kiper
+ * Copyright (c) 2018, Oleksandr Andrushchenko, EPAM Systems Inc.
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+
+#include <asm/tlb.h>
+#include <asm/xen/hypercall.h>
+
+#include <xen/interface/memory.h>
+#include <xen/page.h>
+
+/*
+ * Use one extent per PAGE_SIZE to avoid to break down the page into
+ * multiple frame.
+ */
+#define EXTENT_ORDER (fls(XEN_PFN_PER_PAGE) - 1)
+
+void xenmem_reservation_scrub_page(struct page *page)
+{
+#ifdef CONFIG_XEN_SCRUB_PAGES
+	clear_highpage(page);
+#endif
+}
+EXPORT_SYMBOL(xenmem_reservation_scrub_page);
+
+void xenmem_reservation_va_mapping_update(unsigned long count,
+					  struct page **pages,
+					  xen_pfn_t *frames)
+{
+#ifdef CONFIG_XEN_HAVE_PVMMU
+	int i;
+
+	for (i = 0; i < count; i++) {
+		struct page *page;
+
+		page = pages[i];
+		BUG_ON(page == NULL);
+
+		/*
+		 * We don't support PV MMU when Linux and Xen is using
+		 * different page granularity.
+		 */
+		BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
+
+		if (!xen_feature(XENFEAT_auto_translated_physmap)) {
+			unsigned long pfn = page_to_pfn(page);
+
+			set_phys_to_machine(pfn, frames[i]);
+
+			/* Link back into the page tables if not highmem. */
+			if (!PageHighMem(page)) {
+				int ret;
+
+				ret = HYPERVISOR_update_va_mapping(
+						(unsigned long)__va(pfn << PAGE_SHIFT),
+						mfn_pte(frames[i], PAGE_KERNEL),
+						0);
+				BUG_ON(ret);
+			}
+		}
+	}
+#endif
+}
+EXPORT_SYMBOL(xenmem_reservation_va_mapping_update);
+
+void xenmem_reservation_va_mapping_reset(unsigned long count,
+					 struct page **pages)
+{
+#ifdef CONFIG_XEN_HAVE_PVMMU
+	int i;
+
+	for (i = 0; i < count; i++) {
+		/*
+		 * We don't support PV MMU when Linux and Xen is using
+		 * different page granularity.
+		 */
+		BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
+
+		if (!xen_feature(XENFEAT_auto_translated_physmap)) {
+			struct page *page = pages[i];
+			unsigned long pfn = page_to_pfn(page);
+
+			if (!PageHighMem(page)) {
+				int ret;
+
+				ret = HYPERVISOR_update_va_mapping(
+						(unsigned long)__va(pfn << PAGE_SHIFT),
+						__pte_ma(0), 0);
+				BUG_ON(ret);
+			}
+			__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
+		}
+	}
+#endif
+}
+EXPORT_SYMBOL(xenmem_reservation_va_mapping_reset);
+
+int xenmem_reservation_increase(int count, xen_pfn_t *frames)
+{
+	struct xen_memory_reservation reservation = {
+		.address_bits = 0,
+		.extent_order = EXTENT_ORDER,
+		.domid        = DOMID_SELF
+	};
+
+	set_xen_guest_handle(reservation.extent_start, frames);
+	reservation.nr_extents = count;
+	return HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation);
+}
+EXPORT_SYMBOL(xenmem_reservation_increase);
+
+int xenmem_reservation_decrease(int count, xen_pfn_t *frames)
+{
+	struct xen_memory_reservation reservation = {
+		.address_bits = 0,
+		.extent_order = EXTENT_ORDER,
+		.domid        = DOMID_SELF
+	};
+
+	set_xen_guest_handle(reservation.extent_start, frames);
+	reservation.nr_extents = count;
+	return HYPERVISOR_memory_op(XENMEM_decrease_reservation, &reservation);
+}
+EXPORT_SYMBOL(xenmem_reservation_decrease);

include/xen/mem_reservation.h line 1

Quoted 7 lines

include/xen/mem_reservation.h new file +1 -0

new file mode 100644
index 000000000000..9306d9b8743c
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 OR MIT */

and here too.

-boris

Quoted 20 lines

include/xen/mem_reservation.h new file +20 -0

+
+/*
+ * Xen memory reservation utilities.
+ *
+ * Copyright (c) 2003, B Dragovic
+ * Copyright (c) 2003-2004, M Williamson, K Fraser
+ * Copyright (c) 2005 Dan M. Smith, IBM Corporation
+ * Copyright (c) 2010 Daniel Kiper
+ * Copyright (c) 2018, Oleksandr Andrushchenko, EPAM Systems Inc.
+ */
+
+#ifndef _XENMEM_RESERVATION_H
+#define _XENMEM_RESERVATION_H
+
+void xenmem_reservation_scrub_page(struct page *page);
+
+void xenmem_reservation_va_mapping_update(unsigned long count,
+					  struct page **pages,
+					  xen_pfn_t *frames);
+

include/xen/mem_reservation.h line 29

Quoted 8 lines

include/xen/mem_reservation.h new file +8 -0

+void xenmem_reservation_va_mapping_reset(unsigned long count,
+					 struct page **pages);
+
+int xenmem_reservation_increase(int count, xen_pfn_t *frames);
+
+int xenmem_reservation_decrease(int count, xen_pfn_t *frames);
+
+#endif

Quoted 53 lines · Boris Ostrovsky

On 05/25/2018 11:33 AM, Oleksandr Andrushchenko wrote:

drivers/xen/balloon.c +1 -7

@@ -463,11 +457,6 @@ static enum bp_state
 increase_reservation(unsigned long nr_pages)
       int rc;
       unsigned long i;
       struct page   *page;
-    struct xen_memory_reservation reservation = {
-        .address_bits = 0,
-        .extent_order = EXTENT_ORDER,
-        .domid        = DOMID_SELF
-    };
         if (nr_pages > ARRAY_SIZE(frame_list))
           nr_pages = ARRAY_SIZE(frame_list);

@@ -486,9 +475,7 @@ static enum bp_state
 increase_reservation(unsigned long nr_pages)
           page = balloon_next_page(page);
       }
   -    set_xen_guest_handle(reservation.extent_start, frame_list);
-    reservation.nr_extents = nr_pages;
-    rc = HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation);
+    rc = xenmem_reservation_increase(nr_pages, frame_list);
       if (rc <= 0)
           return BP_EAGAIN;

  @@ -496,29 +483,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
          page = balloon_retrieve(false);
          BUG_ON(page == NULL);
  -#ifdef CONFIG_XEN_HAVE_PVMMU
-        /*
-         * We don't support PV MMU when Linux and Xen is using -         * different page granularity.
-         */
-        BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);

drivers/xen/balloon.c +1 -17

-
-        if (!xen_feature(XENFEAT_auto_translated_physmap)) {
-            unsigned long pfn = page_to_pfn(page);
-
-            set_phys_to_machine(pfn, frame_list[i]);
-
-            /* Link back into the page tables if not highmem. */
-            if (!PageHighMem(page)) {
-                int ret;
-                ret = HYPERVISOR_update_va_mapping(
-                        (unsigned long)__va(pfn << PAGE_SHIFT),
-                        mfn_pte(frame_list[i], PAGE_KERNEL),
-                        0);
-                BUG_ON(ret);
-            }
-        }
-#endif
+        xenmem_reservation_va_mapping_update(1, &page, &frame_list[i]);

Quoted 4 lines

Can you make a single call to xenmem_reservation_va_mapping_update(rc, ...)? You need to keep track of pages but presumable they can be put into an array (or a list). In fact, perhaps we can have balloon_retrieve() return a set of pages.

This is actually how it is used later on for dma-buf, but I just didn't want
to alter original balloon code too much, but this can be done, in order of simplicity:

1. Similar to frame_list, e.g. static array of struct page* of size ARRAY_SIZE(frame_list):
more static memory is used, but no allocations

2. Allocated at run-time with kcalloc: allocation can fail

3. Make balloon_retrieve() return a set of pages: will require list/array allocation
and handling, allocation may fail, balloon_retrieve prototype change

Could you please tell which of the above will fit better?

Quoted 2 lines

Quoted 62 lines · Oleksandr Andrushchenko

Quoted 52 lines · Boris Ostrovsky

On 05/25/2018 11:33 AM, Oleksandr Andrushchenko wrote:

drivers/xen/balloon.c +1 -7

@@ -463,11 +457,6 @@ static enum bp_state
 increase_reservation(unsigned long nr_pages)
       int rc;
       unsigned long i;
       struct page   *page;
-    struct xen_memory_reservation reservation = {
-        .address_bits = 0,
-        .extent_order = EXTENT_ORDER,
-        .domid        = DOMID_SELF
-    };
         if (nr_pages > ARRAY_SIZE(frame_list))
           nr_pages = ARRAY_SIZE(frame_list);

@@ -486,9 +475,7 @@ static enum bp_state
 increase_reservation(unsigned long nr_pages)
           page = balloon_next_page(page);
       }
   -    set_xen_guest_handle(reservation.extent_start, frame_list);
-    reservation.nr_extents = nr_pages;
-    rc = HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation);
+    rc = xenmem_reservation_increase(nr_pages, frame_list);
       if (rc <= 0)
           return BP_EAGAIN;
   @@ -496,29 +483,7 @@ static enum bp_state

increase_reservation(unsigned long nr_pages)
          page = balloon_retrieve(false); BUG_ON(page == NULL);
  -#ifdef CONFIG_XEN_HAVE_PVMMU
-        /*
-         * We don't support PV MMU when Linux and Xen is using -         * different page granularity.
-         */
-        BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);

drivers/xen/balloon.c +1 -17

-
-        if (!xen_feature(XENFEAT_auto_translated_physmap)) {
-            unsigned long pfn = page_to_pfn(page);
-
-            set_phys_to_machine(pfn, frame_list[i]);
-
-            /* Link back into the page tables if not highmem. */
-            if (!PageHighMem(page)) {
-                int ret;
-                ret = HYPERVISOR_update_va_mapping(
-                        (unsigned long)__va(pfn << PAGE_SHIFT),
-                        mfn_pte(frame_list[i], PAGE_KERNEL),
-                        0);
-                BUG_ON(ret);
-            }
-        }
-#endif
+        xenmem_reservation_va_mapping_update(1, &page, &frame_list[i]);

Quoted 4 lines

Can you make a single call to xenmem_reservation_va_mapping_update(rc, ...)? You need to keep track of pages but presumable they can be put into an array (or a list). In fact, perhaps we can have balloon_retrieve() return a set of pages.

This is actually how it is used later on for dma-buf, but I just didn't want
to alter original balloon code too much, but this can be done, in order of simplicity:

Quoted 5 lines

1. Similar to frame_list, e.g. static array of struct page* of size ARRAY_SIZE(frame_list):
more static memory is used, but no allocations

2. Allocated at run-time with kcalloc: allocation can fail

If this is called in freeing DMA buffer code path or in error path then we shouldn't do it.

Quoted 3 lines

3. Make balloon_retrieve() return a set of pages: will require list/array allocation
and handling, allocation may fail, balloon_retrieve prototype change

balloon pages are strung on the lru list. Can we keep have balloon_retrieve return a list of pages on that list?

-boris

Quoted 4 lines

Could you please tell which of the above will fit better?

Quoted 1 line

Quoted 74 lines · Boris Ostrovsky

Quoted 63 lines · Oleksandr Andrushchenko

Quoted 53 lines · Boris Ostrovsky

On 05/25/2018 11:33 AM, Oleksandr Andrushchenko wrote:

drivers/xen/balloon.c +1 -7

@@ -463,11 +457,6 @@ static enum bp_state
 increase_reservation(unsigned long nr_pages)
        int rc;
        unsigned long i;
        struct page   *page;
-    struct xen_memory_reservation reservation = {
-        .address_bits = 0,
-        .extent_order = EXTENT_ORDER,
-        .domid        = DOMID_SELF
-    };
          if (nr_pages > ARRAY_SIZE(frame_list))
            nr_pages = ARRAY_SIZE(frame_list);

@@ -486,9 +475,7 @@ static enum bp_state
 increase_reservation(unsigned long nr_pages)
            page = balloon_next_page(page);
        }
    -    set_xen_guest_handle(reservation.extent_start, frame_list);
-    reservation.nr_extents = nr_pages;
-    rc = HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation);
+    rc = xenmem_reservation_increase(nr_pages, frame_list);
        if (rc <= 0)
            return BP_EAGAIN;

   @@ -496,29 +483,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
           page = balloon_retrieve(false); BUG_ON(page == NULL);
   -#ifdef CONFIG_XEN_HAVE_PVMMU
-        /*
-         * We don't support PV MMU when Linux and Xen is using -         * different page granularity.
-         */
-        BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);

drivers/xen/balloon.c +1 -17

-
-        if (!xen_feature(XENFEAT_auto_translated_physmap)) {
-            unsigned long pfn = page_to_pfn(page);
-
-            set_phys_to_machine(pfn, frame_list[i]);
-
-            /* Link back into the page tables if not highmem. */
-            if (!PageHighMem(page)) {
-                int ret;
-                ret = HYPERVISOR_update_va_mapping(
-                        (unsigned long)__va(pfn << PAGE_SHIFT),
-                        mfn_pte(frame_list[i], PAGE_KERNEL),
-                        0);
-                BUG_ON(ret);
-            }
-        }
-#endif
+        xenmem_reservation_va_mapping_update(1, &page,

&frame_list[i]);

Quoted 4 lines

Can you make a single call to xenmem_reservation_va_mapping_update(rc, ...)? You need to keep track of pages but presumable they can be put into an array (or a list). In fact, perhaps we can have balloon_retrieve() return a set of pages.

This is actually how it is used later on for dma-buf, but I just didn't want
to alter original balloon code too much, but this can be done, in order of simplicity:

Quoted 5 lines

1. Similar to frame_list, e.g. static array of struct page* of size ARRAY_SIZE(frame_list):
more static memory is used, but no allocations

2. Allocated at run-time with kcalloc: allocation can fail

If this is called in freeing DMA buffer code path or in error path then we shouldn't do it.

Quoted 6 lines

Quoted 3 lines

3. Make balloon_retrieve() return a set of pages: will require list/array allocation
and handling, allocation may fail, balloon_retrieve prototype change

balloon pages are strung on the lru list. Can we keep have balloon_retrieve return a list of pages on that list?

First of all, before we go deep in details, I will highlight the goal of the requested change: for balloon driver we call xenmem_reservation_va_mapping_update(*1*, &page, &frame_list[i]); from increase_reservation
and
xenmem_reservation_va_mapping_reset(*1*, &page);
from decrease_reservation and it seems to be not elegant because of that one page/frame passed while we might have multiple pages/frames passed at once.

In the balloon driver the producer of pages for increase_reservation is balloon_retrieve(false) and for decrease_reservation it is alloc_page(gfp).
In case of decrease_reservation the page is added on the list:
LIST_HEAD(pages);
[...]
list_add(&page->lru, &pages);

and in case of increase_reservation it is retrieved page by page and can be put on a list as well with the same code from decrease_reservation, e.g.
LIST_HEAD(pages);
[...]
list_add(&page->lru, &pages);

Thus, both decrease_reservation and increase_reservation may hold their pages on a list before calling xenmem_reservation_va_mapping_{update|reset}.

For that we need a prototype change:
xenmem_reservation_va_mapping_reset(<nr_pages>, <list of pages>); But for xenmem_reservation_va_mapping_update it will look like:
xenmem_reservation_va_mapping_update(<nr_pages>, <list of pages>, <array of frames>)
which seems to be inconsistent. Converting entries of the static frame_list array
into corresponding list doesn't seem to be cute as well.

For dma-buf use-case arrays are more preferable as dma-buf constructs scatter-gather
tables from array of pages etc. and if page list is passed then it needs to be
converted into page array anyways.

So, we can:
1. Keep the prototypes as is, e.g. accept array of pages and use nr_pages == 1 in
case of balloon driver (existing code)
2. Statically allocate struct page* array in the balloon driver and fill it with pages
when those pages are retrieved:
static struct page *page_list[ARRAY_SIZE(frame_list)]; which will take additional 8KiB of space on 64-bit platform, but simplify things a lot.
3. Allocate struct page *page_list[ARRAY_SIZE(frame_list)] dynamically

As to Boris' suggestion "balloon pages are strung on the lru list. Can we keep have
balloon_retrieve return a list of pages on that list?" Because of alloc_xenballooned_pages' retry logic for page retireval, e.g. while (pgno < nr_pages) {
        page = balloon_retrieve(true);
        if (page) {
[...]
        } else {
            ret = add_ballooned_pages(nr_pages - pgno);
[...]
    }
I wouldn't change things that much.

IMO, we can keep 1 page based API with the only overhead for balloon driver of
function calls to xenmem_reservation_va_mapping_{update|reset} for each page.

Quoted 74 lines · Boris Ostrovsky

Quoted 63 lines · Oleksandr Andrushchenko

Quoted 53 lines · Boris Ostrovsky

On 05/25/2018 11:33 AM, Oleksandr Andrushchenko wrote:

drivers/xen/balloon.c +1 -7

@@ -463,11 +457,6 @@ static enum bp_state
 increase_reservation(unsigned long nr_pages)
        int rc;
        unsigned long i;
        struct page   *page;
-    struct xen_memory_reservation reservation = {
-        .address_bits = 0,
-        .extent_order = EXTENT_ORDER,
-        .domid        = DOMID_SELF
-    };
          if (nr_pages > ARRAY_SIZE(frame_list))
            nr_pages = ARRAY_SIZE(frame_list);

@@ -486,9 +475,7 @@ static enum bp_state
 increase_reservation(unsigned long nr_pages)
            page = balloon_next_page(page);
        }
    -    set_xen_guest_handle(reservation.extent_start, frame_list);
-    reservation.nr_extents = nr_pages;
-    rc = HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation);
+    rc = xenmem_reservation_increase(nr_pages, frame_list);
        if (rc <= 0)
            return BP_EAGAIN;

   @@ -496,29 +483,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
           page = balloon_retrieve(false); BUG_ON(page == NULL);
   -#ifdef CONFIG_XEN_HAVE_PVMMU
-        /*
-         * We don't support PV MMU when Linux and Xen is using -         * different page granularity.
-         */
-        BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);

drivers/xen/balloon.c +1 -17

-
-        if (!xen_feature(XENFEAT_auto_translated_physmap)) {
-            unsigned long pfn = page_to_pfn(page);
-
-            set_phys_to_machine(pfn, frame_list[i]);
-
-            /* Link back into the page tables if not highmem. */
-            if (!PageHighMem(page)) {
-                int ret;
-                ret = HYPERVISOR_update_va_mapping(
-                        (unsigned long)__va(pfn << PAGE_SHIFT),
-                        mfn_pte(frame_list[i], PAGE_KERNEL),
-                        0);
-                BUG_ON(ret);
-            }
-        }
-#endif
+        xenmem_reservation_va_mapping_update(1, &page,

&frame_list[i]);

Quoted 4 lines

Can you make a single call to xenmem_reservation_va_mapping_update(rc, ...)? You need to keep track of pages but presumable they can be put into an array (or a list). In fact, perhaps we can have balloon_retrieve() return a set of pages.

This is actually how it is used later on for dma-buf, but I just didn't want
to alter original balloon code too much, but this can be done, in order of simplicity:

Quoted 5 lines

1. Similar to frame_list, e.g. static array of struct page* of size ARRAY_SIZE(frame_list):
more static memory is used, but no allocations

2. Allocated at run-time with kcalloc: allocation can fail

If this is called in freeing DMA buffer code path or in error path then we shouldn't do it.

Quoted 6 lines

Quoted 3 lines

3. Make balloon_retrieve() return a set of pages: will require list/array allocation
and handling, allocation may fail, balloon_retrieve prototype change

balloon pages are strung on the lru list. Can we keep have balloon_retrieve return a list of pages on that list?

First of all, before we go deep in details, I will highlight the goal of the requested change: for balloon driver we call xenmem_reservation_va_mapping_update(*1*, &page, &frame_list[i]); from increase_reservation
and
xenmem_reservation_va_mapping_reset(*1*, &page);
from decrease_reservation and it seems to be not elegant because of that one page/frame passed while we might have multiple pages/frames passed at once.

In the balloon driver the producer of pages for increase_reservation is balloon_retrieve(false) and for decrease_reservation it is alloc_page(gfp).
In case of decrease_reservation the page is added on the list:
LIST_HEAD(pages);
[...]
list_add(&page->lru, &pages);

and in case of increase_reservation it is retrieved page by page and can be put on a list as well with the same code from decrease_reservation, e.g.
LIST_HEAD(pages);
[...]
list_add(&page->lru, &pages);

Thus, both decrease_reservation and increase_reservation may hold their pages on a list before calling xenmem_reservation_va_mapping_{update|reset}.

For that we need a prototype change:
xenmem_reservation_va_mapping_reset(<nr_pages>, <list of pages>); But for xenmem_reservation_va_mapping_update it will look like:
xenmem_reservation_va_mapping_update(<nr_pages>, <list of pages>, <array of frames>)
which seems to be inconsistent. Converting entries of the static frame_list array
into corresponding list doesn't seem to be cute as well.

For dma-buf use-case arrays are more preferable as dma-buf constructs scatter-gather
tables from array of pages etc. and if page list is passed then it needs to be
converted into page array anyways.

So, we can:
1. Keep the prototypes as is, e.g. accept array of pages and use nr_pages == 1 in
case of balloon driver (existing code)
2. Statically allocate struct page* array in the balloon driver and fill it with pages
when those pages are retrieved:
static struct page *page_list[ARRAY_SIZE(frame_list)]; which will take additional 8KiB of space on 64-bit platform, but simplify things a lot.
3. Allocate struct page *page_list[ARRAY_SIZE(frame_list)] dynamically

As to Boris' suggestion "balloon pages are strung on the lru list. Can we keep have
balloon_retrieve return a list of pages on that list?" Because of alloc_xenballooned_pages' retry logic for page retireval, e.g. while (pgno < nr_pages) {
        page = balloon_retrieve(true);
        if (page) {
[...]
        } else {
            ret = add_ballooned_pages(nr_pages - pgno);
[...]
    }
I wouldn't change things that much.

IMO, we can keep 1 page based API with the only overhead for balloon driver of
function calls to xenmem_reservation_va_mapping_{update|reset} for each page.

Quoted 149 lines · Oleksandr Andrushchenko

Quoted 73 lines · Boris Ostrovsky

Quoted 63 lines · Oleksandr Andrushchenko

Quoted 53 lines · Boris Ostrovsky

On 05/25/2018 11:33 AM, Oleksandr Andrushchenko wrote:

drivers/xen/balloon.c +1 -7

@@ -463,11 +457,6 @@ static enum bp_state
 increase_reservation(unsigned long nr_pages)
        int rc;
        unsigned long i;
        struct page   *page;
-    struct xen_memory_reservation reservation = {
-        .address_bits = 0,
-        .extent_order = EXTENT_ORDER,
-        .domid        = DOMID_SELF
-    };
          if (nr_pages > ARRAY_SIZE(frame_list))
            nr_pages = ARRAY_SIZE(frame_list);

@@ -486,9 +475,7 @@ static enum bp_state
 increase_reservation(unsigned long nr_pages)
            page = balloon_next_page(page);
        }
    -    set_xen_guest_handle(reservation.extent_start, frame_list);
-    reservation.nr_extents = nr_pages;
-    rc = HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation);
+    rc = xenmem_reservation_increase(nr_pages, frame_list);
        if (rc <= 0)
            return BP_EAGAIN;
    @@ -496,29 +483,7 @@ static enum bp_state

increase_reservation(unsigned long nr_pages)
           page = balloon_retrieve(false); BUG_ON(page == NULL);
   -#ifdef CONFIG_XEN_HAVE_PVMMU
-        /*
-         * We don't support PV MMU when Linux and Xen is using -         * different page granularity.
-         */
-        BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);

drivers/xen/balloon.c +1 -17

-
-        if (!xen_feature(XENFEAT_auto_translated_physmap)) {
-            unsigned long pfn = page_to_pfn(page);
-
-            set_phys_to_machine(pfn, frame_list[i]);
-
-            /* Link back into the page tables if not highmem. */
-            if (!PageHighMem(page)) {
-                int ret;
-                ret = HYPERVISOR_update_va_mapping(
-                        (unsigned long)__va(pfn << PAGE_SHIFT),
-                        mfn_pte(frame_list[i], PAGE_KERNEL),
-                        0);
-                BUG_ON(ret);
-            }
-        }
-#endif
+        xenmem_reservation_va_mapping_update(1, &page,

&frame_list[i]);

Quoted 4 lines

Can you make a single call to xenmem_reservation_va_mapping_update(rc, ...)? You need to keep track of pages but presumable they can be put into an array (or a list). In fact, perhaps we can have balloon_retrieve() return a set of pages.

This is actually how it is used later on for dma-buf, but I just didn't want
to alter original balloon code too much, but this can be done, in order of simplicity:

Quoted 5 lines

1. Similar to frame_list, e.g. static array of struct page* of size ARRAY_SIZE(frame_list):
more static memory is used, but no allocations

2. Allocated at run-time with kcalloc: allocation can fail

If this is called in freeing DMA buffer code path or in error path then we shouldn't do it.

Quoted 5 lines

Quoted 3 lines

3. Make balloon_retrieve() return a set of pages: will require list/array allocation
and handling, allocation may fail, balloon_retrieve prototype change

balloon pages are strung on the lru list. Can we keep have balloon_retrieve return a list of pages on that list?

First of all, before we go deep in details, I will highlight the goal of the requested change: for balloon driver we call xenmem_reservation_va_mapping_update(*1*, &page, &frame_list[i]); from increase_reservation
and
xenmem_reservation_va_mapping_reset(*1*, &page);
from decrease_reservation and it seems to be not elegant because of that one page/frame passed while we might have multiple pages/frames passed at once.

In the balloon driver the producer of pages for increase_reservation is balloon_retrieve(false) and for decrease_reservation it is alloc_page(gfp).
In case of decrease_reservation the page is added on the list:
LIST_HEAD(pages);
[...]
list_add(&page->lru, &pages);

and in case of increase_reservation it is retrieved page by page and can be put on a list as well with the same code from decrease_reservation, e.g.
LIST_HEAD(pages);
[...]
list_add(&page->lru, &pages);

Thus, both decrease_reservation and increase_reservation may hold their pages on a list before calling xenmem_reservation_va_mapping_{update|reset}.

For that we need a prototype change:
xenmem_reservation_va_mapping_reset(<nr_pages>, <list of pages>); But for xenmem_reservation_va_mapping_update it will look like:
xenmem_reservation_va_mapping_update(<nr_pages>, <list of pages>, <array of frames>)
which seems to be inconsistent. Converting entries of the static frame_list array
into corresponding list doesn't seem to be cute as well.

For dma-buf use-case arrays are more preferable as dma-buf constructs scatter-gather
tables from array of pages etc. and if page list is passed then it needs to be
converted into page array anyways.

So, we can:
1. Keep the prototypes as is, e.g. accept array of pages and use nr_pages == 1 in
case of balloon driver (existing code)
2. Statically allocate struct page* array in the balloon driver and fill it with pages
when those pages are retrieved:
static struct page *page_list[ARRAY_SIZE(frame_list)]; which will take additional 8KiB of space on 64-bit platform, but simplify things a lot.
3. Allocate struct page *page_list[ARRAY_SIZE(frame_list)] dynamically

As to Boris' suggestion "balloon pages are strung on the lru list. Can we keep have
balloon_retrieve return a list of pages on that list?" Because of alloc_xenballooned_pages' retry logic for page retireval, e.g.
    while (pgno < nr_pages) {
        page = balloon_retrieve(true);
        if (page) {
[...]
        } else {
            ret = add_ballooned_pages(nr_pages - pgno);
[...]
    }
I wouldn't change things that much.

Quoted 4 lines

IMO, we can keep 1 page based API with the only overhead for balloon driver of
function calls to xenmem_reservation_va_mapping_{update|reset} for each page.

I still think what I suggested is doable but we can come back to it later and keep your per-page implementation for now.

BTW, I also think you can further simplify xenmem_reservation_va_mapping_* routines by bailing out right away if xen_feature(XENFEAT_auto_translated_physmap). In fact, you might even make them inlines, along the lines of

inline void xenmem_reservation_va_mapping_reset(unsigned long count,
struct page **pages)
{

#ifdef CONFIG_XEN_HAVE_PVMMU
	if (!xen_feature(XENFEAT_auto_translated_physmap))
		__xenmem_reservation_va_mapping_reset(...)

#endif
}

Or some such.

(quoted diff) +0 -2

-boris
 
-boris

Quoted 7 lines · Boris Ostrovsky

BTW, I also think you can further simplify xenmem_reservation_va_mapping_* routines by bailing out right away if xen_feature(XENFEAT_auto_translated_physmap). In fact, you might even make them inlines, along the lines of

Quoted 8 lines

inline void xenmem_reservation_va_mapping_reset(unsigned long count, struct page **pages)
{
#ifdef CONFIG_XEN_HAVE_PVMMU
    if (!xen_feature(XENFEAT_auto_translated_physmap)) __xenmem_reservation_va_mapping_reset(...)
#endif
}

How about:

#ifdef CONFIG_XEN_HAVE_PVMMU
static inline __xenmem_reservation_va_mapping_reset(struct page *page) {
[...]
}
#endif

and

void xenmem_reservation_va_mapping_reset(unsigned long count, struct page **pages)
{
#ifdef CONFIG_XEN_HAVE_PVMMU
    if (!xen_feature(XENFEAT_auto_translated_physmap)) { int i;

        for (i = 0; i < count; i++) __xenmem_reservation_va_mapping_reset(pages[i]); }
#endif
}

This way I can use __xenmem_reservation_va_mapping_reset(page); instead of xenmem_reservation_va_mapping_reset(1, &page);

Quoted 39 lines · Oleksandr Andrushchenko

Quoted 6 lines · Boris Ostrovsky

BTW, I also think you can further simplify xenmem_reservation_va_mapping_* routines by bailing out right away if xen_feature(XENFEAT_auto_translated_physmap). In fact, you might even make them inlines, along the lines of

Quoted 8 lines

inline void xenmem_reservation_va_mapping_reset(unsigned long count,
                     struct page **pages)
{
#ifdef CONFIG_XEN_HAVE_PVMMU
    if (!xen_feature(XENFEAT_auto_translated_physmap)) __xenmem_reservation_va_mapping_reset(...)
#endif
}

How about:

#ifdef CONFIG_XEN_HAVE_PVMMU
static inline __xenmem_reservation_va_mapping_reset(struct page *page) {
[...]
}
#endif

and

void xenmem_reservation_va_mapping_reset(unsigned long count,
                     struct page **pages)
{
#ifdef CONFIG_XEN_HAVE_PVMMU
    if (!xen_feature(XENFEAT_auto_translated_physmap)) { int i;

        for (i = 0; i < count; i++) __xenmem_reservation_va_mapping_reset(pages[i]); }
#endif
}

Quoted 2 lines

This way I can use __xenmem_reservation_va_mapping_reset(page); instead of xenmem_reservation_va_mapping_reset(1, &page);

Sure, this also works.

-boris

From d41751068ac80ca5a375909d6c01cb25716a4975 Mon Sep 17 00:00:00 2001

Quoted 40 lines · Boris Ostrovsky

Quoted 39 lines · Oleksandr Andrushchenko

Quoted 6 lines · Boris Ostrovsky

BTW, I also think you can further simplify xenmem_reservation_va_mapping_* routines by bailing out right away if xen_feature(XENFEAT_auto_translated_physmap). In fact, you might even make them inlines, along the lines of

Quoted 8 lines

inline void xenmem_reservation_va_mapping_reset(unsigned long count, struct page **pages)
{
#ifdef CONFIG_XEN_HAVE_PVMMU
     if (!xen_feature(XENFEAT_auto_translated_physmap)) __xenmem_reservation_va_mapping_reset(...) #endif
}

How about:

#ifdef CONFIG_XEN_HAVE_PVMMU
static inline __xenmem_reservation_va_mapping_reset(struct page *page) {
[...]
}
#endif

and

void xenmem_reservation_va_mapping_reset(unsigned long count, struct page **pages)
{
#ifdef CONFIG_XEN_HAVE_PVMMU
     if (!xen_feature(XENFEAT_auto_translated_physmap)) { int i;

         for (i = 0; i < count; i++) __xenmem_reservation_va_mapping_reset(pages[i]); }
#endif
}

Quoted 4 lines

Quoted 2 lines

This way I can use __xenmem_reservation_va_mapping_reset(page); instead of xenmem_reservation_va_mapping_reset(1, &page);

Sure, this also works.

Could you please take look at the patch attached if this is what we want?

Quoted 2 lines

-boris

Thank you,
Oleksandr

Quoted 40 lines · Boris Ostrovsky

Quoted 39 lines · Oleksandr Andrushchenko

Quoted 6 lines · Boris Ostrovsky

BTW, I also think you can further simplify xenmem_reservation_va_mapping_* routines by bailing out right away if xen_feature(XENFEAT_auto_translated_physmap). In fact, you might even make them inlines, along the lines of

Quoted 8 lines

inline void xenmem_reservation_va_mapping_reset(unsigned long count, struct page **pages)
{
#ifdef CONFIG_XEN_HAVE_PVMMU
     if (!xen_feature(XENFEAT_auto_translated_physmap)) __xenmem_reservation_va_mapping_reset(...) #endif
}

How about:

#ifdef CONFIG_XEN_HAVE_PVMMU
static inline __xenmem_reservation_va_mapping_reset(struct page *page) {
[...]
}
#endif

and

void xenmem_reservation_va_mapping_reset(unsigned long count, struct page **pages)
{
#ifdef CONFIG_XEN_HAVE_PVMMU
     if (!xen_feature(XENFEAT_auto_translated_physmap)) { int i;

         for (i = 0; i < count; i++) __xenmem_reservation_va_mapping_reset(pages[i]); }
#endif
}

Quoted 4 lines

Quoted 2 lines

This way I can use __xenmem_reservation_va_mapping_reset(page); instead of xenmem_reservation_va_mapping_reset(1, &page);

Sure, this also works.

Could you please take look at the patch attached if this is what we want?

Quoted 2 lines

-boris

Thank you,
Oleksandr

Quoted 39 lines · Oleksandr Andrushchenko

Quoted 6 lines · Boris Ostrovsky

BTW, I also think you can further simplify xenmem_reservation_va_mapping_* routines by bailing out right away if xen_feature(XENFEAT_auto_translated_physmap). In fact, you might even make them inlines, along the lines of

Quoted 8 lines

inline void xenmem_reservation_va_mapping_reset(unsigned long count,
                     struct page **pages)
{
#ifdef CONFIG_XEN_HAVE_PVMMU
    if (!xen_feature(XENFEAT_auto_translated_physmap)) __xenmem_reservation_va_mapping_reset(...)
#endif
}

How about:

#ifdef CONFIG_XEN_HAVE_PVMMU
static inline __xenmem_reservation_va_mapping_reset(struct page *page) {
[...]
}
#endif

and

void xenmem_reservation_va_mapping_reset(unsigned long count,
                     struct page **pages)
{
#ifdef CONFIG_XEN_HAVE_PVMMU
    if (!xen_feature(XENFEAT_auto_translated_physmap)) { int i;

        for (i = 0; i < count; i++) __xenmem_reservation_va_mapping_reset(pages[i]); }
#endif
}

Quoted 2 lines

This way I can use __xenmem_reservation_va_mapping_reset(page); instead of xenmem_reservation_va_mapping_reset(1, &page);

Sure, this also works.

-boris

From d41751068ac80ca5a375909d6c01cb25716a4975 Mon Sep 17 00:00:00 2001

Quoted 7 lines · Boris Ostrovsky

BTW, I also think you can further simplify xenmem_reservation_va_mapping_* routines by bailing out right away if xen_feature(XENFEAT_auto_translated_physmap). In fact, you might even make them inlines, along the lines of

Quoted 8 lines

inline void xenmem_reservation_va_mapping_reset(unsigned long count, struct page **pages)
{
#ifdef CONFIG_XEN_HAVE_PVMMU
    if (!xen_feature(XENFEAT_auto_translated_physmap)) __xenmem_reservation_va_mapping_reset(...)
#endif
}

How about:

#ifdef CONFIG_XEN_HAVE_PVMMU
static inline __xenmem_reservation_va_mapping_reset(struct page *page) {
[...]
}
#endif

and

void xenmem_reservation_va_mapping_reset(unsigned long count, struct page **pages)
{
#ifdef CONFIG_XEN_HAVE_PVMMU
    if (!xen_feature(XENFEAT_auto_translated_physmap)) { int i;

        for (i = 0; i < count; i++) __xenmem_reservation_va_mapping_reset(pages[i]); }
#endif
}

This way I can use __xenmem_reservation_va_mapping_reset(page); instead of xenmem_reservation_va_mapping_reset(1, &page);

Quoted 149 lines · Oleksandr Andrushchenko

Quoted 73 lines · Boris Ostrovsky

Quoted 63 lines · Oleksandr Andrushchenko

Quoted 53 lines · Boris Ostrovsky

On 05/25/2018 11:33 AM, Oleksandr Andrushchenko wrote:

drivers/xen/balloon.c +1 -7

@@ -463,11 +457,6 @@ static enum bp_state
 increase_reservation(unsigned long nr_pages)
        int rc;
        unsigned long i;
        struct page   *page;
-    struct xen_memory_reservation reservation = {
-        .address_bits = 0,
-        .extent_order = EXTENT_ORDER,
-        .domid        = DOMID_SELF
-    };
          if (nr_pages > ARRAY_SIZE(frame_list))
            nr_pages = ARRAY_SIZE(frame_list);

@@ -486,9 +475,7 @@ static enum bp_state
 increase_reservation(unsigned long nr_pages)
            page = balloon_next_page(page);
        }
    -    set_xen_guest_handle(reservation.extent_start, frame_list);
-    reservation.nr_extents = nr_pages;
-    rc = HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation);
+    rc = xenmem_reservation_increase(nr_pages, frame_list);
        if (rc <= 0)
            return BP_EAGAIN;
    @@ -496,29 +483,7 @@ static enum bp_state

increase_reservation(unsigned long nr_pages)
           page = balloon_retrieve(false); BUG_ON(page == NULL);
   -#ifdef CONFIG_XEN_HAVE_PVMMU
-        /*
-         * We don't support PV MMU when Linux and Xen is using -         * different page granularity.
-         */
-        BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);

drivers/xen/balloon.c +1 -17

-
-        if (!xen_feature(XENFEAT_auto_translated_physmap)) {
-            unsigned long pfn = page_to_pfn(page);
-
-            set_phys_to_machine(pfn, frame_list[i]);
-
-            /* Link back into the page tables if not highmem. */
-            if (!PageHighMem(page)) {
-                int ret;
-                ret = HYPERVISOR_update_va_mapping(
-                        (unsigned long)__va(pfn << PAGE_SHIFT),
-                        mfn_pte(frame_list[i], PAGE_KERNEL),
-                        0);
-                BUG_ON(ret);
-            }
-        }
-#endif
+        xenmem_reservation_va_mapping_update(1, &page,

&frame_list[i]);

Quoted 4 lines

Can you make a single call to xenmem_reservation_va_mapping_update(rc, ...)? You need to keep track of pages but presumable they can be put into an array (or a list). In fact, perhaps we can have balloon_retrieve() return a set of pages.

This is actually how it is used later on for dma-buf, but I just didn't want
to alter original balloon code too much, but this can be done, in order of simplicity:

Quoted 5 lines

1. Similar to frame_list, e.g. static array of struct page* of size ARRAY_SIZE(frame_list):
more static memory is used, but no allocations

2. Allocated at run-time with kcalloc: allocation can fail

If this is called in freeing DMA buffer code path or in error path then we shouldn't do it.

Quoted 5 lines

Quoted 3 lines

3. Make balloon_retrieve() return a set of pages: will require list/array allocation
and handling, allocation may fail, balloon_retrieve prototype change

balloon pages are strung on the lru list. Can we keep have balloon_retrieve return a list of pages on that list?

First of all, before we go deep in details, I will highlight the goal of the requested change: for balloon driver we call xenmem_reservation_va_mapping_update(*1*, &page, &frame_list[i]); from increase_reservation
and
xenmem_reservation_va_mapping_reset(*1*, &page);
from decrease_reservation and it seems to be not elegant because of that one page/frame passed while we might have multiple pages/frames passed at once.

In the balloon driver the producer of pages for increase_reservation is balloon_retrieve(false) and for decrease_reservation it is alloc_page(gfp).
In case of decrease_reservation the page is added on the list:
LIST_HEAD(pages);
[...]
list_add(&page->lru, &pages);

and in case of increase_reservation it is retrieved page by page and can be put on a list as well with the same code from decrease_reservation, e.g.
LIST_HEAD(pages);
[...]
list_add(&page->lru, &pages);

Thus, both decrease_reservation and increase_reservation may hold their pages on a list before calling xenmem_reservation_va_mapping_{update|reset}.

For that we need a prototype change:
xenmem_reservation_va_mapping_reset(<nr_pages>, <list of pages>); But for xenmem_reservation_va_mapping_update it will look like:
xenmem_reservation_va_mapping_update(<nr_pages>, <list of pages>, <array of frames>)
which seems to be inconsistent. Converting entries of the static frame_list array
into corresponding list doesn't seem to be cute as well.

For dma-buf use-case arrays are more preferable as dma-buf constructs scatter-gather
tables from array of pages etc. and if page list is passed then it needs to be
converted into page array anyways.

So, we can:
1. Keep the prototypes as is, e.g. accept array of pages and use nr_pages == 1 in
case of balloon driver (existing code)
2. Statically allocate struct page* array in the balloon driver and fill it with pages
when those pages are retrieved:
static struct page *page_list[ARRAY_SIZE(frame_list)]; which will take additional 8KiB of space on 64-bit platform, but simplify things a lot.
3. Allocate struct page *page_list[ARRAY_SIZE(frame_list)] dynamically

As to Boris' suggestion "balloon pages are strung on the lru list. Can we keep have
balloon_retrieve return a list of pages on that list?" Because of alloc_xenballooned_pages' retry logic for page retireval, e.g.
    while (pgno < nr_pages) {
        page = balloon_retrieve(true);
        if (page) {
[...]
        } else {
            ret = add_ballooned_pages(nr_pages - pgno);
[...]
    }
I wouldn't change things that much.

Quoted 4 lines

IMO, we can keep 1 page based API with the only overhead for balloon driver of
function calls to xenmem_reservation_va_mapping_{update|reset} for each page.

I still think what I suggested is doable but we can come back to it later and keep your per-page implementation for now.

BTW, I also think you can further simplify xenmem_reservation_va_mapping_* routines by bailing out right away if xen_feature(XENFEAT_auto_translated_physmap). In fact, you might even make them inlines, along the lines of

inline void xenmem_reservation_va_mapping_reset(unsigned long count,
struct page **pages)
{

#ifdef CONFIG_XEN_HAVE_PVMMU
	if (!xen_feature(XENFEAT_auto_translated_physmap))
		__xenmem_reservation_va_mapping_reset(...)

#endif
}

Quoted 62 lines · Oleksandr Andrushchenko

Quoted 52 lines · Boris Ostrovsky

On 05/25/2018 11:33 AM, Oleksandr Andrushchenko wrote:

drivers/xen/balloon.c +1 -7

@@ -463,11 +457,6 @@ static enum bp_state
 increase_reservation(unsigned long nr_pages)
       int rc;
       unsigned long i;
       struct page   *page;
-    struct xen_memory_reservation reservation = {
-        .address_bits = 0,
-        .extent_order = EXTENT_ORDER,
-        .domid        = DOMID_SELF
-    };
         if (nr_pages > ARRAY_SIZE(frame_list))
           nr_pages = ARRAY_SIZE(frame_list);

@@ -486,9 +475,7 @@ static enum bp_state
 increase_reservation(unsigned long nr_pages)
           page = balloon_next_page(page);
       }
   -    set_xen_guest_handle(reservation.extent_start, frame_list);
-    reservation.nr_extents = nr_pages;
-    rc = HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation);
+    rc = xenmem_reservation_increase(nr_pages, frame_list);
       if (rc <= 0)
           return BP_EAGAIN;
   @@ -496,29 +483,7 @@ static enum bp_state

increase_reservation(unsigned long nr_pages)
          page = balloon_retrieve(false); BUG_ON(page == NULL);
  -#ifdef CONFIG_XEN_HAVE_PVMMU
-        /*
-         * We don't support PV MMU when Linux and Xen is using -         * different page granularity.
-         */
-        BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);

drivers/xen/balloon.c +1 -17

-
-        if (!xen_feature(XENFEAT_auto_translated_physmap)) {
-            unsigned long pfn = page_to_pfn(page);
-
-            set_phys_to_machine(pfn, frame_list[i]);
-
-            /* Link back into the page tables if not highmem. */
-            if (!PageHighMem(page)) {
-                int ret;
-                ret = HYPERVISOR_update_va_mapping(
-                        (unsigned long)__va(pfn << PAGE_SHIFT),
-                        mfn_pte(frame_list[i], PAGE_KERNEL),
-                        0);
-                BUG_ON(ret);
-            }
-        }
-#endif
+        xenmem_reservation_va_mapping_update(1, &page, &frame_list[i]);

Quoted 4 lines

Can you make a single call to xenmem_reservation_va_mapping_update(rc, ...)? You need to keep track of pages but presumable they can be put into an array (or a list). In fact, perhaps we can have balloon_retrieve() return a set of pages.

This is actually how it is used later on for dma-buf, but I just didn't want
to alter original balloon code too much, but this can be done, in order of simplicity:

Quoted 5 lines

1. Similar to frame_list, e.g. static array of struct page* of size ARRAY_SIZE(frame_list):
more static memory is used, but no allocations

2. Allocated at run-time with kcalloc: allocation can fail

If this is called in freeing DMA buffer code path or in error path then we shouldn't do it.

Quoted 3 lines

3. Make balloon_retrieve() return a set of pages: will require list/array allocation
and handling, allocation may fail, balloon_retrieve prototype change

balloon pages are strung on the lru list. Can we keep have balloon_retrieve return a list of pages on that list?

-boris

Quoted 4 lines

Could you please tell which of the above will fit better?

Quoted 1 line

Quoted 53 lines · Boris Ostrovsky

On 05/25/2018 11:33 AM, Oleksandr Andrushchenko wrote:

drivers/xen/balloon.c +1 -7

@@ -463,11 +457,6 @@ static enum bp_state
 increase_reservation(unsigned long nr_pages)
       int rc;
       unsigned long i;
       struct page   *page;
-    struct xen_memory_reservation reservation = {
-        .address_bits = 0,
-        .extent_order = EXTENT_ORDER,
-        .domid        = DOMID_SELF
-    };
         if (nr_pages > ARRAY_SIZE(frame_list))
           nr_pages = ARRAY_SIZE(frame_list);

@@ -486,9 +475,7 @@ static enum bp_state
 increase_reservation(unsigned long nr_pages)
           page = balloon_next_page(page);
       }
   -    set_xen_guest_handle(reservation.extent_start, frame_list);
-    reservation.nr_extents = nr_pages;
-    rc = HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation);
+    rc = xenmem_reservation_increase(nr_pages, frame_list);
       if (rc <= 0)
           return BP_EAGAIN;

  @@ -496,29 +483,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
          page = balloon_retrieve(false);
          BUG_ON(page == NULL);
  -#ifdef CONFIG_XEN_HAVE_PVMMU
-        /*
-         * We don't support PV MMU when Linux and Xen is using -         * different page granularity.
-         */
-        BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);

drivers/xen/balloon.c +1 -17

-
-        if (!xen_feature(XENFEAT_auto_translated_physmap)) {
-            unsigned long pfn = page_to_pfn(page);
-
-            set_phys_to_machine(pfn, frame_list[i]);
-
-            /* Link back into the page tables if not highmem. */
-            if (!PageHighMem(page)) {
-                int ret;
-                ret = HYPERVISOR_update_va_mapping(
-                        (unsigned long)__va(pfn << PAGE_SHIFT),
-                        mfn_pte(frame_list[i], PAGE_KERNEL),
-                        0);
-                BUG_ON(ret);
-            }
-        }
-#endif
+        xenmem_reservation_va_mapping_update(1, &page, &frame_list[i]);

Quoted 4 lines

Can you make a single call to xenmem_reservation_va_mapping_update(rc, ...)? You need to keep track of pages but presumable they can be put into an array (or a list). In fact, perhaps we can have balloon_retrieve() return a set of pages.

This is actually how it is used later on for dma-buf, but I just didn't want
to alter original balloon code too much, but this can be done, in order of simplicity:

1. Similar to frame_list, e.g. static array of struct page* of size ARRAY_SIZE(frame_list):
more static memory is used, but no allocations

2. Allocated at run-time with kcalloc: allocation can fail

3. Make balloon_retrieve() return a set of pages: will require list/array allocation
and handling, allocation may fail, balloon_retrieve prototype change

Could you please tell which of the above will fit better?

Quoted 2 lines

Quoted 6 lines · Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <[email protected]>

Memory {increase|decrease}_reservation and VA mappings update/reset code used in balloon driver can be made common, so other drivers can also re-use the same functionality without open-coding. Create a dedicated module

IIUIC this is not really a module, it's a common file.

Quoted 100 lines

for the shared code and export corresponding
symbols for other kernel modules.

Signed-off-by Oleksandr Andrushchenko <[email protected]>

---
  drivers/xen/Makefile          |   1 +
  drivers/xen/balloon.c         |  71 ++----------------
  drivers/xen/mem-reservation.c | 134 ++++++++++++++++++++++++++++++++++
  include/xen/mem_reservation.h |  29 ++++++++
  4 files changed, 170 insertions(+), 65 deletions(-)

create mode 100644 drivers/xen/mem-reservation.c create mode 100644 include/xen/mem_reservation.h

drivers/xen/Makefile +1 -0

index 451e833f5931..3c87b0c3aca6 100644
@@ -2,6 +2,7 @@
  obj-$(CONFIG_HOTPLUG_CPU)		+= cpu_hotplug.o
  obj-$(CONFIG_X86)			+= fallback.o
  obj-y	+= grant-table.o features.o balloon.o manage.o preempt.o time.o
+obj-y	+= mem-reservation.o
  obj-y	+= events/
  obj-y	+= xenbus/
 

drivers/xen/balloon.c +2 -36

index 065f0b607373..57b482d67a3a 100644
@@ -71,6 +71,7 @@
  #include <xen/balloon.h>
  #include <xen/features.h>
  #include <xen/page.h>
+#include <xen/mem_reservation.h>
 
  static int xen_hotplug_unpopulated;
 

@@ -157,13 +158,6 @@ static DECLARE_DELAYED_WORK(balloon_worker, balloon_process);
  #define GFP_BALLOON \
  	(GFP_HIGHUSER | __GFP_NOWARN | __GFP_NORETRY | __GFP_NOMEMALLOC)
 
-static void scrub_page(struct page *page)
-{
-#ifdef CONFIG_XEN_SCRUB_PAGES
-	clear_highpage(page);
-#endif
-}
-
  /* balloon_append: add the given page to the balloon. */
  static void __balloon_append(struct page *page)
  {

@@ -463,11 +457,6 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
  	int rc;
  	unsigned long i;
  	struct page   *page;
-	struct xen_memory_reservation reservation = {
-		.address_bits = 0,
-		.extent_order = EXTENT_ORDER,
-		.domid        = DOMID_SELF
-	};
 
  	if (nr_pages > ARRAY_SIZE(frame_list))
  		nr_pages = ARRAY_SIZE(frame_list);

@@ -486,9 +475,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
  		page = balloon_next_page(page);
  	}
 
-	set_xen_guest_handle(reservation.extent_start, frame_list);
-	reservation.nr_extents = nr_pages;
-	rc = HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation);
+	rc = xenmem_reservation_increase(nr_pages, frame_list);
  	if (rc <= 0)
  		return BP_EAGAIN;
 

@@ -496,29 +483,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
  		page = balloon_retrieve(false);
  		BUG_ON(page == NULL);
 
-#ifdef CONFIG_XEN_HAVE_PVMMU
-		/*
-		 * We don't support PV MMU when Linux and Xen is using
-		 * different page granularity.
-		 */
-		BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
-
-		if (!xen_feature(XENFEAT_auto_translated_physmap)) {
-			unsigned long pfn = page_to_pfn(page);
-
-			set_phys_to_machine(pfn, frame_list[i]);
-
-			/* Link back into the page tables if not highmem. */
-			if (!PageHighMem(page)) {
-				int ret;
-				ret = HYPERVISOR_update_va_mapping(
-						(unsigned long)__va(pfn << PAGE_SHIFT),
-						mfn_pte(frame_list[i], PAGE_KERNEL),
-						0);
-				BUG_ON(ret);
-			}

drivers/xen/balloon.c static enum bp_state increase_reservation(unsigned long nr_pages) · line 486

Quoted 3 lines

drivers/xen/balloon.c +1 -2

-		}
-#endif
+		xenmem_reservation_va_mapping_update(1, &page, &frame_list[i]);

Can you make a single call to xenmem_reservation_va_mapping_update(rc, ...)? You need to keep track of pages but presumable they can be put into an array (or a list). In fact, perhaps we can have balloon_retrieve() return a set of pages.

Quoted 31 lines

drivers/xen/balloon.c +1 -6

  		/* Relinquish the page back to the allocator. */
  		free_reserved_page(page);

@@ -535,11 +500,6 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
  	unsigned long i;
  	struct page *page, *tmp;
  	int ret;
-	struct xen_memory_reservation reservation = {
-		.address_bits = 0,
-		.extent_order = EXTENT_ORDER,
-		.domid        = DOMID_SELF
-	};
  	LIST_HEAD(pages);
 
  	if (nr_pages > ARRAY_SIZE(frame_list))

@@ -553,7 +513,7 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
  			break;
  		}
  		adjust_managed_page_count(page, -1);
-		scrub_page(page);
+		xenmem_reservation_scrub_page(page);
  		list_add(&page->lru, &pages);
  	}
 

@@ -575,25 +535,8 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
  		/* XENMEM_decrease_reservation requires a GFN */
  		frame_list[i++] = xen_page_to_gfn(page);
 

drivers/xen/balloon.c static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) · line 538

Quoted 10 lines

drivers/xen/balloon.c +1 -9

-#ifdef CONFIG_XEN_HAVE_PVMMU
-		/*
-		 * We don't support PV MMU when Linux and Xen is using
-		 * different page granularity.
-		 */
-		BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
-
-		if (!xen_feature(XENFEAT_auto_translated_physmap)) {
-			unsigned long pfn = page_to_pfn(page);
+		xenmem_reservation_va_mapping_reset(1, &page);

and here too.

Quoted 24 lines

drivers/xen/balloon.c +1 -12

-			if (!PageHighMem(page)) {
-				ret = HYPERVISOR_update_va_mapping(
-						(unsigned long)__va(pfn << PAGE_SHIFT),
-						__pte_ma(0), 0);
-				BUG_ON(ret);
-			}
-			__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
-		}
-#endif
  		list_del(&page->lru);
 
  		balloon_append(page);

@@ -601,9 +544,7 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
 
  	flush_tlb_all();
 
-	set_xen_guest_handle(reservation.extent_start, frame_list);
-	reservation.nr_extents   = nr_pages;
-	ret = HYPERVISOR_memory_op(XENMEM_decrease_reservation, &reservation);
+	ret = xenmem_reservation_decrease(nr_pages, frame_list);
  	BUG_ON(ret != nr_pages);
 

drivers/xen/mem-reservation.c line 1

Quoted 8 lines

drivers/xen/balloon.c +0 -0

  	balloon_stats.current_pages -= nr_pages;

drivers/xen/mem-reservation.c new file +1 -0

new file mode 100644
index 000000000000..29882e4324f5
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT

Why is this "OR MIT"? The original file was licensed GPLv2 only.

Quoted 134 lines

drivers/xen/mem-reservation.c new file +133 -0

+
+/******************************************************************************
+ * Xen memory reservation utilities.
+ *
+ * Copyright (c) 2003, B Dragovic
+ * Copyright (c) 2003-2004, M Williamson, K Fraser
+ * Copyright (c) 2005 Dan M. Smith, IBM Corporation
+ * Copyright (c) 2010 Daniel Kiper
+ * Copyright (c) 2018, Oleksandr Andrushchenko, EPAM Systems Inc.
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+
+#include <asm/tlb.h>
+#include <asm/xen/hypercall.h>
+
+#include <xen/interface/memory.h>
+#include <xen/page.h>
+
+/*
+ * Use one extent per PAGE_SIZE to avoid to break down the page into
+ * multiple frame.
+ */
+#define EXTENT_ORDER (fls(XEN_PFN_PER_PAGE) - 1)
+
+void xenmem_reservation_scrub_page(struct page *page)
+{
+#ifdef CONFIG_XEN_SCRUB_PAGES
+	clear_highpage(page);
+#endif
+}
+EXPORT_SYMBOL(xenmem_reservation_scrub_page);
+
+void xenmem_reservation_va_mapping_update(unsigned long count,
+					  struct page **pages,
+					  xen_pfn_t *frames)
+{
+#ifdef CONFIG_XEN_HAVE_PVMMU
+	int i;
+
+	for (i = 0; i < count; i++) {
+		struct page *page;
+
+		page = pages[i];
+		BUG_ON(page == NULL);
+
+		/*
+		 * We don't support PV MMU when Linux and Xen is using
+		 * different page granularity.
+		 */
+		BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
+
+		if (!xen_feature(XENFEAT_auto_translated_physmap)) {
+			unsigned long pfn = page_to_pfn(page);
+
+			set_phys_to_machine(pfn, frames[i]);
+
+			/* Link back into the page tables if not highmem. */
+			if (!PageHighMem(page)) {
+				int ret;
+
+				ret = HYPERVISOR_update_va_mapping(
+						(unsigned long)__va(pfn << PAGE_SHIFT),
+						mfn_pte(frames[i], PAGE_KERNEL),
+						0);
+				BUG_ON(ret);
+			}
+		}
+	}
+#endif
+}
+EXPORT_SYMBOL(xenmem_reservation_va_mapping_update);
+
+void xenmem_reservation_va_mapping_reset(unsigned long count,
+					 struct page **pages)
+{
+#ifdef CONFIG_XEN_HAVE_PVMMU
+	int i;
+
+	for (i = 0; i < count; i++) {
+		/*
+		 * We don't support PV MMU when Linux and Xen is using
+		 * different page granularity.
+		 */
+		BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
+
+		if (!xen_feature(XENFEAT_auto_translated_physmap)) {
+			struct page *page = pages[i];
+			unsigned long pfn = page_to_pfn(page);
+
+			if (!PageHighMem(page)) {
+				int ret;
+
+				ret = HYPERVISOR_update_va_mapping(
+						(unsigned long)__va(pfn << PAGE_SHIFT),
+						__pte_ma(0), 0);
+				BUG_ON(ret);
+			}
+			__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
+		}
+	}
+#endif
+}
+EXPORT_SYMBOL(xenmem_reservation_va_mapping_reset);
+
+int xenmem_reservation_increase(int count, xen_pfn_t *frames)
+{
+	struct xen_memory_reservation reservation = {
+		.address_bits = 0,
+		.extent_order = EXTENT_ORDER,
+		.domid        = DOMID_SELF
+	};
+
+	set_xen_guest_handle(reservation.extent_start, frames);
+	reservation.nr_extents = count;
+	return HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation);
+}
+EXPORT_SYMBOL(xenmem_reservation_increase);
+
+int xenmem_reservation_decrease(int count, xen_pfn_t *frames)
+{
+	struct xen_memory_reservation reservation = {
+		.address_bits = 0,
+		.extent_order = EXTENT_ORDER,
+		.domid        = DOMID_SELF
+	};
+
+	set_xen_guest_handle(reservation.extent_start, frames);
+	reservation.nr_extents = count;
+	return HYPERVISOR_memory_op(XENMEM_decrease_reservation, &reservation);
+}
+EXPORT_SYMBOL(xenmem_reservation_decrease);

include/xen/mem_reservation.h line 1

Quoted 7 lines

include/xen/mem_reservation.h new file +1 -0

new file mode 100644
index 000000000000..9306d9b8743c
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 OR MIT */

and here too.

-boris

Quoted 20 lines

include/xen/mem_reservation.h new file +20 -0

+
+/*
+ * Xen memory reservation utilities.
+ *
+ * Copyright (c) 2003, B Dragovic
+ * Copyright (c) 2003-2004, M Williamson, K Fraser
+ * Copyright (c) 2005 Dan M. Smith, IBM Corporation
+ * Copyright (c) 2010 Daniel Kiper
+ * Copyright (c) 2018, Oleksandr Andrushchenko, EPAM Systems Inc.
+ */
+
+#ifndef _XENMEM_RESERVATION_H
+#define _XENMEM_RESERVATION_H
+
+void xenmem_reservation_scrub_page(struct page *page);
+
+void xenmem_reservation_va_mapping_update(unsigned long count,
+					  struct page **pages,
+					  xen_pfn_t *frames);
+

include/xen/mem_reservation.h line 29

Quoted 8 lines

include/xen/mem_reservation.h new file +8 -0

+void xenmem_reservation_va_mapping_reset(unsigned long count,
+					 struct page **pages);
+
+int xenmem_reservation_increase(int count, xen_pfn_t *frames);
+
+int xenmem_reservation_decrease(int count, xen_pfn_t *frames);
+
+#endif