From: Yunsheng Lin <hidden> Date: 2021-07-05 03:58:23
tools/include/* have a lot of abstract layer for building
kernel code from userspace, so reuse or add the abstract
layer in tools/include/ to build the ptr_ring for ringtest
testing.
The same abstract layer can be used to build the ptr_ring
for ptr_ring benchmark app too, see [1].
1. https://lkml.org/lkml/2021/7/1/275
Yunsheng Lin (2):
tools: add missing infrastructure for building ptr_ring.h
tools/virtio: use common infrastructure to build ptr_ring.h
tools/include/asm/cache.h | 56 ++++++++++++++++++++
tools/include/asm/processor.h | 36 +++++++++++++
tools/include/generated/autoconf.h | 1 +
tools/include/linux/align.h | 15 ++++++
tools/include/linux/cache.h | 87 +++++++++++++++++++++++++++++++
tools/include/linux/gfp.h | 4 ++
tools/include/linux/slab.h | 46 +++++++++++++++++
tools/include/linux/spinlock.h | 2 -
tools/virtio/ringtest/Makefile | 2 +-
tools/virtio/ringtest/main.h | 100 +++---------------------------------
tools/virtio/ringtest/ptr_ring.c | 102 ++-----------------------------------
11 files changed, 257 insertions(+), 194 deletions(-)
create mode 100644 tools/include/asm/cache.h
create mode 100644 tools/include/asm/processor.h
create mode 100644 tools/include/generated/autoconf.h
create mode 100644 tools/include/linux/align.h
create mode 100644 tools/include/linux/cache.h
create mode 100644 tools/include/linux/slab.h
--
2.7.4
From: Yunsheng Lin <hidden> Date: 2021-07-05 03:58:16
Use the common infrastructure in tools/include to build
ptr_ring.h in user space.
Signed-off-by: Yunsheng Lin <redacted>
---
tools/virtio/ringtest/Makefile | 2 +-
tools/virtio/ringtest/main.h | 100 +++-----------------------------------
tools/virtio/ringtest/ptr_ring.c | 102 ++-------------------------------------
3 files changed, 12 insertions(+), 192 deletions(-)
@@ -87,18 +94,6 @@ void wait_for_call(void);externunsignedring_size;-/* Compiler barrier - similar to what Linux uses */-#define barrier() asm volatile("" ::: "memory")--/* Is there a portable way to do this? */-#if defined(__x86_64__) || defined(__i386__)-#define cpu_relax() asm ("rep; nop" ::: "memory")-#elif defined(__s390x__)-#define cpu_relax() barrier()-#else-#define cpu_relax() assert(0)-#endif-externbooldo_relax;staticinlinevoidbusy_wait(void)
From: Yunsheng Lin <hidden> Date: 2021-07-05 03:58:19
In order to build ptr_ring.h in userspace, the cacheline
aligning, cpu_relax() and slab related infrastructure is
needed, so add them in this patch.
As L1_CACHE_BYTES may be different for different arch, which
is mostly defined in include/generated/autoconf.h, so user may
need to do "make defconfig" before building a tool using the
API in linux/cache.h.
Also "linux/lockdep.h" is not added in "tools/include" yet,
so remove it in "linux/spinlock.h", and the only place using
"linux/spinlock.h" is tools/testing/radix-tree, removing that
does not break radix-tree testing.
Signed-off-by: Yunsheng Lin <redacted>
---
tools/include/asm/cache.h | 56 ++++++++++++++++++++++++
tools/include/asm/processor.h | 36 ++++++++++++++++
tools/include/generated/autoconf.h | 1 +
tools/include/linux/align.h | 15 +++++++
tools/include/linux/cache.h | 87 ++++++++++++++++++++++++++++++++++++++
tools/include/linux/gfp.h | 4 ++
tools/include/linux/slab.h | 46 ++++++++++++++++++++
tools/include/linux/spinlock.h | 2 -
8 files changed, 245 insertions(+), 2 deletions(-)
create mode 100644 tools/include/asm/cache.h
create mode 100644 tools/include/asm/processor.h
create mode 100644 tools/include/generated/autoconf.h
create mode 100644 tools/include/linux/align.h
create mode 100644 tools/include/linux/cache.h
create mode 100644 tools/include/linux/slab.h
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Date: 2021-07-05 09:56:47
On Mon, Jul 05, 2021 at 11:57:33AM +0800, Yunsheng Lin wrote:
tools/include/* have a lot of abstract layer for building
kernel code from userspace, so reuse or add the abstract
layer in tools/include/ to build the ptr_ring for ringtest
testing.
Maybe somebody can change this to be able to include in-tree headers directly?
Besides above, had you tested this with `make O=...`?
--
With Best Regards,
Andy Shevchenko
From: Yunsheng Lin <hidden> Date: 2021-07-05 12:06:56
On 2021/7/5 17:56, Andy Shevchenko wrote:
On Mon, Jul 05, 2021 at 11:57:33AM +0800, Yunsheng Lin wrote:
quoted
tools/include/* have a lot of abstract layer for building
kernel code from userspace, so reuse or add the abstract
layer in tools/include/ to build the ptr_ring for ringtest
testing.
Maybe somebody can change this to be able to include in-tree headers directly?
If the above works, maybe the files in tools/include/* is not
necessary any more, just use the in-tree headers to compile
the user space app?
Or I missed something here?
Besides above, had you tested this with `make O=...`?
You are right, the generated/autoconf.h is in another directory
with `make O=...`.
Any nice idea to fix the above problem?
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Date: 2021-07-05 14:57:41
On Mon, Jul 05, 2021 at 08:06:50PM +0800, Yunsheng Lin wrote:
On 2021/7/5 17:56, Andy Shevchenko wrote:
quoted
On Mon, Jul 05, 2021 at 11:57:33AM +0800, Yunsheng Lin wrote:
quoted
tools/include/* have a lot of abstract layer for building
kernel code from userspace, so reuse or add the abstract
layer in tools/include/ to build the ptr_ring for ringtest
testing.
Maybe somebody can change this to be able to include in-tree headers directly?
If the above works, maybe the files in tools/include/* is not
necessary any more, just use the in-tree headers to compile
the user space app?
Or I missed something here?
I don't know if it works or not or why that decision had been made to
copy'n'paste headers (Yes, I know they have some modifications).
Somebody needs to check that and see what can be done in order to avoid copying
entire include into tools/include.
quoted
Besides above, had you tested this with `make O=...`?
You are right, the generated/autoconf.h is in another directory
with `make O=...`.
Any nice idea to fix the above problem?
No idea. But I consider breakage of O= is a show stopper.
--
With Best Regards,
Andy Shevchenko
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2021-07-05 18:26:42
On Mon, Jul 05, 2021 at 08:06:50PM +0800, Yunsheng Lin wrote:
On 2021/7/5 17:56, Andy Shevchenko wrote:
quoted
On Mon, Jul 05, 2021 at 11:57:33AM +0800, Yunsheng Lin wrote:
quoted
tools/include/* have a lot of abstract layer for building
kernel code from userspace, so reuse or add the abstract
layer in tools/include/ to build the ptr_ring for ringtest
testing.
Maybe somebody can change this to be able to include in-tree headers directly?
If the above works, maybe the files in tools/include/* is not
necessary any more, just use the in-tree headers to compile
the user space app?
Or I missed something here?
why would it work? kernel headers outside of uapi are not
intended to be consumed by userspace.
quoted
Besides above, had you tested this with `make O=...`?
You are right, the generated/autoconf.h is in another directory
with `make O=...`.
Any nice idea to fix the above problem?
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Date: 2021-07-05 18:36:38
On Mon, Jul 05, 2021 at 02:26:32PM -0400, Michael S. Tsirkin wrote:
On Mon, Jul 05, 2021 at 08:06:50PM +0800, Yunsheng Lin wrote:
quoted
On 2021/7/5 17:56, Andy Shevchenko wrote:
quoted
On Mon, Jul 05, 2021 at 11:57:33AM +0800, Yunsheng Lin wrote:
quoted
tools/include/* have a lot of abstract layer for building
kernel code from userspace, so reuse or add the abstract
layer in tools/include/ to build the ptr_ring for ringtest
testing.
Maybe somebody can change this to be able to include in-tree headers directly?
If the above works, maybe the files in tools/include/* is not
necessary any more, just use the in-tree headers to compile
the user space app?
Or I missed something here?
why would it work? kernel headers outside of uapi are not
intended to be consumed by userspace.
The problem here, that we are almost getting two copies of the headers, and
tools are not in a good maintenance, so it's often desynchronized from the
actual Linux headers. This will become more and more diverse if we keep same
way of operation. So, I would rather NAK any new copies of the headers from
include/ to tools/include.
quoted
quoted
Besides above, had you tested this with `make O=...`?
You are right, the generated/autoconf.h is in another directory
with `make O=...`.
Any nice idea to fix the above problem?
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2021-07-05 18:39:36
On Mon, Jul 05, 2021 at 11:57:34AM +0800, Yunsheng Lin wrote:
In order to build ptr_ring.h in userspace, the cacheline
aligning, cpu_relax() and slab related infrastructure is
needed, so add them in this patch.
As L1_CACHE_BYTES may be different for different arch, which
is mostly defined in include/generated/autoconf.h, so user may
need to do "make defconfig" before building a tool using the
API in linux/cache.h.
Also "linux/lockdep.h" is not added in "tools/include" yet,
so remove it in "linux/spinlock.h", and the only place using
"linux/spinlock.h" is tools/testing/radix-tree, removing that
does not break radix-tree testing.
Signed-off-by: Yunsheng Lin <redacted>
This is hard to review.
Try to split this please. Functional changes separate from
merely moving code around.
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2021-07-05 18:42:29
On Mon, Jul 05, 2021 at 09:36:26PM +0300, Andy Shevchenko wrote:
On Mon, Jul 05, 2021 at 02:26:32PM -0400, Michael S. Tsirkin wrote:
quoted
On Mon, Jul 05, 2021 at 08:06:50PM +0800, Yunsheng Lin wrote:
quoted
On 2021/7/5 17:56, Andy Shevchenko wrote:
quoted
On Mon, Jul 05, 2021 at 11:57:33AM +0800, Yunsheng Lin wrote:
quoted
tools/include/* have a lot of abstract layer for building
kernel code from userspace, so reuse or add the abstract
layer in tools/include/ to build the ptr_ring for ringtest
testing.
Maybe somebody can change this to be able to include in-tree headers directly?
If the above works, maybe the files in tools/include/* is not
necessary any more, just use the in-tree headers to compile
the user space app?
Or I missed something here?
why would it work? kernel headers outside of uapi are not
intended to be consumed by userspace.
The problem here, that we are almost getting two copies of the headers, and
tools are not in a good maintenance, so it's often desynchronized from the
actual Linux headers. This will become more and more diverse if we keep same
way of operation. So, I would rather NAK any new copies of the headers from
include/ to tools/include.
We already have the copies
yes they are not maintained well ... what's the plan then?
NAK won't help us improve the situation.
I would say copies are kind of okay just make sure they are
built with kconfig. Then any breakage will be
detected.
quoted
quoted
quoted
Besides above, had you tested this with `make O=...`?
You are right, the generated/autoconf.h is in another directory
with `make O=...`.
Any nice idea to fix the above problem?
From: Andy Shevchenko <hidden> Date: 2021-07-05 19:06:11
On Mon, Jul 5, 2021 at 9:45 PM Michael S. Tsirkin [off-list ref] wrote:
On Mon, Jul 05, 2021 at 09:36:26PM +0300, Andy Shevchenko wrote:
quoted
On Mon, Jul 05, 2021 at 02:26:32PM -0400, Michael S. Tsirkin wrote:
quoted
On Mon, Jul 05, 2021 at 08:06:50PM +0800, Yunsheng Lin wrote:
quoted
On 2021/7/5 17:56, Andy Shevchenko wrote:
quoted
On Mon, Jul 05, 2021 at 11:57:33AM +0800, Yunsheng Lin wrote:
quoted
tools/include/* have a lot of abstract layer for building
kernel code from userspace, so reuse or add the abstract
layer in tools/include/ to build the ptr_ring for ringtest
testing.
Maybe somebody can change this to be able to include in-tree headers directly?
If the above works, maybe the files in tools/include/* is not
necessary any more, just use the in-tree headers to compile
the user space app?
Or I missed something here?
why would it work? kernel headers outside of uapi are not
intended to be consumed by userspace.
The problem here, that we are almost getting two copies of the headers, and
tools are not in a good maintenance, so it's often desynchronized from the
actual Linux headers. This will become more and more diverse if we keep same
way of operation. So, I would rather NAK any new copies of the headers from
include/ to tools/include.
We already have the copies
yes they are not maintained well ... what's the plan then?
NAK won't help us improve the situation.
I understand and the proposal is to leave only the files which are not
the same (can we do kinda wrappers or so in tools/include rather than
copying everything?).
I would say copies are kind of okay just make sure they are
built with kconfig. Then any breakage will be
detected.
quoted
quoted
quoted
quoted
Besides above, had you tested this with `make O=...`?
You are right, the generated/autoconf.h is in another directory
with `make O=...`.
Any nice idea to fix the above problem?
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2021-07-05 19:32:09
On Mon, Jul 05, 2021 at 10:05:30PM +0300, Andy Shevchenko wrote:
On Mon, Jul 5, 2021 at 9:45 PM Michael S. Tsirkin [off-list ref] wrote:
quoted
On Mon, Jul 05, 2021 at 09:36:26PM +0300, Andy Shevchenko wrote:
quoted
On Mon, Jul 05, 2021 at 02:26:32PM -0400, Michael S. Tsirkin wrote:
quoted
On Mon, Jul 05, 2021 at 08:06:50PM +0800, Yunsheng Lin wrote:
quoted
On 2021/7/5 17:56, Andy Shevchenko wrote:
quoted
On Mon, Jul 05, 2021 at 11:57:33AM +0800, Yunsheng Lin wrote:
quoted
tools/include/* have a lot of abstract layer for building
kernel code from userspace, so reuse or add the abstract
layer in tools/include/ to build the ptr_ring for ringtest
testing.
Maybe somebody can change this to be able to include in-tree headers directly?
If the above works, maybe the files in tools/include/* is not
necessary any more, just use the in-tree headers to compile
the user space app?
Or I missed something here?
why would it work? kernel headers outside of uapi are not
intended to be consumed by userspace.
The problem here, that we are almost getting two copies of the headers, and
tools are not in a good maintenance, so it's often desynchronized from the
actual Linux headers. This will become more and more diverse if we keep same
way of operation. So, I would rather NAK any new copies of the headers from
include/ to tools/include.
We already have the copies
yes they are not maintained well ... what's the plan then?
NAK won't help us improve the situation.
I understand and the proposal is to leave only the files which are not
the same (can we do kinda wrappers or so in tools/include rather than
copying everything?).
I have no idea how we'd do all this. When I did tools/virtio I already
tried to minimize copying. Want to try to do better?
quoted
I would say copies are kind of okay just make sure they are
built with kconfig. Then any breakage will be
detected.
quoted
quoted
quoted
quoted
Besides above, had you tested this with `make O=...`?
You are right, the generated/autoconf.h is in another directory
with `make O=...`.
Any nice idea to fix the above problem?
From: Yunsheng Lin <hidden> Date: 2021-07-06 01:36:44
On 2021/7/6 3:05, Andy Shevchenko wrote:
On Mon, Jul 5, 2021 at 9:45 PM Michael S. Tsirkin [off-list ref] wrote:
quoted
On Mon, Jul 05, 2021 at 09:36:26PM +0300, Andy Shevchenko wrote:
quoted
On Mon, Jul 05, 2021 at 02:26:32PM -0400, Michael S. Tsirkin wrote:
quoted
On Mon, Jul 05, 2021 at 08:06:50PM +0800, Yunsheng Lin wrote:
quoted
On 2021/7/5 17:56, Andy Shevchenko wrote:
quoted
On Mon, Jul 05, 2021 at 11:57:33AM +0800, Yunsheng Lin wrote:
quoted
tools/include/* have a lot of abstract layer for building
kernel code from userspace, so reuse or add the abstract
layer in tools/include/ to build the ptr_ring for ringtest
testing.
Maybe somebody can change this to be able to include in-tree headers directly?
If the above works, maybe the files in tools/include/* is not
necessary any more, just use the in-tree headers to compile
the user space app?
Or I missed something here?
why would it work? kernel headers outside of uapi are not
intended to be consumed by userspace.
The problem here, that we are almost getting two copies of the headers, and
tools are not in a good maintenance, so it's often desynchronized from the
actual Linux headers. This will become more and more diverse if we keep same
way of operation. So, I would rather NAK any new copies of the headers from
include/ to tools/include.
We already have the copies
yes they are not maintained well ... what's the plan then?
NAK won't help us improve the situation.
I understand and the proposal is to leave only the files which are not
the same (can we do kinda wrappers or so in tools/include rather than
copying everything?).
I am not sure the proposal is the right direction.
As mentioned by Michael, kernel headers outside of uapi are not
intended to be consumed by userspace, so those header might be
changed without considering of the code using them in tools/,
using the wrappers might cause more breaking of tools/.
And grepping through the tools/include does not seems to be
a lot of wrapper(only some low level asm include file like
tools/include/asm/barrier.h has the wrapper, which is supposed
not to be changed very often?)
so using wrappers does not seem to be the best choice here.
quoted
I would say copies are kind of okay just make sure they are
built with kconfig. Then any breakage will be
detected.
quoted
quoted
quoted
quoted
Besides above, had you tested this with `make O=...`?
You are right, the generated/autoconf.h is in another directory
with `make O=...`.
Any nice idea to fix the above problem?
From: Yunsheng Lin <hidden> Date: 2021-07-06 02:06:08
On 2021/7/6 2:39, Michael S. Tsirkin wrote:
On Mon, Jul 05, 2021 at 11:57:34AM +0800, Yunsheng Lin wrote:
quoted
In order to build ptr_ring.h in userspace, the cacheline
aligning, cpu_relax() and slab related infrastructure is
needed, so add them in this patch.
As L1_CACHE_BYTES may be different for different arch, which
is mostly defined in include/generated/autoconf.h, so user may
need to do "make defconfig" before building a tool using the
API in linux/cache.h.
Also "linux/lockdep.h" is not added in "tools/include" yet,
so remove it in "linux/spinlock.h", and the only place using
"linux/spinlock.h" is tools/testing/radix-tree, removing that
does not break radix-tree testing.
Signed-off-by: Yunsheng Lin <redacted>
This is hard to review.
Try to split this please. Functional changes separate from
merely moving code around.
Does this have a chance to work outside of kernel?
I am not sure I understand what you meant here.
sched_yield() is a pthread API, so it should work in the
user space.
And it allow the rigntest to compile when it is built on
the arch which is not handled as above.
quoted
+#endif
did you actually test or even test build all these arches?
Not sure we need to bother with hacks like these.
Only x86_64 and arm64 arches have been built and tested.
This is added referring the tools/include/asm/barrier.h.
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2021-07-18 02:09:48
On Tue, Jul 06, 2021 at 10:04:02AM +0800, Yunsheng Lin wrote:
On 2021/7/6 2:39, Michael S. Tsirkin wrote:
quoted
On Mon, Jul 05, 2021 at 11:57:34AM +0800, Yunsheng Lin wrote:
quoted
In order to build ptr_ring.h in userspace, the cacheline
aligning, cpu_relax() and slab related infrastructure is
needed, so add them in this patch.
As L1_CACHE_BYTES may be different for different arch, which
is mostly defined in include/generated/autoconf.h, so user may
need to do "make defconfig" before building a tool using the
API in linux/cache.h.
Also "linux/lockdep.h" is not added in "tools/include" yet,
so remove it in "linux/spinlock.h", and the only place using
"linux/spinlock.h" is tools/testing/radix-tree, removing that
does not break radix-tree testing.
Signed-off-by: Yunsheng Lin <redacted>
This is hard to review.
Try to split this please. Functional changes separate from
merely moving code around.
Does this have a chance to work outside of kernel?
I am not sure I understand what you meant here.
sched_yield() is a pthread API, so it should work in the
user space.
And it allow the rigntest to compile when it is built on
the arch which is not handled as above.
It might compile but is likely too heavy to behave
reasonably.
Also, given you did not actually test it I don't
think you should add such arch code.
Note you broke at least s390 here:
../../arch/s390/include/vdso/processor.h
does not actually exist. Where these headers
do exit they tend to include lots of code which won't
build out of kernel.
All this is just for cpu_relax - open coding that seems way easier.
quoted
quoted
+#endif
did you actually test or even test build all these arches?
Not sure we need to bother with hacks like these.
Only x86_64 and arm64 arches have been built and tested.
In that case I think you should not add code that you
have not even built let alone tested.
This is added referring the tools/include/asm/barrier.h.
Does this have a chance to work outside of kernel?
I am not sure I understand what you meant here.
sched_yield() is a pthread API, so it should work in the
user space.
And it allow the rigntest to compile when it is built on
the arch which is not handled as above.
It might compile but is likely too heavy to behave
reasonably.
Also, given you did not actually test it I don't
think you should add such arch code.
Note you broke at least s390 here:
../../arch/s390/include/vdso/processor.h
does not actually exist. Where these headers
do exit they tend to include lots of code which won't
build out of kernel.
You are right, it should be in:
../../arch/s390/include/asm/vdso/processor.h
All this is just for cpu_relax - open coding that seems way easier.
Sure.
As Eugenio has posted a patchset to fix the compilation, which does
not seems to be merged yet and may have some merging conflicts with
this patchset, so either wait for the Eugenio' patchset to be merged
before proceeding with this patchset, or explicitly note the dependency
of Eugenio' patchset when sending the new version of patchset. I am not
familiar with the merging flow of virtio to say which way is better, any
suggestion how to proceed with this patchset?
1. https://lkml.org/lkml/2021/7/6/1132
quoted
quoted
quoted
+#endif
did you actually test or even test build all these arches?
Not sure we need to bother with hacks like these.
Only x86_64 and arm64 arches have been built and tested.
In that case I think you should not add code that you
have not even built let alone tested.
Ok.
quoted
This is added referring the tools/include/asm/barrier.h.
Does this have a chance to work outside of kernel?
I am not sure I understand what you meant here.
sched_yield() is a pthread API, so it should work in the
user space.
And it allow the rigntest to compile when it is built on
the arch which is not handled as above.
It might compile but is likely too heavy to behave
reasonably.
Also, given you did not actually test it I don't
think you should add such arch code.
Note you broke at least s390 here:
../../arch/s390/include/vdso/processor.h
does not actually exist. Where these headers
do exit they tend to include lots of code which won't
build out of kernel.
You are right, it should be in:
../../arch/s390/include/asm/vdso/processor.h
quoted
All this is just for cpu_relax - open coding that seems way easier.
Sure.
As Eugenio has posted a patchset to fix the compilation, which does
not seems to be merged yet and may have some merging conflicts with
this patchset, so either wait for the Eugenio' patchset to be merged
before proceeding with this patchset, or explicitly note the dependency
of Eugenio' patchset when sending the new version of patchset. I am not
familiar with the merging flow of virtio to say which way is better, any
suggestion how to proceed with this patchset?
1. https://lkml.org/lkml/2021/7/6/1132
quoted
quoted
quoted
quoted
+#endif
did you actually test or even test build all these arches?
Not sure we need to bother with hacks like these.
Only x86_64 and arm64 arches have been built and tested.
In that case I think you should not add code that you
have not even built let alone tested.
Ok.
quoted
quoted
This is added referring the tools/include/asm/barrier.h.