[PATCH] xen/bitmap: Drop unused headers

[PATCH] xen/bitmap: Drop unused headers

[Andrew Cooper]

Nothing in bitmap.h uses lib.h, and there's no point including types.h when we
need to include bitops.h anyway.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: George Dunlap <George.Dunlap@citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Wei Liu <wl@xen.org>
CC: Julien Grall <julien@xen.org>
---
 xen/include/xen/bitmap.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/xen/include/xen/bitmap.h b/xen/include/xen/bitmap.h
index b9f980e91930..9f749e3913d8 100644
--- a/xen/include/xen/bitmap.h
+++ b/xen/include/xen/bitmap.h
@@ -3,8 +3,6 @@
 
 #ifndef __ASSEMBLY__
 
-#include <xen/lib.h>
-#include <xen/types.h>
 #include <xen/bitops.h>
 
 /*
-- 
2.30.2

Re: [PATCH] xen/bitmap: Drop unused headers

[Jan Beulich]

On 01.02.2024 11:35, Andrew Cooper wrote:
> Nothing in bitmap.h uses lib.h, and there's no point including types.h when we
> need to include bitops.h anyway.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH] xen/bitmap: Drop unused headers

[Andrew Cooper]

On 01/02/2024 10:35 am, Andrew Cooper wrote:
> Nothing in bitmap.h uses lib.h, and there's no point including types.h when we
> need to include bitops.h anyway.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: George Dunlap <George.Dunlap@citrix.com>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Wei Liu <wl@xen.org>
> CC: Julien Grall <julien@xen.org>
> ---
>  xen/include/xen/bitmap.h | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/xen/include/xen/bitmap.h b/xen/include/xen/bitmap.h
> index b9f980e91930..9f749e3913d8 100644
> --- a/xen/include/xen/bitmap.h
> +++ b/xen/include/xen/bitmap.h
> @@ -3,8 +3,6 @@
>  
>  #ifndef __ASSEMBLY__
>  
> -#include <xen/lib.h>
> -#include <xen/types.h>
>  #include <xen/bitops.h>

Turns out this went too far, and breaks PPC.  Other arches look ok.

https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/6076263594

bitmap.h uses mem{set,cpy}() so needs string.h.  That's a bug in this
patch specifically.

However, cpumask.h transitively picks up IS_ALIGNED() (so needs
macros.h) and ASSERT() which sadly is still in lib.h

I guess moving the mess from bitmap.h to cpumask.h is a (minor) improvement.

But this comes back to the header tangle which prevented moving BUG() in
the first place.  Sadly there's been no reply to my question in the
debugger.h removal, but I'm going to get that committed and then we can
re-evaluate.

~Andrew

[PATCH] xen/bitmap: Drop unused headers

[Andrew Cooper]

Nothing in bitmap.h uses lib.h.  Reduce to just macros.h and string.h.  Drop
types.h while at it, as it comes in through bitops.h

cpumask.h and nodemask.h only include kernel.h to get container_of().  Move it
into macros.h where it belongs.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Anthony PERARD <anthony.perard@vates.tech>
CC: Michal Orzel <michal.orzel@amd.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Julien Grall <julien@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>

https://gitlab.com/xen-project/hardware/xen-staging/-/pipelines/1810741296

v2:
 * Rebase by a year.
 * Move container_of() too.
---
 xen/include/xen/bitmap.h   |  4 ++--
 xen/include/xen/cpumask.h  |  1 -
 xen/include/xen/kernel.h   | 12 ------------
 xen/include/xen/macros.h   | 13 +++++++++++++
 xen/include/xen/nodemask.h |  1 -
 5 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/xen/include/xen/bitmap.h b/xen/include/xen/bitmap.h
index b5e9cdd3db86..79b2cd171595 100644
--- a/xen/include/xen/bitmap.h
+++ b/xen/include/xen/bitmap.h
@@ -3,9 +3,9 @@
 
 #ifndef __ASSEMBLY__
 
-#include <xen/lib.h>
-#include <xen/types.h>
 #include <xen/bitops.h>
+#include <xen/macros.h>
+#include <xen/string.h>
 
 /*
  * bitmaps provide bit arrays that consume one or more unsigned
diff --git a/xen/include/xen/cpumask.h b/xen/include/xen/cpumask.h
index b713bb69a9cb..a7715dfa0308 100644
--- a/xen/include/xen/cpumask.h
+++ b/xen/include/xen/cpumask.h
@@ -57,7 +57,6 @@
 
 #include <xen/bitmap.h>
 #include <xen/bug.h>
-#include <xen/kernel.h>
 #include <xen/random.h>
 
 typedef struct cpumask{ DECLARE_BITMAP(bits, NR_CPUS); } cpumask_t;
diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
index c5b6cc977772..378f21c247a6 100644
--- a/xen/include/xen/kernel.h
+++ b/xen/include/xen/kernel.h
@@ -8,18 +8,6 @@
 #include <xen/macros.h>
 #include <xen/types.h>
 
-/**
- * container_of - cast a member of a structure out to the containing structure
- *
- * @ptr:	the pointer to the member.
- * @type:	the type of the container struct this is embedded in.
- * @member:	the name of the member within the struct.
- *
- */
-#define container_of(ptr, type, member) ({                      \
-        typeof_field(type, member) *__mptr = (ptr);             \
-        (type *)( (char *)__mptr - offsetof(type,member) );})
-
 /**
  * __struct_group() - Create a mirrored named and anonyomous struct
  *
diff --git a/xen/include/xen/macros.h b/xen/include/xen/macros.h
index cd528fbdb127..58affb1588c5 100644
--- a/xen/include/xen/macros.h
+++ b/xen/include/xen/macros.h
@@ -102,6 +102,19 @@
  */
 #define sizeof_field(type, member) sizeof(((type *)NULL)->member)
 
