* [PATCH net-next 1/2] tools: add missing infrastructure for building ptr_ring.h
2021-07-05 3:57 [PATCH net-next 0/2] refactor the ringtest testing for ptr_ring Yunsheng Lin
@ 2021-07-05 3:57 ` Yunsheng Lin
2021-07-05 18:39 ` 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
2 siblings, 1 reply; 17+ messages in thread
From: Yunsheng Lin @ 2021-07-05 3:57 UTC (permalink / raw)
To: davem, kuba, mst, jasowang
Cc: nickhu, green.hu, deanbo422, akpm, yury.norov, andriy.shevchenko,
ojeda, ndesaulniers, joe, linux-kernel, virtualization, netdev
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 <linyunsheng@huawei.com>
---
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
diff --git a/tools/include/asm/cache.h b/tools/include/asm/cache.h
new file mode 100644
index 0000000..071e310
--- /dev/null
+++ b/tools/include/asm/cache.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __TOOLS_LINUX_ASM_CACHE_H
+#define __TOOLS_LINUX_ASM_CACHE_H
+
+#include <generated/autoconf.h>
+
+#if defined(__i386__) || defined(__x86_64__)
+#define L1_CACHE_SHIFT (CONFIG_X86_L1_CACHE_SHIFT)
+#elif defined(__arm__)
+#define L1_CACHE_SHIFT (CONFIG_ARM_L1_CACHE_SHIFT)
+#elif defined(__aarch64__)
+#define L1_CACHE_SHIFT (6)
+#elif defined(__powerpc__)
+
+/* bytes per L1 cache line */
+#if defined(CONFIG_PPC_8xx)
+#define L1_CACHE_SHIFT 4
+#elif defined(CONFIG_PPC_E500MC)
+#define L1_CACHE_SHIFT 6
+#elif defined(CONFIG_PPC32)
+#if defined(CONFIG_PPC_47x)
+#define L1_CACHE_SHIFT 7
+#else
+#define L1_CACHE_SHIFT 5
+#endif
+#else /* CONFIG_PPC64 */
+#define L1_CACHE_SHIFT 7
+#endif
+
+#elif defined(__sparc__)
+#define L1_CACHE_SHIFT 5
+#elif defined(__alpha__)
+
+#if defined(CONFIG_ALPHA_GENERIC) || defined(CONFIG_ALPHA_EV6)
+#define L1_CACHE_SHIFT 6
+#else
+/* Both EV4 and EV5 are write-through, read-allocate,
+ direct-mapped, physical.
+*/
+#define L1_CACHE_SHIFT 5
+#endif
+
+#elif defined(__mips__)
+#define L1_CACHE_SHIFT CONFIG_MIPS_L1_CACHE_SHIFT
+#elif defined(__ia64__)
+#define L1_CACHE_SHIFT CONFIG_IA64_L1_CACHE_SHIFT
+#elif defined(__nds32__)
+#define L1_CACHE_SHIFT 5
+#else
+#define L1_CACHE_SHIFT 5
+#endif
+
+#define L1_CACHE_BYTES (1 << L1_CACHE_SHIFT)
+
+#endif
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()
+#endif
+
+#endif
diff --git a/tools/include/generated/autoconf.h b/tools/include/generated/autoconf.h
new file mode 100644
index 0000000..c588a2f
--- /dev/null
+++ b/tools/include/generated/autoconf.h
@@ -0,0 +1 @@
+#include "../../../include/generated/autoconf.h"
diff --git a/tools/include/linux/align.h b/tools/include/linux/align.h
new file mode 100644
index 0000000..4e82cdf
--- /dev/null
+++ b/tools/include/linux/align.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __TOOLS_LINUX_ALIGN_H
+#define __TOOLS_LINUX_ALIGN_H
+
+#include <linux/const.h>
+
+/* @a is a power of 2 value */
+#define ALIGN(x, a) __ALIGN_KERNEL((x), (a))
+#define ALIGN_DOWN(x, a) __ALIGN_KERNEL((x) - ((a) - 1), (a))
+#define __ALIGN_MASK(x, mask) __ALIGN_KERNEL_MASK((x), (mask))
+#define PTR_ALIGN(p, a) ((typeof(p))ALIGN((unsigned long)(p), (a)))
+#define PTR_ALIGN_DOWN(p, a) ((typeof(p))ALIGN_DOWN((unsigned long)(p), (a)))
+#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
+
+#endif /* _LINUX_ALIGN_H */
diff --git a/tools/include/linux/cache.h b/tools/include/linux/cache.h
new file mode 100644
index 0000000..8f86b1b
--- /dev/null
+++ b/tools/include/linux/cache.h
@@ -0,0 +1,87 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __TOOLS_LINUX__CACHE_H
+#define __TOOLS_LINUX__CACHE_H
+
+#include <asm/cache.h>
+
+#ifndef L1_CACHE_ALIGN
+#define L1_CACHE_ALIGN(x) __ALIGN_KERNEL(x, L1_CACHE_BYTES)
+#endif
+
+#ifndef SMP_CACHE_BYTES
+#define SMP_CACHE_BYTES L1_CACHE_BYTES
+#endif
+
+/*
+ * __read_mostly is used to keep rarely changing variables out of frequently
+ * updated cachelines. Its use should be reserved for data that is used
+ * frequently in hot paths. Performance traces can help decide when to use
+ * this. You want __read_mostly data to be tightly packed, so that in the
+ * best case multiple frequently read variables for a hot path will be next
+ * to each other in order to reduce the number of cachelines needed to
+ * execute a critical path. We should be mindful and selective of its use.
+ * ie: if you're going to use it please supply a *good* justification in your
+ * commit log
+ */
+#ifndef __read_mostly
+#define __read_mostly
+#endif
+
+/*
+ * __ro_after_init is used to mark things that are read-only after init (i.e.
+ * after mark_rodata_ro() has been called). These are effectively read-only,
+ * but may get written to during init, so can't live in .rodata (via "const").
+ */
+#ifndef __ro_after_init
+#define __ro_after_init __section(".data..ro_after_init")
+#endif
+
+#ifndef ____cacheline_aligned
+#define ____cacheline_aligned __attribute__((__aligned__(SMP_CACHE_BYTES)))
+#endif
+
+#ifndef ____cacheline_aligned_in_smp
+#ifdef CONFIG_SMP
+#define ____cacheline_aligned_in_smp ____cacheline_aligned
+#else
+#define ____cacheline_aligned_in_smp
+#endif /* CONFIG_SMP */
+#endif
+
+#ifndef __cacheline_aligned
+#define __cacheline_aligned \
+ __attribute__((__aligned__(SMP_CACHE_BYTES), \
+ __section__(".data..cacheline_aligned")))
+#endif /* __cacheline_aligned */
+
+#ifndef __cacheline_aligned_in_smp
+#ifdef CONFIG_SMP
+#define __cacheline_aligned_in_smp __cacheline_aligned
+#else
+#define __cacheline_aligned_in_smp
+#endif /* CONFIG_SMP */
+#endif
+
+/*
+ * The maximum alignment needed for some critical structures
+ * These could be inter-node cacheline sizes/L3 cacheline
+ * size etc. Define this in asm/cache.h for your arch
+ */
+#ifndef INTERNODE_CACHE_SHIFT
+#define INTERNODE_CACHE_SHIFT L1_CACHE_SHIFT
+#endif
+
+#if !defined(____cacheline_internodealigned_in_smp)
+#if defined(CONFIG_SMP)
+#define ____cacheline_internodealigned_in_smp \
+ __attribute__((__aligned__(1 << (INTERNODE_CACHE_SHIFT))))
+#else
+#define ____cacheline_internodealigned_in_smp
+#endif
+#endif
+
+#ifndef CONFIG_ARCH_HAS_CACHE_LINE_SIZE
+#define cache_line_size() L1_CACHE_BYTES
+#endif
+
+#endif /* __LINUX_CACHE_H */
diff --git a/tools/include/linux/gfp.h b/tools/include/linux/gfp.h
index 2203075..d7041c0 100644
--- a/tools/include/linux/gfp.h
+++ b/tools/include/linux/gfp.h
@@ -1,4 +1,8 @@
#ifndef _TOOLS_INCLUDE_LINUX_GFP_H
#define _TOOLS_INCLUDE_LINUX_GFP_H
+#include <linux/types.h>
+
+#define __GFP_ZERO 0x100u
+
#endif /* _TOOLS_INCLUDE_LINUX_GFP_H */
diff --git a/tools/include/linux/slab.h b/tools/include/linux/slab.h
new file mode 100644
index 0000000..f0b7da6
--- /dev/null
+++ b/tools/include/linux/slab.h
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __TOOLS_LINUX_SLAB_H
+#define __TOOLS_LINUX_SLAB_H
+
+#include <linux/gfp.h>
+#include <linux/cache.h>
+
+static inline void *kmalloc(size_t size, gfp_t gfp)
+{
+ void *p;
+
+ p = memalign(SMP_CACHE_BYTES, size);
+ if (!p)
+ return p;
+
+ if (gfp & __GFP_ZERO)
+ memset(p, 0, size);
+
+ return p;
+}
+
+static inline void *kzalloc(size_t size, gfp_t flags)
+{
+ return kmalloc(size, flags | __GFP_ZERO);
+}
+
+static inline void *kmalloc_array(size_t n, size_t size, gfp_t flags)
+{
+ return kmalloc(n * size, flags);
+}
+
+static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
+{
+ return kmalloc_array(n, size, flags | __GFP_ZERO);
+}
+
+static inline void kfree(void *p)
+{
+ free(p);
+}
+
+#define kvmalloc_array kmalloc_array
+#define kvfree kfree
+#define KMALLOC_MAX_SIZE SIZE_MAX
+
+#endif
diff --git a/tools/include/linux/spinlock.h b/tools/include/linux/spinlock.h
index c934572..622266b 100644
--- a/tools/include/linux/spinlock.h
+++ b/tools/include/linux/spinlock.h
@@ -37,6 +37,4 @@ static inline bool arch_spin_is_locked(arch_spinlock_t *mutex)
return true;
}
-#include <linux/lockdep.h>
-
#endif
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH net-next 1/2] tools: add missing infrastructure for building ptr_ring.h
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
0 siblings, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2021-07-05 18:39 UTC (permalink / raw)
To: Yunsheng Lin
Cc: davem, kuba, jasowang, nickhu, green.hu, deanbo422, akpm,
yury.norov, andriy.shevchenko, ojeda, ndesaulniers, joe,
linux-kernel, virtualization, netdev
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 <linyunsheng@huawei.com>
This is hard to review.
Try to split this please. Functional changes separate from
merely moving code around.
> ---
> 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
>
> diff --git a/tools/include/asm/cache.h b/tools/include/asm/cache.h
> new file mode 100644
> index 0000000..071e310
> --- /dev/null
> +++ b/tools/include/asm/cache.h
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __TOOLS_LINUX_ASM_CACHE_H
> +#define __TOOLS_LINUX_ASM_CACHE_H
> +
> +#include <generated/autoconf.h>
> +
> +#if defined(__i386__) || defined(__x86_64__)
> +#define L1_CACHE_SHIFT (CONFIG_X86_L1_CACHE_SHIFT)
> +#elif defined(__arm__)
> +#define L1_CACHE_SHIFT (CONFIG_ARM_L1_CACHE_SHIFT)
> +#elif defined(__aarch64__)
> +#define L1_CACHE_SHIFT (6)
> +#elif defined(__powerpc__)
> +
> +/* bytes per L1 cache line */
> +#if defined(CONFIG_PPC_8xx)
> +#define L1_CACHE_SHIFT 4
> +#elif defined(CONFIG_PPC_E500MC)
> +#define L1_CACHE_SHIFT 6
> +#elif defined(CONFIG_PPC32)
> +#if defined(CONFIG_PPC_47x)
> +#define L1_CACHE_SHIFT 7
> +#else
> +#define L1_CACHE_SHIFT 5
> +#endif
> +#else /* CONFIG_PPC64 */
> +#define L1_CACHE_SHIFT 7
> +#endif
> +
> +#elif defined(__sparc__)
> +#define L1_CACHE_SHIFT 5
> +#elif defined(__alpha__)
> +
> +#if defined(CONFIG_ALPHA_GENERIC) || defined(CONFIG_ALPHA_EV6)
> +#define L1_CACHE_SHIFT 6
> +#else
> +/* Both EV4 and EV5 are write-through, read-allocate,
> + direct-mapped, physical.
> +*/
> +#define L1_CACHE_SHIFT 5
> +#endif
> +
> +#elif defined(__mips__)
> +#define L1_CACHE_SHIFT CONFIG_MIPS_L1_CACHE_SHIFT
> +#elif defined(__ia64__)
> +#define L1_CACHE_SHIFT CONFIG_IA64_L1_CACHE_SHIFT
> +#elif defined(__nds32__)
> +#define L1_CACHE_SHIFT 5
> +#else
> +#define L1_CACHE_SHIFT 5
> +#endif
> +
> +#define L1_CACHE_BYTES (1 << L1_CACHE_SHIFT)
> +
> +#endif
> 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?
> +#endif
did you actually test or even test build all these arches?
Not sure we need to bother with hacks like these.
> +
> +#endif
> diff --git a/tools/include/generated/autoconf.h b/tools/include/generated/autoconf.h
> new file mode 100644
> index 0000000..c588a2f
> --- /dev/null
> +++ b/tools/include/generated/autoconf.h
> @@ -0,0 +1 @@
> +#include "../../../include/generated/autoconf.h"
> diff --git a/tools/include/linux/align.h b/tools/include/linux/align.h
> new file mode 100644
> index 0000000..4e82cdf
> --- /dev/null
> +++ b/tools/include/linux/align.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __TOOLS_LINUX_ALIGN_H
> +#define __TOOLS_LINUX_ALIGN_H
> +
> +#include <linux/const.h>
> +
> +/* @a is a power of 2 value */
> +#define ALIGN(x, a) __ALIGN_KERNEL((x), (a))
> +#define ALIGN_DOWN(x, a) __ALIGN_KERNEL((x) - ((a) - 1), (a))
> +#define __ALIGN_MASK(x, mask) __ALIGN_KERNEL_MASK((x), (mask))
> +#define PTR_ALIGN(p, a) ((typeof(p))ALIGN((unsigned long)(p), (a)))
> +#define PTR_ALIGN_DOWN(p, a) ((typeof(p))ALIGN_DOWN((unsigned long)(p), (a)))
> +#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
> +
> +#endif /* _LINUX_ALIGN_H */
> diff --git a/tools/include/linux/cache.h b/tools/include/linux/cache.h
> new file mode 100644
> index 0000000..8f86b1b
> --- /dev/null
> +++ b/tools/include/linux/cache.h
> @@ -0,0 +1,87 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __TOOLS_LINUX__CACHE_H
> +#define __TOOLS_LINUX__CACHE_H
> +
> +#include <asm/cache.h>
> +
> +#ifndef L1_CACHE_ALIGN
> +#define L1_CACHE_ALIGN(x) __ALIGN_KERNEL(x, L1_CACHE_BYTES)
> +#endif
> +
> +#ifndef SMP_CACHE_BYTES
> +#define SMP_CACHE_BYTES L1_CACHE_BYTES
> +#endif
> +
> +/*
> + * __read_mostly is used to keep rarely changing variables out of frequently
> + * updated cachelines. Its use should be reserved for data that is used
> + * frequently in hot paths. Performance traces can help decide when to use
> + * this. You want __read_mostly data to be tightly packed, so that in the
> + * best case multiple frequently read variables for a hot path will be next
> + * to each other in order to reduce the number of cachelines needed to
> + * execute a critical path. We should be mindful and selective of its use.
> + * ie: if you're going to use it please supply a *good* justification in your
> + * commit log
> + */
> +#ifndef __read_mostly
> +#define __read_mostly
> +#endif
> +
> +/*
> + * __ro_after_init is used to mark things that are read-only after init (i.e.
> + * after mark_rodata_ro() has been called). These are effectively read-only,
> + * but may get written to during init, so can't live in .rodata (via "const").
> + */
> +#ifndef __ro_after_init
> +#define __ro_after_init __section(".data..ro_after_init")
> +#endif
> +
> +#ifndef ____cacheline_aligned
> +#define ____cacheline_aligned __attribute__((__aligned__(SMP_CACHE_BYTES)))
> +#endif
> +
> +#ifndef ____cacheline_aligned_in_smp
> +#ifdef CONFIG_SMP
> +#define ____cacheline_aligned_in_smp ____cacheline_aligned
> +#else
> +#define ____cacheline_aligned_in_smp
> +#endif /* CONFIG_SMP */
> +#endif
> +
> +#ifndef __cacheline_aligned
> +#define __cacheline_aligned \
> + __attribute__((__aligned__(SMP_CACHE_BYTES), \
> + __section__(".data..cacheline_aligned")))
> +#endif /* __cacheline_aligned */
> +
> +#ifndef __cacheline_aligned_in_smp
> +#ifdef CONFIG_SMP
> +#define __cacheline_aligned_in_smp __cacheline_aligned
> +#else
> +#define __cacheline_aligned_in_smp
> +#endif /* CONFIG_SMP */
> +#endif
> +
> +/*
> + * The maximum alignment needed for some critical structures
> + * These could be inter-node cacheline sizes/L3 cacheline
> + * size etc. Define this in asm/cache.h for your arch
> + */
> +#ifndef INTERNODE_CACHE_SHIFT
> +#define INTERNODE_CACHE_SHIFT L1_CACHE_SHIFT
> +#endif
> +
> +#if !defined(____cacheline_internodealigned_in_smp)
> +#if defined(CONFIG_SMP)
> +#define ____cacheline_internodealigned_in_smp \
> + __attribute__((__aligned__(1 << (INTERNODE_CACHE_SHIFT))))
> +#else
> +#define ____cacheline_internodealigned_in_smp
> +#endif
> +#endif
> +
> +#ifndef CONFIG_ARCH_HAS_CACHE_LINE_SIZE
> +#define cache_line_size() L1_CACHE_BYTES
> +#endif
> +
> +#endif /* __LINUX_CACHE_H */
> diff --git a/tools/include/linux/gfp.h b/tools/include/linux/gfp.h
> index 2203075..d7041c0 100644
> --- a/tools/include/linux/gfp.h
> +++ b/tools/include/linux/gfp.h
> @@ -1,4 +1,8 @@
> #ifndef _TOOLS_INCLUDE_LINUX_GFP_H
> #define _TOOLS_INCLUDE_LINUX_GFP_H
>
> +#include <linux/types.h>
> +
> +#define __GFP_ZERO 0x100u
> +
> #endif /* _TOOLS_INCLUDE_LINUX_GFP_H */
> diff --git a/tools/include/linux/slab.h b/tools/include/linux/slab.h
> new file mode 100644
> index 0000000..f0b7da6
> --- /dev/null
> +++ b/tools/include/linux/slab.h
> @@ -0,0 +1,46 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __TOOLS_LINUX_SLAB_H
> +#define __TOOLS_LINUX_SLAB_H
> +
> +#include <linux/gfp.h>
> +#include <linux/cache.h>
> +
> +static inline void *kmalloc(size_t size, gfp_t gfp)
> +{
> + void *p;
> +
> + p = memalign(SMP_CACHE_BYTES, size);
> + if (!p)
> + return p;
> +
> + if (gfp & __GFP_ZERO)
> + memset(p, 0, size);
> +
> + return p;
> +}
> +
> +static inline void *kzalloc(size_t size, gfp_t flags)
> +{
> + return kmalloc(size, flags | __GFP_ZERO);
> +}
> +
> +static inline void *kmalloc_array(size_t n, size_t size, gfp_t flags)
> +{
> + return kmalloc(n * size, flags);
> +}
> +
> +static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
> +{
> + return kmalloc_array(n, size, flags | __GFP_ZERO);
> +}
> +
> +static inline void kfree(void *p)
> +{
> + free(p);
> +}
> +
> +#define kvmalloc_array kmalloc_array
> +#define kvfree kfree
> +#define KMALLOC_MAX_SIZE SIZE_MAX
> +
> +#endif
> diff --git a/tools/include/linux/spinlock.h b/tools/include/linux/spinlock.h
> index c934572..622266b 100644
> --- a/tools/include/linux/spinlock.h
> +++ b/tools/include/linux/spinlock.h
> @@ -37,6 +37,4 @@ static inline bool arch_spin_is_locked(arch_spinlock_t *mutex)
> return true;
> }
>
> -#include <linux/lockdep.h>
> -
> #endif
> --
> 2.7.4
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH net-next 1/2] tools: add missing infrastructure for building ptr_ring.h
2021-07-05 18:39 ` Michael S. Tsirkin
@ 2021-07-06 2:04 ` Yunsheng Lin
2021-07-18 2:09 ` Michael S. Tsirkin
0 siblings, 1 reply; 17+ messages in thread
From: Yunsheng Lin @ 2021-07-06 2:04 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: davem, kuba, jasowang, nickhu, green.hu, deanbo422, akpm,
yury.norov, andriy.shevchenko, ojeda, ndesaulniers, joe,
linux-kernel, virtualization, netdev
On 2021/7/6 2:39, Michael S. Tsirkin wrote:
> 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 <linyunsheng@huawei.com>
>
> This is hard to review.
> Try to split this please. Functional changes separate from
> merely moving code around.
Sure.
>
>
>> ---
>> 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
>>
>> diff --git a/tools/include/asm/cache.h b/tools/include/asm/cache.h
>> new file mode 100644
>> index 0000000..071e310
>> --- /dev/null
>> +++ b/tools/include/asm/cache.h
>> @@ -0,0 +1,56 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +
>> +#ifndef __TOOLS_LINUX_ASM_CACHE_H
>> +#define __TOOLS_LINUX_ASM_CACHE_H
>> +
>> +#include <generated/autoconf.h>
>> +
>> +#if defined(__i386__) || defined(__x86_64__)
>> +#define L1_CACHE_SHIFT (CONFIG_X86_L1_CACHE_SHIFT)
>> +#elif defined(__arm__)
>> +#define L1_CACHE_SHIFT (CONFIG_ARM_L1_CACHE_SHIFT)
>> +#elif defined(__aarch64__)
>> +#define L1_CACHE_SHIFT (6)
>> +#elif defined(__powerpc__)
>> +
>> +/* bytes per L1 cache line */
>> +#if defined(CONFIG_PPC_8xx)
>> +#define L1_CACHE_SHIFT 4
>> +#elif defined(CONFIG_PPC_E500MC)
>> +#define L1_CACHE_SHIFT 6
>> +#elif defined(CONFIG_PPC32)
>> +#if defined(CONFIG_PPC_47x)
>> +#define L1_CACHE_SHIFT 7
>> +#else
>> +#define L1_CACHE_SHIFT 5
>> +#endif
>> +#else /* CONFIG_PPC64 */
>> +#define L1_CACHE_SHIFT 7
>> +#endif
>> +
>> +#elif defined(__sparc__)
>> +#define L1_CACHE_SHIFT 5
>> +#elif defined(__alpha__)
>> +
>> +#if defined(CONFIG_ALPHA_GENERIC) || defined(CONFIG_ALPHA_EV6)
>> +#define L1_CACHE_SHIFT 6
>> +#else
>> +/* Both EV4 and EV5 are write-through, read-allocate,
>> + direct-mapped, physical.
>> +*/
>> +#define L1_CACHE_SHIFT 5
>> +#endif
>> +
>> +#elif defined(__mips__)
>> +#define L1_CACHE_SHIFT CONFIG_MIPS_L1_CACHE_SHIFT
>> +#elif defined(__ia64__)
>> +#define L1_CACHE_SHIFT CONFIG_IA64_L1_CACHE_SHIFT
>> +#elif defined(__nds32__)
>> +#define L1_CACHE_SHIFT 5
>> +#else
>> +#define L1_CACHE_SHIFT 5
>> +#endif
>> +
>> +#define L1_CACHE_BYTES (1 << L1_CACHE_SHIFT)
>> +
>> +#endif
>> 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.
>
>> +#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.
>
>
>> +
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 1/2] tools: add missing infrastructure for building ptr_ring.h
2021-07-06 2:04 ` Yunsheng Lin
@ 2021-07-18 2:09 ` Michael S. Tsirkin
2021-07-19 1:40 ` Yunsheng Lin
0 siblings, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2021-07-18 2:09 UTC (permalink / raw)
To: Yunsheng Lin
Cc: davem, kuba, jasowang, nickhu, green.hu, deanbo422, akpm,
yury.norov, andriy.shevchenko, ojeda, ndesaulniers, joe,
linux-kernel, virtualization, netdev
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:
> >> 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 <linyunsheng@huawei.com>
> >
> > This is hard to review.
> > Try to split this please. Functional changes separate from
> > merely moving code around.
>
> Sure.
>
> >
> >
> >> ---
> >> 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
> >>
> >> diff --git a/tools/include/asm/cache.h b/tools/include/asm/cache.h
> >> new file mode 100644
> >> index 0000000..071e310
> >> --- /dev/null
> >> +++ b/tools/include/asm/cache.h
> >> @@ -0,0 +1,56 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 */
> >> +
> >> +#ifndef __TOOLS_LINUX_ASM_CACHE_H
> >> +#define __TOOLS_LINUX_ASM_CACHE_H
> >> +
> >> +#include <generated/autoconf.h>
> >> +
> >> +#if defined(__i386__) || defined(__x86_64__)
> >> +#define L1_CACHE_SHIFT (CONFIG_X86_L1_CACHE_SHIFT)
> >> +#elif defined(__arm__)
> >> +#define L1_CACHE_SHIFT (CONFIG_ARM_L1_CACHE_SHIFT)
> >> +#elif defined(__aarch64__)
> >> +#define L1_CACHE_SHIFT (6)
> >> +#elif defined(__powerpc__)
> >> +
> >> +/* bytes per L1 cache line */
> >> +#if defined(CONFIG_PPC_8xx)
> >> +#define L1_CACHE_SHIFT 4
> >> +#elif defined(CONFIG_PPC_E500MC)
> >> +#define L1_CACHE_SHIFT 6
> >> +#elif defined(CONFIG_PPC32)
> >> +#if defined(CONFIG_PPC_47x)
> >> +#define L1_CACHE_SHIFT 7
> >> +#else
> >> +#define L1_CACHE_SHIFT 5
> >> +#endif
> >> +#else /* CONFIG_PPC64 */
> >> +#define L1_CACHE_SHIFT 7
> >> +#endif
> >> +
> >> +#elif defined(__sparc__)
> >> +#define L1_CACHE_SHIFT 5
> >> +#elif defined(__alpha__)
> >> +
> >> +#if defined(CONFIG_ALPHA_GENERIC) || defined(CONFIG_ALPHA_EV6)
> >> +#define L1_CACHE_SHIFT 6
> >> +#else
> >> +/* Both EV4 and EV5 are write-through, read-allocate,
> >> + direct-mapped, physical.
> >> +*/
> >> +#define L1_CACHE_SHIFT 5
> >> +#endif
> >> +
> >> +#elif defined(__mips__)
> >> +#define L1_CACHE_SHIFT CONFIG_MIPS_L1_CACHE_SHIFT
> >> +#elif defined(__ia64__)
> >> +#define L1_CACHE_SHIFT CONFIG_IA64_L1_CACHE_SHIFT
> >> +#elif defined(__nds32__)
> >> +#define L1_CACHE_SHIFT 5
> >> +#else
> >> +#define L1_CACHE_SHIFT 5
> >> +#endif
> >> +
> >> +#define L1_CACHE_BYTES (1 << L1_CACHE_SHIFT)
> >> +
> >> +#endif
> >> 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.
All this is just for cpu_relax - open coding that seems way easier.
> >
> >> +#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.
>
> >
> >
> >> +
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 1/2] tools: add missing infrastructure for building ptr_ring.h
2021-07-18 2:09 ` Michael S. Tsirkin
@ 2021-07-19 1:40 ` Yunsheng Lin
2021-07-19 11:58 ` Michael S. Tsirkin
0 siblings, 1 reply; 17+ messages in thread
From: Yunsheng Lin @ 2021-07-19 1:40 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: davem, kuba, jasowang, nickhu, green.hu, deanbo422, akpm,
yury.norov, andriy.shevchenko, ojeda, ndesaulniers, joe,
linux-kernel, virtualization, netdev, Eugenio Pérez
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.
>>
>>>
>>>
>>>> +
>
> .
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 1/2] tools: add missing infrastructure for building ptr_ring.h
2021-07-19 1:40 ` Yunsheng Lin
@ 2021-07-19 11:58 ` Michael S. Tsirkin
0 siblings, 0 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2021-07-19 11:58 UTC (permalink / raw)
To: Yunsheng Lin
Cc: davem, kuba, jasowang, nickhu, green.hu, deanbo422, akpm,
yury.norov, andriy.shevchenko, ojeda, ndesaulniers, joe,
linux-kernel, virtualization, netdev, Eugenio Pérez
On Mon, Jul 19, 2021 at 09:40:39AM +0800, Yunsheng Lin wrote:
> 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.
> >>
> >>>
> >>>
> >>>> +
> >
> > .
I will merge Eugenio's patchset soon.
--
MST
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next 2/2] tools/virtio: use common infrastructure to build ptr_ring.h
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 3:57 ` Yunsheng Lin
2021-07-05 9:56 ` [PATCH net-next 0/2] refactor the ringtest testing for ptr_ring Andy Shevchenko
2 siblings, 0 replies; 17+ messages in thread
From: Yunsheng Lin @ 2021-07-05 3:57 UTC (permalink / raw)
To: davem, kuba, mst, jasowang
Cc: nickhu, green.hu, deanbo422, akpm, yury.norov, andriy.shevchenko,
ojeda, ndesaulniers, joe, linux-kernel, virtualization, netdev
Use the common infrastructure in tools/include to build
ptr_ring.h in user space.
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
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(-)
diff --git a/tools/virtio/ringtest/Makefile b/tools/virtio/ringtest/Makefile
index 85c98c2..89fc024 100644
--- a/tools/virtio/ringtest/Makefile
+++ b/tools/virtio/ringtest/Makefile
@@ -3,7 +3,7 @@ all:
all: ring virtio_ring_0_9 virtio_ring_poll virtio_ring_inorder ptr_ring noring
-CFLAGS += -Wall
+CFLAGS += -Wall -I../../include
CFLAGS += -pthread -O2 -ggdb -flto -fwhole-program
LDFLAGS += -pthread -O2 -ggdb -flto -fwhole-program
diff --git a/tools/virtio/ringtest/main.h b/tools/virtio/ringtest/main.h
index 6d1fccd..95ea050 100644
--- a/tools/virtio/ringtest/main.h
+++ b/tools/virtio/ringtest/main.h
@@ -10,6 +10,13 @@
#include <stdbool.h>
+#include <linux/compiler.h>
+#include <asm/barrier.h>
+#include <asm/processor.h>
+
+#define smp_acquire smp_rmb
+#define smp_release smp_wmb
+
extern int param;
extern bool do_exit;
@@ -87,18 +94,6 @@ void wait_for_call(void);
extern unsigned ring_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
-
extern bool do_relax;
static inline void busy_wait(void)
@@ -110,85 +105,4 @@ static inline void busy_wait(void)
barrier();
}
-#if defined(__x86_64__) || defined(__i386__)
-#define smp_mb() asm volatile("lock; addl $0,-132(%%rsp)" ::: "memory", "cc")
-#else
-/*
- * Not using __ATOMIC_SEQ_CST since gcc docs say they are only synchronized
- * with other __ATOMIC_SEQ_CST calls.
- */
-#define smp_mb() __sync_synchronize()
-#endif
-
-/*
- * This abuses the atomic builtins for thread fences, and
- * adds a compiler barrier.
- */
-#define smp_release() do { \
- barrier(); \
- __atomic_thread_fence(__ATOMIC_RELEASE); \
-} while (0)
-
-#define smp_acquire() do { \
- __atomic_thread_fence(__ATOMIC_ACQUIRE); \
- barrier(); \
-} while (0)
-
-#if defined(__i386__) || defined(__x86_64__) || defined(__s390x__)
-#define smp_wmb() barrier()
-#else
-#define smp_wmb() smp_release()
-#endif
-
-#ifdef __alpha__
-#define smp_read_barrier_depends() smp_acquire()
-#else
-#define smp_read_barrier_depends() do {} while(0)
-#endif
-
-static __always_inline
-void __read_once_size(const volatile void *p, void *res, int size)
-{
- switch (size) { \
- case 1: *(unsigned char *)res = *(volatile unsigned char *)p; break; \
- case 2: *(unsigned short *)res = *(volatile unsigned short *)p; break; \
- case 4: *(unsigned int *)res = *(volatile unsigned int *)p; break; \
- case 8: *(unsigned long long *)res = *(volatile unsigned long long *)p; break; \
- default: \
- barrier(); \
- __builtin_memcpy((void *)res, (const void *)p, size); \
- barrier(); \
- } \
-}
-
-static __always_inline void __write_once_size(volatile void *p, void *res, int size)
-{
- switch (size) {
- case 1: *(volatile unsigned char *)p = *(unsigned char *)res; break;
- case 2: *(volatile unsigned short *)p = *(unsigned short *)res; break;
- case 4: *(volatile unsigned int *)p = *(unsigned int *)res; break;
- case 8: *(volatile unsigned long long *)p = *(unsigned long long *)res; break;
- default:
- barrier();
- __builtin_memcpy((void *)p, (const void *)res, size);
- barrier();
- }
-}
-
-#define READ_ONCE(x) \
-({ \
- union { typeof(x) __val; char __c[1]; } __u; \
- __read_once_size(&(x), __u.__c, sizeof(x)); \
- smp_read_barrier_depends(); /* Enforce dependency ordering from x */ \
- __u.__val; \
-})
-
-#define WRITE_ONCE(x, val) \
-({ \
- union { typeof(x) __val; char __c[1]; } __u = \
- { .__val = (typeof(x)) (val) }; \
- __write_once_size(&(x), __u.__c, sizeof(x)); \
- __u.__val; \
-})
-
#endif
diff --git a/tools/virtio/ringtest/ptr_ring.c b/tools/virtio/ringtest/ptr_ring.c
index c9b2633..e058874 100644
--- a/tools/virtio/ringtest/ptr_ring.c
+++ b/tools/virtio/ringtest/ptr_ring.c
@@ -10,104 +10,10 @@
#include <errno.h>
#include <limits.h>
-#define SMP_CACHE_BYTES 64
-#define cache_line_size() SMP_CACHE_BYTES
-#define ____cacheline_aligned_in_smp __attribute__ ((aligned (SMP_CACHE_BYTES)))
-#define unlikely(x) (__builtin_expect(!!(x), 0))
-#define likely(x) (__builtin_expect(!!(x), 1))
-#define ALIGN(x, a) (((x) + (a) - 1) / (a) * (a))
-#define SIZE_MAX (~(size_t)0)
-#define KMALLOC_MAX_SIZE SIZE_MAX
-
-typedef pthread_spinlock_t spinlock_t;
-
-typedef int gfp_t;
-#define __GFP_ZERO 0x1
-
-static void *kmalloc(unsigned size, gfp_t gfp)
-{
- void *p = memalign(64, size);
- if (!p)
- return p;
-
- if (gfp & __GFP_ZERO)
- memset(p, 0, size);
- return p;
-}
-
-static inline void *kzalloc(unsigned size, gfp_t flags)
-{
- return kmalloc(size, flags | __GFP_ZERO);
-}
-
-static inline void *kmalloc_array(size_t n, size_t size, gfp_t flags)
-{
- if (size != 0 && n > SIZE_MAX / size)
- return NULL;
- return kmalloc(n * size, flags);
-}
-
-static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
-{
- return kmalloc_array(n, size, flags | __GFP_ZERO);
-}
-
-static void kfree(void *p)
-{
- if (p)
- free(p);
-}
-
-#define kvmalloc_array kmalloc_array
-#define kvfree kfree
-
-static void spin_lock_init(spinlock_t *lock)
-{
- int r = pthread_spin_init(lock, 0);
- assert(!r);
-}
-
-static void spin_lock(spinlock_t *lock)
-{
- int ret = pthread_spin_lock(lock);
- assert(!ret);
-}
-
-static void spin_unlock(spinlock_t *lock)
-{
- int ret = pthread_spin_unlock(lock);
- assert(!ret);
-}
-
-static void spin_lock_bh(spinlock_t *lock)
-{
- spin_lock(lock);
-}
-
-static void spin_unlock_bh(spinlock_t *lock)
-{
- spin_unlock(lock);
-}
-
-static void spin_lock_irq(spinlock_t *lock)
-{
- spin_lock(lock);
-}
-
-static void spin_unlock_irq(spinlock_t *lock)
-{
- spin_unlock(lock);
-}
-
-static void spin_lock_irqsave(spinlock_t *lock, unsigned long f)
-{
- spin_lock(lock);
-}
-
-static void spin_unlock_irqrestore(spinlock_t *lock, unsigned long f)
-{
- spin_unlock(lock);
-}
+#include <linux/align.h>
+#include <linux/cache.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
#include "../../../include/linux/ptr_ring.h"
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH net-next 0/2] refactor the ringtest testing for ptr_ring
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 3:57 ` [PATCH net-next 2/2] tools/virtio: use common infrastructure to build ptr_ring.h Yunsheng Lin
@ 2021-07-05 9:56 ` Andy Shevchenko
2021-07-05 12:06 ` Yunsheng Lin
2 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2021-07-05 9:56 UTC (permalink / raw)
To: Yunsheng Lin
Cc: davem, kuba, mst, jasowang, nickhu, green.hu, deanbo422, akpm,
yury.norov, ojeda, ndesaulniers, joe, linux-kernel,
virtualization, netdev
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=...`?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 0/2] refactor the ringtest testing for ptr_ring
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
0 siblings, 2 replies; 17+ messages in thread
From: Yunsheng Lin @ 2021-07-05 12:06 UTC (permalink / raw)
To: Andy Shevchenko
Cc: davem, kuba, mst, jasowang, nickhu, green.hu, deanbo422, akpm,
yury.norov, ojeda, ndesaulniers, joe, linux-kernel,
virtualization, netdev
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?
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 0/2] refactor the ringtest testing for ptr_ring
2021-07-05 12:06 ` Yunsheng Lin
@ 2021-07-05 14:57 ` Andy Shevchenko
2021-07-05 18:26 ` Michael S. Tsirkin
1 sibling, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2021-07-05 14:57 UTC (permalink / raw)
To: Yunsheng Lin
Cc: davem, kuba, mst, jasowang, nickhu, green.hu, deanbo422, akpm,
yury.norov, ojeda, ndesaulniers, joe, linux-kernel,
virtualization, netdev
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.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 0/2] refactor the ringtest testing for ptr_ring
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
1 sibling, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2021-07-05 18:26 UTC (permalink / raw)
To: Yunsheng Lin
Cc: Andy Shevchenko, davem, kuba, jasowang, nickhu, green.hu,
deanbo422, akpm, yury.norov, ojeda, ndesaulniers, joe,
linux-kernel, virtualization, netdev
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?
>
> >
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 0/2] refactor the ringtest testing for ptr_ring
2021-07-05 18:26 ` Michael S. Tsirkin
@ 2021-07-05 18:36 ` Andy Shevchenko
2021-07-05 18:42 ` Michael S. Tsirkin
0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2021-07-05 18:36 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Yunsheng Lin, davem, kuba, jasowang, nickhu, green.hu, deanbo422,
akpm, yury.norov, ojeda, ndesaulniers, joe, linux-kernel,
virtualization, netdev
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?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 0/2] refactor the ringtest testing for ptr_ring
2021-07-05 18:36 ` Andy Shevchenko
@ 2021-07-05 18:42 ` Michael S. Tsirkin
2021-07-05 19:05 ` Andy Shevchenko
0 siblings, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2021-07-05 18:42 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Yunsheng Lin, davem, kuba, jasowang, nickhu, green.hu, deanbo422,
akpm, yury.norov, ojeda, ndesaulniers, joe, linux-kernel,
virtualization, netdev
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 0/2] refactor the ringtest testing for ptr_ring
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
0 siblings, 2 replies; 17+ messages in thread
From: Andy Shevchenko @ 2021-07-05 19:05 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Andy Shevchenko, Yunsheng Lin, David S. Miller, Jakub Kicinski,
Jason Wang, Nick Hu, Greentime Hu, Vincent Chen, Andrew Morton,
Yury Norov, Miguel Ojeda, ndesaulniers, Joe Perches,
Linux Kernel Mailing List, virtualization, netdev
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?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 0/2] refactor the ringtest testing for ptr_ring
2021-07-05 19:05 ` Andy Shevchenko
@ 2021-07-05 19:31 ` Michael S. Tsirkin
2021-07-06 1:35 ` Yunsheng Lin
1 sibling, 0 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2021-07-05 19:31 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andy Shevchenko, Yunsheng Lin, David S. Miller, Jakub Kicinski,
Jason Wang, Nick Hu, Greentime Hu, Vincent Chen, Andrew Morton,
Yury Norov, Miguel Ojeda, ndesaulniers, Joe Perches,
Linux Kernel Mailing List, virtualization, netdev
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 0/2] refactor the ringtest testing for ptr_ring
2021-07-05 19:05 ` Andy Shevchenko
2021-07-05 19:31 ` Michael S. Tsirkin
@ 2021-07-06 1:35 ` Yunsheng Lin
1 sibling, 0 replies; 17+ messages in thread
From: Yunsheng Lin @ 2021-07-06 1:35 UTC (permalink / raw)
To: Andy Shevchenko, Michael S. Tsirkin
Cc: Andy Shevchenko, David S. Miller, Jakub Kicinski, Jason Wang,
Nick Hu, Greentime Hu, Vincent Chen, Andrew Morton, Yury Norov,
Miguel Ojeda, ndesaulniers, Joe Perches,
Linux Kernel Mailing List, virtualization, netdev
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?
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread