mbox series

[net-next,0/2] refactor the ringtest testing for ptr_ring

Message ID 1625457455-4667-1-git-send-email-linyunsheng@huawei.com (mailing list archive)
Headers show
Series refactor the ringtest testing for ptr_ring | expand

Message

Yunsheng Lin July 5, 2021, 3:57 a.m. UTC
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

Comments

Andy Shevchenko July 5, 2021, 9:56 a.m. UTC | #1
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.

...

>  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

Maybe somebody can change this to be able to include in-tree headers directly?

Besides above, had you tested this with `make O=...`?
Yunsheng Lin July 5, 2021, 12:06 p.m. UTC | #2
On 2021/7/5 17:56, Andy Shevchenko wrote:
> 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.
> 
> ...
> 
>>  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
> 
> 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?

>
Andy Shevchenko July 5, 2021, 2:57 p.m. UTC | #3
On Mon, Jul 05, 2021 at 08:06:50PM +0800, Yunsheng Lin wrote:
> On 2021/7/5 17:56, Andy Shevchenko wrote:
> > 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.
> > 
> > ...
> > 
> >>  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
> > 
> > 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.

> > 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.
Michael S. Tsirkin July 5, 2021, 6:26 p.m. UTC | #4
On Mon, Jul 05, 2021 at 08:06:50PM +0800, Yunsheng Lin wrote:
> On 2021/7/5 17:56, Andy Shevchenko wrote:
> > 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.
> > 
> > ...
> > 
> >>  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
> > 
> > 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.


> > 
> > 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?
> 
> >
Andy Shevchenko July 5, 2021, 6:36 p.m. UTC | #5
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:
> > On 2021/7/5 17:56, Andy Shevchenko wrote:
> > > 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.
> > > 
> > > ...
> > > 
> > >>  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
> > > 
> > > 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.

> > > 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?
Michael S. Tsirkin July 5, 2021, 6:42 p.m. UTC | #6
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:
> > On Mon, Jul 05, 2021 at 08:06:50PM +0800, Yunsheng Lin wrote:
> > > On 2021/7/5 17:56, Andy Shevchenko wrote:
> > > > 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.
> > > > 
> > > > ...
> > > > 
> > > >>  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
> > > > 
> > > > 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.

> > > > 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?
> 
> -- 
> With Best Regards,
> Andy Shevchenko
>
Andy Shevchenko July 5, 2021, 7:05 p.m. UTC | #7
On Mon, Jul 5, 2021 at 9:45 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> 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:
> > > On Mon, Jul 05, 2021 at 08:06:50PM +0800, Yunsheng Lin wrote:
> > > > On 2021/7/5 17:56, Andy Shevchenko wrote:
> > > > > 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.
> > > > >
> > > > > ...
> > > > >
> > > > >>  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
> > > > >
> > > > > 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.
>
> > > > > 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?
Michael S. Tsirkin July 5, 2021, 7:31 p.m. UTC | #8
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 <mst@redhat.com> wrote:
> >
> > 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:
> > > > On Mon, Jul 05, 2021 at 08:06:50PM +0800, Yunsheng Lin wrote:
> > > > > On 2021/7/5 17:56, Andy Shevchenko wrote:
> > > > > > 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.
> > > > > >
> > > > > > ...
> > > > > >
> > > > > >>  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
> > > > > >
> > > > > > 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?

> > I would say copies are kind of okay just make sure they are
> > built with kconfig. Then any breakage will be
> > detected.
> >
> > > > > > 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?
> 
> 
> -- 
> With Best Regards,
> Andy Shevchenko
Yunsheng Lin July 6, 2021, 1:35 a.m. UTC | #9
On 2021/7/6 3:05, Andy Shevchenko wrote:
> On Mon, Jul 5, 2021 at 9:45 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>> 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:
>>>> On Mon, Jul 05, 2021 at 08:06:50PM +0800, Yunsheng Lin wrote:
>>>>> On 2021/7/5 17:56, Andy Shevchenko wrote:
>>>>>> 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.
>>>>>>
>>>>>> ...
>>>>>>
>>>>>>>  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
>>>>>>
>>>>>> 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.

> 
>> I would say copies are kind of okay just make sure they are
>> built with kconfig. Then any breakage will be
>> detected.
>>
>>>>>> 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?
> 
>