On Tue, Apr 11, 2023 at 12:25:39PM +0900, Shunsuke Mie wrote: > > On 2023/04/11 4:44, Michael S. Tsirkin wrote: > > On Mon, Apr 10, 2023 at 08:40:34PM +0200, Greg Kroah-Hartman wrote: > > > On Mon, Apr 10, 2023 at 08:00:33AM -0400, Michael S. Tsirkin wrote: > > > > On Mon, Apr 10, 2023 at 08:28:45PM +0900, Shunsuke Mie wrote: > > > > > Fix the build dependency for virtio_test. The virtio_ring that is used from > > > > > the test requires container_of_const(). Change to use container_of.h kernel > > > > > header directly and adapt related codes. > > > > > > > > > > Signed-off-by: Shunsuke Mie <mie@xxxxxxxxxx> > > > > This is only for next right? That's where container_of_const > > > > things are I think ... > > > container_of_const() is in 6.2. > > > > Absolutely but the patch switching virtio to that is not there. > > > > > > > > > --- > > > > > tools/include/linux/types.h | 1 - > > > > > tools/virtio/linux/compiler.h | 2 ++ > > > > > tools/virtio/linux/kernel.h | 5 +---- > > > > > tools/virtio/linux/module.h | 1 - > > > > > tools/virtio/linux/uaccess.h | 11 ++--------- > > > > > 5 files changed, 5 insertions(+), 15 deletions(-) > > > > > > > > > > diff --git a/tools/include/linux/types.h b/tools/include/linux/types.h > > > > > index 051fdeaf2670..f1896b70a8e5 100644 > > > > > --- a/tools/include/linux/types.h > > > > > +++ b/tools/include/linux/types.h > > > > > @@ -49,7 +49,6 @@ typedef __s8 s8; > > > > > #endif > > > > > #define __force > > > > > -#define __user > > > Why is this needed? > > > > > > > > #define __must_check > > > > > #define __cold > > > > > diff --git a/tools/virtio/linux/compiler.h b/tools/virtio/linux/compiler.h > > > > > index 2c51bccb97bb..1f3a15b954b9 100644 > > > > > --- a/tools/virtio/linux/compiler.h > > > > > +++ b/tools/virtio/linux/compiler.h > > > > > @@ -2,6 +2,8 @@ > > > > > #ifndef LINUX_COMPILER_H > > > > > #define LINUX_COMPILER_H > > > > > +#include "../../../include/linux/compiler_types.h" > > > While I understand your need to not want to duplicate code, what in the > > > world is this doing? Why not use the in-kernel compiler.h instead? Why > > > are you copying loads of .h files into tools/virtio/? What is this for > > > and why not just use the real files so you don't have to even attempt to > > > try to keep things in sync (hint, they will always be out of sync.) > > Because it's doing a very weird hack: building some kernel > > code into a userspace binary, used for just for testing. > > This is all not part of kernel build intentionally, no > > one is supposed to use this binary in production, but > > it was helpful in finding bugs in virtio core in the past > > so I keep it around. > Would it be possible to switch to in-kernel tests, such as KUnit? If it's > possible, we could use the kernel headers and implementations as they are, > and we could address the concerns that Greg raised I think. > > > Best regards, > > Shunsuke I think your patch is fine as is. Using kunit is certainly possible, but won't be a small project. tools/virtio was always a quick hack to help experiment quickly, so I'm not worried by it being broken often - whoever wants to play with it next, fixes it up. -- MST _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization