Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Yunsheng Lin <linyunsheng@huawei.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: davem@davemloft.net, kuba@kernel.org, jasowang@redhat.com,
	nickhu@andestech.com, green.hu@gmail.com, deanbo422@gmail.com,
	akpm@linux-foundation.org, yury.norov@gmail.com,
	andriy.shevchenko@linux.intel.com, ojeda@kernel.org,
	ndesaulniers@gooogle.com, joe@perches.com,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	netdev@vger.kernel.org, "Eugenio Pérez" <eperezma@redhat.com>
Subject: Re: [PATCH net-next 1/2] tools: add missing infrastructure for building ptr_ring.h
Date: Mon, 19 Jul 2021 09:40:39 +0800	[thread overview]
Message-ID: <5d320b37-18f3-e853-ceb7-21af7ca12763@huawei.com> (raw)
In-Reply-To: <20210717220239-mutt-send-email-mst@kernel.org>

On 2021/7/18 10:09, Michael S. Tsirkin wrote:
> On Tue, Jul 06, 2021 at 10:04:02AM +0800, Yunsheng Lin wrote:
>> On 2021/7/6 2:39, Michael S. Tsirkin wrote:
>>> On Mon, Jul 05, 2021 at 11:57:34AM +0800, Yunsheng Lin wrote:

[..]

>>>> diff --git a/tools/include/asm/processor.h b/tools/include/asm/processor.h
>>>> new file mode 100644
>>>> index 0000000..3198ad6
>>>> --- /dev/null
>>>> +++ b/tools/include/asm/processor.h
>>>> @@ -0,0 +1,36 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>> +
>>>> +#ifndef __TOOLS_LINUX_ASM_PROCESSOR_H
>>>> +#define __TOOLS_LINUX_ASM_PROCESSOR_H
>>>> +
>>>> +#include <pthread.h>
>>>> +
>>>> +#if defined(__i386__) || defined(__x86_64__)
>>>> +#include "../../arch/x86/include/asm/vdso/processor.h"
>>>> +#elif defined(__arm__)
>>>> +#include "../../arch/arm/include/asm/vdso/processor.h"
>>>> +#elif defined(__aarch64__)
>>>> +#include "../../arch/arm64/include/asm/vdso/processor.h"
>>>> +#elif defined(__powerpc__)
>>>> +#include "../../arch/powerpc/include/vdso/processor.h"
>>>> +#elif defined(__s390__)
>>>> +#include "../../arch/s390/include/vdso/processor.h"
>>>> +#elif defined(__sh__)
>>>> +#include "../../arch/sh/include/asm/processor.h"
>>>> +#elif defined(__sparc__)
>>>> +#include "../../arch/sparc/include/asm/processor.h"
>>>> +#elif defined(__alpha__)
>>>> +#include "../../arch/alpha/include/asm/processor.h"
>>>> +#elif defined(__mips__)
>>>> +#include "../../arch/mips/include/asm/vdso/processor.h"
>>>> +#elif defined(__ia64__)
>>>> +#include "../../arch/ia64/include/asm/processor.h"
>>>> +#elif defined(__xtensa__)
>>>> +#include "../../arch/xtensa/include/asm/processor.h"
>>>> +#elif defined(__nds32__)
>>>> +#include "../../arch/nds32/include/asm/processor.h"
>>>> +#else
>>>> +#define cpu_relax()	sched_yield()
>>>
>>> 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

> 
> 
>>>
>>>> +#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.

> 
> 
>> This is added referring the tools/include/asm/barrier.h.
>>
>>>
>>>
>>>> +
> 
> .
> 

  reply	other threads:[~2021-07-19  1:40 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-05  3:57 [PATCH net-next 0/2] refactor the ringtest testing for ptr_ring Yunsheng Lin
2021-07-05  3:57 ` [PATCH net-next 1/2] tools: add missing infrastructure for building ptr_ring.h Yunsheng Lin
2021-07-05 18:39   ` Michael S. Tsirkin
2021-07-06  2:04     ` Yunsheng Lin
2021-07-18  2:09       ` Michael S. Tsirkin
2021-07-19  1:40         ` Yunsheng Lin [this message]
2021-07-19 11:58           ` Michael S. Tsirkin
2021-07-05  3:57 ` [PATCH net-next 2/2] tools/virtio: use common infrastructure to build ptr_ring.h Yunsheng Lin
2021-07-05  9:56 ` [PATCH net-next 0/2] refactor the ringtest testing for ptr_ring Andy Shevchenko
2021-07-05 12:06   ` Yunsheng Lin
2021-07-05 14:57     ` Andy Shevchenko
2021-07-05 18:26     ` Michael S. Tsirkin
2021-07-05 18:36       ` Andy Shevchenko
2021-07-05 18:42         ` Michael S. Tsirkin
2021-07-05 19:05           ` Andy Shevchenko
2021-07-05 19:31             ` Michael S. Tsirkin
2021-07-06  1:35             ` Yunsheng Lin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5d320b37-18f3-e853-ceb7-21af7ca12763@huawei.com \
    --to=linyunsheng@huawei.com \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=davem@davemloft.net \
    --cc=deanbo422@gmail.com \
    --cc=eperezma@redhat.com \
    --cc=green.hu@gmail.com \
    --cc=jasowang@redhat.com \
    --cc=joe@perches.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=ndesaulniers@gooogle.com \
    --cc=netdev@vger.kernel.org \
    --cc=nickhu@andestech.com \
    --cc=ojeda@kernel.org \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=yury.norov@gmail.com \
    --subject='Re: [PATCH net-next 1/2] tools: add missing infrastructure for building ptr_ring.h' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).