Shunsuke Mie
2023-Apr-10 10:56 UTC
[PATCH] tools/virtio: fix build caused by virtio_ring changes
On 2023/04/10 19:00, Shunsuke Mie wrote:> > On 2023/04/10 18:46, Michael S. Tsirkin wrote: >> On Mon, Apr 10, 2023 at 06:24:19PM +0900, Shunsuke Mie wrote: >>> Fix the build dependency for virtio_test. virtio_ring requires >>> container_of_const() and struce device. >>> >>> Signed-off-by: Shunsuke Mie <mie at igel.co.jp> >>> --- >>> ? tools/include/linux/container_of.h | 36 >>> ++++++++++++++++++++++++++++++ >>> ? tools/virtio/linux/compiler.h????? |? 3 +++ >>> ? tools/virtio/linux/kernel.h??????? |? 5 +---- >>> ? tools/virtio/linux/module.h??????? |? 1 + >>> ? 4 files changed, 41 insertions(+), 4 deletions(-) >>> ? create mode 100644 tools/include/linux/container_of.h >>> >>> diff --git a/tools/include/linux/container_of.h >>> b/tools/include/linux/container_of.h >>> new file mode 100644 >>> index 000000000000..06e293b7cfda >>> --- /dev/null >>> +++ b/tools/include/linux/container_of.h >>> @@ -0,0 +1,36 @@ >>> +/* SPDX-License-Identifier: GPL-2.0 */ >>> +#ifndef _LINUX_CONTAINER_OF_H >>> +#define _LINUX_CONTAINER_OF_H >>> + >>> +#include <linux/build_bug.h> >>> +#include <linux/stddef.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. >>> + * >>> + * WARNING: any const qualifier of @ptr is lost. >>> + */ >>> +#define container_of(ptr, type, member) ({??????????????? \ >>> +??? void *__mptr = (void *)(ptr);??????????????????? \ >>> +??? static_assert(__same_type(*(ptr), ((type *)0)->member) ||??? \ >>> +????????????? __same_type(*(ptr), void),??????????? \ >>> +????????????? "pointer type mismatch in container_of()"); \ >>> +??? ((type *)(__mptr - offsetof(type, member))); }) >>> + >>> +/** >>> + * container_of_const - cast a member of a structure out to the >>> containing >>> + *??????????? structure and preserve the const-ness of the pointer >>> + * @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_const(ptr, type, member) \ >>> +??? _Generic(ptr,??????????????????????????? \ >>> +??????? const typeof(*(ptr)) *: ((const type *)container_of(ptr, >>> type, member)),\ >>> +??????? default: ((type *)container_of(ptr, type, member)) \ >>> +??? ) >>> + >>> +#endif??? /* _LINUX_CONTAINER_OF_H */ >> >> Please just do >> #include "../../../include/linux/container_of.h" >> instead. > Okey, I'll do that. >> >> >>> diff --git a/tools/virtio/linux/compiler.h >>> b/tools/virtio/linux/compiler.h >>> index 2c51bccb97bb..ac27b3ea6e67 100644 >>> --- a/tools/virtio/linux/compiler.h >>> +++ b/tools/virtio/linux/compiler.h >>> @@ -8,4 +8,7 @@ >>> ? #define READ_ONCE(var) (*((volatile typeof(var) *)(&(var)))) >>> ? ? #define __aligned(x) __attribute((__aligned__(x))) >>> + >>> +#define __same_type(a, b) __builtin_types_compatible_p(typeof(a), >>> typeof(b)) >>> + >>> ? #endif >> #include "../../../include/linux/compiler_types.h" instead? > I'll try it. >>> diff --git a/tools/virtio/linux/kernel.h b/tools/virtio/linux/kernel.h >>> index 8b877167933d..3cd20cb92649 100644 >>> --- a/tools/virtio/linux/kernel.h >>> +++ b/tools/virtio/linux/kernel.h >>> @@ -10,6 +10,7 @@ >>> ? #include <stdarg.h> >>> ? ? #include <linux/compiler.h> >>> +#include <linux/container_of.h> >>> ? #include <linux/log2.h> >>> ? #include <linux/types.h> >>> ? #include <linux/overflow.h> >>> @@ -107,10 +108,6 @@ static inline void free_page(unsigned long addr) >>> ????? free((void *)addr); >>> ? } >>> ? -#define container_of(ptr, type, member) ({??????????? \ >>> -??? const typeof( ((type *)0)->member ) *__mptr = (ptr);??? \ >>> -??? (type *)( (char *)__mptr - offsetof(type,member) );}) >>> - >>> ? # ifndef likely >>> ? #? define likely(x)??? (__builtin_expect(!!(x), 1)) >>> ? # endif >>> diff --git a/tools/virtio/linux/module.h b/tools/virtio/linux/module.h >>> index 9dfa96fea2b2..e2e791dbd104 100644 >>> --- a/tools/virtio/linux/module.h >>> +++ b/tools/virtio/linux/module.h >>> @@ -5,3 +5,4 @@ >>> ????? static __attribute__((unused)) const char >>> *__MODULE_LICENSE_name = \ >>> ????????? __MODULE_LICENSE_value >>> ? +struct device; >> >> It's not there in include/linux/module - pls put it in the >> same here where it is for linux. > I understood. I'll add some files that has the definition.In vringh.c, 'struct device' is used in virtio_ring.h. Upon investigating the preprocessed file, it was discovered that 'struct device' is defined in bitmap.h, which is included from architecture-specific headers. On x86, the nesting includes 'linux/module.h' -> ... -> 'arch/x86/include/asm/msr.h' -> 'arch/x86/include/asm/cpumask.h' -> 'linux/cpumask.h' -> 'linux/bitmap.h'. Could you advise on the appropriate way to include this file?>>> -- >>> 2.25.1 > > Best regards, > > Shunsuke. >
Michael S. Tsirkin
2023-Apr-10 10:58 UTC
[PATCH] tools/virtio: fix build caused by virtio_ring changes
On Mon, Apr 10, 2023 at 07:56:13PM +0900, Shunsuke Mie wrote:> > On 2023/04/10 19:00, Shunsuke Mie wrote: > > > > On 2023/04/10 18:46, Michael S. Tsirkin wrote: > > > On Mon, Apr 10, 2023 at 06:24:19PM +0900, Shunsuke Mie wrote: > > > > Fix the build dependency for virtio_test. virtio_ring requires > > > > container_of_const() and struce device. > > > > > > > > Signed-off-by: Shunsuke Mie <mie at igel.co.jp> > > > > --- > > > > ? tools/include/linux/container_of.h | 36 > > > > ++++++++++++++++++++++++++++++ > > > > ? tools/virtio/linux/compiler.h????? |? 3 +++ > > > > ? tools/virtio/linux/kernel.h??????? |? 5 +---- > > > > ? tools/virtio/linux/module.h??????? |? 1 + > > > > ? 4 files changed, 41 insertions(+), 4 deletions(-) > > > > ? create mode 100644 tools/include/linux/container_of.h > > > > > > > > diff --git a/tools/include/linux/container_of.h > > > > b/tools/include/linux/container_of.h > > > > new file mode 100644 > > > > index 000000000000..06e293b7cfda > > > > --- /dev/null > > > > +++ b/tools/include/linux/container_of.h > > > > @@ -0,0 +1,36 @@ > > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > > +#ifndef _LINUX_CONTAINER_OF_H > > > > +#define _LINUX_CONTAINER_OF_H > > > > + > > > > +#include <linux/build_bug.h> > > > > +#include <linux/stddef.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. > > > > + * > > > > + * WARNING: any const qualifier of @ptr is lost. > > > > + */ > > > > +#define container_of(ptr, type, member) ({??????????????? \ > > > > +??? void *__mptr = (void *)(ptr);??????????????????? \ > > > > +??? static_assert(__same_type(*(ptr), ((type *)0)->member) ||??? \ > > > > +????????????? __same_type(*(ptr), void),??????????? \ > > > > +????????????? "pointer type mismatch in container_of()"); \ > > > > +??? ((type *)(__mptr - offsetof(type, member))); }) > > > > + > > > > +/** > > > > + * container_of_const - cast a member of a structure out to the > > > > containing > > > > + *??????????? structure and preserve the const-ness of the pointer > > > > + * @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_const(ptr, type, member) \ > > > > +??? _Generic(ptr,??????????????????????????? \ > > > > +??????? const typeof(*(ptr)) *: ((const type > > > > *)container_of(ptr, type, member)),\ > > > > +??????? default: ((type *)container_of(ptr, type, member)) \ > > > > +??? ) > > > > + > > > > +#endif??? /* _LINUX_CONTAINER_OF_H */ > > > > > > Please just do > > > #include "../../../include/linux/container_of.h" > > > instead. > > Okey, I'll do that. > > > > > > > > > > diff --git a/tools/virtio/linux/compiler.h > > > > b/tools/virtio/linux/compiler.h > > > > index 2c51bccb97bb..ac27b3ea6e67 100644 > > > > --- a/tools/virtio/linux/compiler.h > > > > +++ b/tools/virtio/linux/compiler.h > > > > @@ -8,4 +8,7 @@ > > > > ? #define READ_ONCE(var) (*((volatile typeof(var) *)(&(var)))) > > > > ? ? #define __aligned(x) __attribute((__aligned__(x))) > > > > + > > > > +#define __same_type(a, b) > > > > __builtin_types_compatible_p(typeof(a), typeof(b)) > > > > + > > > > ? #endif > > > #include "../../../include/linux/compiler_types.h" instead? > > I'll try it. > > > > diff --git a/tools/virtio/linux/kernel.h b/tools/virtio/linux/kernel.h > > > > index 8b877167933d..3cd20cb92649 100644 > > > > --- a/tools/virtio/linux/kernel.h > > > > +++ b/tools/virtio/linux/kernel.h > > > > @@ -10,6 +10,7 @@ > > > > ? #include <stdarg.h> > > > > ? ? #include <linux/compiler.h> > > > > +#include <linux/container_of.h> > > > > ? #include <linux/log2.h> > > > > ? #include <linux/types.h> > > > > ? #include <linux/overflow.h> > > > > @@ -107,10 +108,6 @@ static inline void free_page(unsigned long addr) > > > > ????? free((void *)addr); > > > > ? } > > > > ? -#define container_of(ptr, type, member) ({??????????? \ > > > > -??? const typeof( ((type *)0)->member ) *__mptr = (ptr);??? \ > > > > -??? (type *)( (char *)__mptr - offsetof(type,member) );}) > > > > - > > > > ? # ifndef likely > > > > ? #? define likely(x)??? (__builtin_expect(!!(x), 1)) > > > > ? # endif > > > > diff --git a/tools/virtio/linux/module.h b/tools/virtio/linux/module.h > > > > index 9dfa96fea2b2..e2e791dbd104 100644 > > > > --- a/tools/virtio/linux/module.h > > > > +++ b/tools/virtio/linux/module.h > > > > @@ -5,3 +5,4 @@ > > > > ????? static __attribute__((unused)) const char > > > > *__MODULE_LICENSE_name = \ > > > > ????????? __MODULE_LICENSE_value > > > > ? +struct device; > > > > > > It's not there in include/linux/module - pls put it in the > > > same here where it is for linux. > > I understood. I'll add some files that has the definition. > > In vringh.c, 'struct device' is used in virtio_ring.h. Upon investigating > the preprocessed file, it was > > discovered that 'struct device' is defined in bitmap.h, which is included > from architecture-specific headers. > > On x86, the nesting includes > > 'linux/module.h' -> ... -> 'arch/x86/include/asm/msr.h' -> > 'arch/x86/include/asm/cpumask.h' -> 'linux/cpumask.h' -> 'linux/bitmap.h'. > > Could you advise on the appropriate way to include this file?Let's just add struct device; in virtio_ring.h, make it self-sufficient.> > > > -- > > > > 2.25.1 > > > > Best regards, > > > > Shunsuke. > >