+/**
+ * container_of - cast a member of a structure out to the containing structure
+ *
+ * @ptr:	the pointer to the member.
+ * @type:	the type of the container struct this is embedded in.
+ * @member:	the name of the member within the struct.
+ */
+#define container_of(ptr, type, member)                         \
+    ({                                                          \
+        typeof_field(type, member) *__mptr = (ptr);             \
+        (type *)((void *)__mptr - offsetof(type, member));      \
+    })
+
 /* Cast an arbitrary integer to a pointer. */
 #define _p(x) ((void *)(unsigned long)(x))
 
diff --git a/xen/include/xen/nodemask.h b/xen/include/xen/nodemask.h
index 1dd6c7458e77..60d6455feb8b 100644
--- a/xen/include/xen/nodemask.h
+++ b/xen/include/xen/nodemask.h
@@ -53,7 +53,6 @@
  * for_each_online_node(node)		for-loop node over node_online_map
  */
 
-#include <xen/kernel.h>
 #include <xen/bitmap.h>
 #include <xen/numa.h>
 
-- 
2.39.5

Re: [PATCH] xen/bitmap: Drop unused headers

[Julien Grall]

Hi Andrew,

On 10/05/2025 14:12, Andrew Cooper wrote:
> Nothing in bitmap.h uses lib.h.  Reduce to just macros.h and string.h.  Drop
> types.h while at it, as it comes in through bitops.h
> 
> cpumask.h and nodemask.h only include kernel.h to get container_of().  Move it
> into macros.h where it belongs.

The wording implies that container_of() will be moved as-is (barring 
coding style). However...

[...]

> -/**
> - * container_of - cast a member of a structure out to the containing structure
> - *
> - * @ptr:	the pointer to the member.
> - * @type:	the type of the container struct this is embedded in.
> - * @member:	the name of the member within the struct.
> - *
> - */
> -#define container_of(ptr, type, member) ({                      \
> -        typeof_field(type, member) *__mptr = (ptr);             \
> -        (type *)( (char *)__mptr - offsetof(type,member) );})
> -

I see the cast was switch from "char *" to ...

>   /**
>    * __struct_group() - Create a mirrored named and anonyomous struct
>    *
> diff --git a/xen/include/xen/macros.h b/xen/include/xen/macros.h
> index cd528fbdb127..58affb1588c5 100644
> --- a/xen/include/xen/macros.h
> +++ b/xen/include/xen/macros.h
> @@ -102,6 +102,19 @@
>    */
>   #define sizeof_field(type, member) sizeof(((type *)NULL)->member)
>   
> +/**
> + * container_of - cast a member of a structure out to the containing structure
> + *
> + * @ptr:	the pointer to the member.
> + * @type:	the type of the container struct this is embedded in.
> + * @member:	the name of the member within the struct.
> + */
> +#define container_of(ptr, type, member)                         \
> +    ({                                                          \
> +        typeof_field(type, member) *__mptr = (ptr);             \
> +        (type *)((void *)__mptr - offsetof(type, member));      \

... "void *". AFAICT, this is doesn't change anything. However, I think 
it is worth mentioning in the commit message to show this was intended.

With that:

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall

Re: [PATCH] xen/bitmap: Drop unused headers

[Roger Pau Monné]

On Sat, May 10, 2025 at 02:12:45PM +0100, Andrew Cooper wrote:
> Nothing in bitmap.h uses lib.h.  Reduce to just macros.h and string.h.  Drop
> types.h while at it, as it comes in through bitops.h
> 
> cpumask.h and nodemask.h only include kernel.h to get container_of().  Move it
> into macros.h where it belongs.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.