[Xen-devel] [PATCH v3] xen/mm: Fix page_list_* helpers to evaluate all their arguments

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

If an architecture does not provide a custom page_list_entry, default
page_list_* helpers are provided, wrapping list_head as an underlying type for
page_list_head.

The two declarations of the page_list_* helpers differ between defines and
static inline functions, where the defines discard some of their parameters.

This causes a compilation failure if CONFIG_BIGMEM and debug=n in p2m-pod.c:

  p2m-pod.c: In function ‘p2m_pod_cache_add’:
  p2m-pod.c:72:20: error: unused variable ‘d’ [-Werror=unused-variable]
       struct domain *d = p2m->domain;
                      ^
  cc1: all warnings being treated as errors

because the use of d outside of the !NDEBUG section doesn't get evaluated as a
parameter by page_list_del().

Fix this by turning all #defines into static inline functions, so all
parameters are evaluated even if they are not used.

This reveals a build issue on ARM.  page_alloc.c references
d->arch.relmem_list in the previously-discarded parameter.  Fix this by
introducing relmem_list for ARM (currently unused).

While editing this area, correct the return type of page_list_empty from int
to bool_t.

Reported-by: Doug Goldstein <cardoe@xxxxxxxxxx>
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Reviewed-by: Doug Goldstein <cardoe@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Tim Deegan <tim@xxxxxxx>
CC: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
CC: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
CC: Julien Grall <julien.grall@xxxxxxx>

v2:
 * Remove explicit casts, and missing !! in page_list_empty()

v3:
 * Fix build on ARM.  As it is referenced from common, relmem_list shouldn't
   be an .arch variable.  However, moving it would split it away from the
   relmem enumeration, which is specific to the arch.  (Basically - I lack
   sufficient TUITs to disentagle this properly, and this is the most simple 
fix.)
---
 xen/include/asm-arm/domain.h |  1 +
 xen/include/xen/mm.h         | 95 ++++++++++++++++++++++++++++++++++----------
 2 files changed, 75 insertions(+), 21 deletions(-)

diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index c35ed40..c274547 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -65,6 +65,7 @@ struct arch_domain
         RELMEM_mapping,
         RELMEM_done,
     } relmem;
+    struct page_list_head relmem_list;
 
     /* Virtual CPUID */
     uint32_t vpidr;
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index a795dd6..8600cf6 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -220,7 +220,7 @@ struct page_list_head
 # define INIT_PAGE_LIST_HEAD(head) ((head)->tail = (head)->next = NULL)
 # define INIT_PAGE_LIST_ENTRY(ent) ((ent)->prev = (ent)->next = PAGE_LIST_NULL)
 
-static inline int
+static inline bool_t
 page_list_empty(const struct page_list_head *head)
 {
     return !head->next;
@@ -392,31 +392,84 @@ page_list_splice(struct page_list_head *list, struct 
page_list_head *head)
 # define PAGE_LIST_HEAD                  LIST_HEAD
 # define INIT_PAGE_LIST_HEAD             INIT_LIST_HEAD
 # define INIT_PAGE_LIST_ENTRY            INIT_LIST_HEAD
-# define page_list_empty                 list_empty
-# define page_list_first(hd)             \
-    list_first_entry(hd, struct page_info, list)
-# define page_list_last(hd)              \
-    list_last_entry(hd, struct page_info, list)
-# define page_list_next(pg, hd)          list_next_entry(pg, list)
-# define page_list_prev(pg, hd)          list_prev_entry(pg, list)
-# define page_list_add(pg, hd)           list_add(&(pg)->list, hd)
-# define page_list_add_tail(pg, hd)      list_add_tail(&(pg)->list, hd)
-# define page_list_del(pg, hd)           list_del(&(pg)->list)
-# define page_list_del2(pg, hd1, hd2)    list_del(&(pg)->list)
-# define page_list_remove_head(hd)       (!page_list_empty(hd) ? \
-    ({ \
-        struct page_info *__pg = page_list_first(hd); \
-        list_del(&__pg->list); \
-        __pg; \
-    }) : NULL)
-# define page_list_move(dst, src)        (!list_empty(src) ? \
-    list_replace_init(src, dst) : (void)0)
+
+static inline bool_t
+page_list_empty(const struct page_list_head *head)
+{
+    return !!list_empty(head);
+}
+static inline struct page_info *
+page_list_first(const struct page_list_head *head)
+{
+    return list_first_entry(head, struct page_info, list);
+}
+static inline struct page_info *
+page_list_last(const struct page_list_head *head)
+{
+    return list_last_entry(head, struct page_info, list);
+}
+static inline struct page_info *
+page_list_next(const struct page_info *page,
+               const struct page_list_head *head)
+{
+    return list_entry(page->list.next, struct page_info, list);
+}
+static inline struct page_info *
+page_list_prev(const struct page_info *page,
+               const struct page_list_head *head)
+{
+    return list_entry(page->list.prev, struct page_info, list);
+}
+static inline void
+page_list_add(struct page_info *page, struct page_list_head *head)
+{
+    list_add(&page->list, head);
+}
+static inline void
+page_list_add_tail(struct page_info *page, struct page_list_head *head)
+{
+    list_add_tail(&page->list, head);
+}
+static inline void
+page_list_del(struct page_info *page, struct page_list_head *head)
+{
+    list_del(&page->list);
+}
+static inline void
+page_list_del2(struct page_info *page, struct page_list_head *head1,
+               struct page_list_head *head2)
+{
+    list_del(&page->list);
+}
+static inline struct page_info *
+page_list_remove_head(struct page_list_head *head)
+{
+    struct page_info *pg;
+
+    if ( page_list_empty(head) )
+        return NULL;
+
+    pg = page_list_first(head);
+    list_del(&pg->list);
+    return pg;
+}
+static inline void
+page_list_move(struct page_list_head *dst, struct page_list_head *src)
+{
+    if ( !list_empty(src) )
+        list_replace_init(src, dst);
+}
+static inline void
+page_list_splice(struct page_list_head *list, struct page_list_head *head)
+{
+    list_splice(list, head);
+}
+
 # define page_list_for_each(pos, head)   list_for_each_entry(pos, head, list)
 # define page_list_for_each_safe(pos, tmp, head) \
     list_for_each_entry_safe(pos, tmp, head, list)
 # define page_list_for_each_safe_reverse(pos, tmp, head) \
     list_for_each_entry_safe_reverse(pos, tmp, head, list)
-# define page_list_splice(list, hd)        list_splice(list, hd)
 #endif
 
 static inline unsigned int get_order_from_bytes(paddr_t size)
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel